| Message ID | 20250501062344.2526061-31-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary: compile target/arm twice | expand |
On 4/30/25 23:23, Pierrick Bouvier wrote: > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > target/arm/ptw.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index 424d1b54275..f428c9b9267 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -737,7 +737,14 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, > uint64_t new_val, S1Translate *ptw, > ARMMMUFaultInfo *fi) > { > -#if defined(TARGET_AARCH64) && defined(CONFIG_TCG) > +#ifndef CONFIG_TCG > + /* non-TCG guests only use debug-mode. */ > + g_assert_not_reached(); > +#endif > + > + /* AArch32 does not have FEAT_HADFS */ > + g_assert(arm_feature(env, ARM_FEATURE_AARCH64)); > + I don't think we need an assert here. The ifdef for aarch64 also protects the qatomic_cmpxchg__nocheck below, because aarch64 guest can only be built with a 64-bit host. Are we still able to build qemu-system-arm on a 32-bit host with these changes? It may be tricky to check, because the two easiest 32-bit hosts (i686, armv7) also happen to have a 64-bit cmpxchg. I think "make docker-test-build@debian-mipsel-cross" will be the right test. If that fails, I think you could s/TARGET_AARCH64/CONFIG_ATOMIC64/ here. r~
On 5/1/25 9:24 AM, Richard Henderson wrote: > On 4/30/25 23:23, Pierrick Bouvier wrote: >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> target/arm/ptw.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/target/arm/ptw.c b/target/arm/ptw.c >> index 424d1b54275..f428c9b9267 100644 >> --- a/target/arm/ptw.c >> +++ b/target/arm/ptw.c >> @@ -737,7 +737,14 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, >> uint64_t new_val, S1Translate *ptw, >> ARMMMUFaultInfo *fi) >> { >> -#if defined(TARGET_AARCH64) && defined(CONFIG_TCG) >> +#ifndef CONFIG_TCG >> + /* non-TCG guests only use debug-mode. */ >> + g_assert_not_reached(); >> +#endif >> + >> + /* AArch32 does not have FEAT_HADFS */ >> + g_assert(arm_feature(env, ARM_FEATURE_AARCH64)); >> + > > I don't think we need an assert here. > > The ifdef for aarch64 also protects the qatomic_cmpxchg__nocheck below, because aarch64 > guest can only be built with a 64-bit host. > > Are we still able to build qemu-system-arm on a 32-bit host with these changes? It may be > tricky to check, because the two easiest 32-bit hosts (i686, armv7) also happen to have a > 64-bit cmpxchg. I think "make docker-test-build@debian-mipsel-cross" will be the right test. > Good catch. I was indeed building only with i686. I'll add mipsel instead. /usr/lib/gcc-cross/mipsel-linux-gnu/12/../../../../mipsel-linux-gnu/bin/ld: libtarget_system_arm.a.p/target_arm_ptw.c.o: undefined reference to symbol '__atomic_compare_exchange_8@@LIBATOMIC_1.0' > If that fails, I think you could s/TARGET_AARCH64/CONFIG_ATOMIC64/ here. > I'll revert this change and simply do this replace, adding a comment as well. > > r~
On 5/1/25 9:24 AM, Richard Henderson wrote: > Are we still able to build qemu-system-arm on a 32-bit host with these changes? It may be > tricky to check, because the two easiest 32-bit hosts (i686, armv7) also happen to have a > 64-bit cmpxchg. I think "make docker-test-build@debian-mipsel-cross" will be the right test. By the way, I'm usually using debian-s390x-cross for testing a HOST_BIG_ENDIAN build. Is that the best choice available?
On 5/3/25 15:48, Pierrick Bouvier wrote: > On 5/1/25 9:24 AM, Richard Henderson wrote: >> Are we still able to build qemu-system-arm on a 32-bit host with these changes? It may be >> tricky to check, because the two easiest 32-bit hosts (i686, armv7) also happen to have a >> 64-bit cmpxchg. I think "make docker-test-build@debian-mipsel-cross" will be the right >> test. > > By the way, I'm usually using debian-s390x-cross for testing a HOST_BIG_ENDIAN build. Is > that the best choice available? Yes. r~
diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 424d1b54275..f428c9b9267 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -737,7 +737,14 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, uint64_t new_val, S1Translate *ptw, ARMMMUFaultInfo *fi) { -#if defined(TARGET_AARCH64) && defined(CONFIG_TCG) +#ifndef CONFIG_TCG + /* non-TCG guests only use debug-mode. */ + g_assert_not_reached(); +#endif + + /* AArch32 does not have FEAT_HADFS */ + g_assert(arm_feature(env, ARM_FEATURE_AARCH64)); + uint64_t cur_val; void *host = ptw->out_host; @@ -851,10 +858,6 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, cur_val = le64_to_cpu(cur_val); } return cur_val; -#else - /* AArch32 does not have FEAT_HADFS; non-TCG guests only use debug-mode. */ - g_assert_not_reached(); -#endif } static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- target/arm/ptw.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)