| Message ID | 20250512180502.2395029-31-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary: compile target/arm twice | expand |
On 12/5/25 20:04, Pierrick Bouvier wrote: > This function needs 64 bit compare exchange, so we hide implementation > for hosts not supporting it (some 32 bit target, which don't run 64 bit > guests anyway). > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > target/arm/ptw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index 68ec3f5e755..44170d831cc 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -737,7 +737,7 @@ 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) > +#if defined(CONFIG_ATOMIC64) && defined(CONFIG_TCG) > uint64_t cur_val; > void *host = ptw->out_host; > I'd feel safer squashing: -- >8 -- @@ -743,2 +743,5 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, + /* AArch32 does not have FEAT_HADFS */ + assert(cpu_isar_feature(aa64_hafs, env_archcpu(env))); + if (unlikely(!host)) { @@ -854,3 +857,3 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, #else - /* AArch32 does not have FEAT_HADFS; non-TCG guests only use debug-mode. */ + /* non-TCG guests only use debug-mode. */ g_assert_not_reached(); --- With that: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 5/13/25 03:41, Philippe Mathieu-Daudé wrote: > On 12/5/25 20:04, Pierrick Bouvier wrote: >> This function needs 64 bit compare exchange, so we hide implementation >> for hosts not supporting it (some 32 bit target, which don't run 64 bit >> guests anyway). >> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> target/arm/ptw.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/target/arm/ptw.c b/target/arm/ptw.c >> index 68ec3f5e755..44170d831cc 100644 >> --- a/target/arm/ptw.c >> +++ b/target/arm/ptw.c >> @@ -737,7 +737,7 @@ 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) >> +#if defined(CONFIG_ATOMIC64) && defined(CONFIG_TCG) >> uint64_t cur_val; >> void *host = ptw->out_host; > > I'd feel safer squashing: > > -- >8 -- > @@ -743,2 +743,5 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, > > + /* AArch32 does not have FEAT_HADFS */ > + assert(cpu_isar_feature(aa64_hafs, env_archcpu(env))); Why? This is checked in the setting of param.{ha,hd}. See aa{32,64}_va_parameters. r~
On 13/5/25 19:03, Richard Henderson wrote: > On 5/13/25 03:41, Philippe Mathieu-Daudé wrote: >> On 12/5/25 20:04, Pierrick Bouvier wrote: >>> This function needs 64 bit compare exchange, so we hide implementation >>> for hosts not supporting it (some 32 bit target, which don't run 64 bit >>> guests anyway). >>> >>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>> --- >>> target/arm/ptw.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/target/arm/ptw.c b/target/arm/ptw.c >>> index 68ec3f5e755..44170d831cc 100644 >>> --- a/target/arm/ptw.c >>> +++ b/target/arm/ptw.c >>> @@ -737,7 +737,7 @@ 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) >>> +#if defined(CONFIG_ATOMIC64) && defined(CONFIG_TCG) >>> uint64_t cur_val; >>> void *host = ptw->out_host; >> >> I'd feel safer squashing: >> >> -- >8 -- >> @@ -743,2 +743,5 @@ static uint64_t arm_casq_ptw(CPUARMState *env, >> uint64_t old_val, >> >> + /* AArch32 does not have FEAT_HADFS */ >> + assert(cpu_isar_feature(aa64_hafs, env_archcpu(env))); > > Why? This is checked in the setting of param.{ha,hd}. > See aa{32,64}_va_parameters. I suppose the "AArch32 does not have FEAT_HADFS" is misleading then. Better if no changes are necessary and this series can go as is.
On 5/13/25 18:12, Philippe Mathieu-Daudé wrote: > On 13/5/25 19:03, Richard Henderson wrote: >> On 5/13/25 03:41, Philippe Mathieu-Daudé wrote: >>> + /* AArch32 does not have FEAT_HADFS */ >>> + assert(cpu_isar_feature(aa64_hafs, env_archcpu(env))); >> >> Why? This is checked in the setting of param.{ha,hd}. >> See aa{32,64}_va_parameters. > > I suppose the "AArch32 does not have FEAT_HADFS" is misleading then. It isn't -- aa32_va_parameters does not set param.{ha,hd}. r~
diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 68ec3f5e755..44170d831cc 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -737,7 +737,7 @@ 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) +#if defined(CONFIG_ATOMIC64) && defined(CONFIG_TCG) uint64_t cur_val; void *host = ptw->out_host;