| Message ID | 20250318213209.2579218-7-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg, codebase: Build once patches | expand |
On 3/18/25 14:31, Richard Henderson wrote: > The implementation of cpu_mmu_index was split between cpu-common.h > and cpu-all.h, depending on CONFIG_USER_ONLY. We already have the > plumbing common to user and system mode. Using MMU_USER_IDX > requires the cpu.h for a specific target, and so is restricted to > when we're compiling per-target. > A side question: Why is MMU_USER_IDX different depending on architecture? I'm trying to understand why (and by what) previous indexes are reserved when MMU_USER_IDX is not zero. > Include the new header only where needed. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/exec/cpu-all.h | 6 ------ > include/exec/cpu-common.h | 20 ------------------ > include/exec/cpu-mmu-index.h | 39 +++++++++++++++++++++++++++++++++++ > include/exec/cpu_ldst.h | 1 + > semihosting/uaccess.c | 1 + > target/arm/gdbstub64.c | 3 +++ > target/hppa/mem_helper.c | 1 + > target/i386/tcg/translate.c | 1 + > target/loongarch/cpu_helper.c | 1 + > target/microblaze/helper.c | 1 + > target/microblaze/mmu.c | 1 + > target/openrisc/translate.c | 1 + > target/sparc/cpu.c | 1 + > target/sparc/mmu_helper.c | 1 + > target/tricore/helper.c | 1 + > target/xtensa/mmu_helper.c | 1 + > 16 files changed, 54 insertions(+), 26 deletions(-) > create mode 100644 include/exec/cpu-mmu-index.h > > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h > index 902ca1f3c7..6108351f58 100644 > --- a/include/exec/cpu-all.h > +++ b/include/exec/cpu-all.h > @@ -48,8 +48,6 @@ CPUArchState *cpu_copy(CPUArchState *env); > > #ifdef CONFIG_USER_ONLY > > -static inline int cpu_mmu_index(CPUState *cs, bool ifetch); > - > /* > * Allow some level of source compatibility with softmmu. We do not > * support any of the more exotic features, so only invalid pages may > @@ -59,10 +57,6 @@ static inline int cpu_mmu_index(CPUState *cs, bool ifetch); > #define TLB_MMIO (1 << (TARGET_PAGE_BITS_MIN - 2)) > #define TLB_WATCHPOINT 0 > > -static inline int cpu_mmu_index(CPUState *cs, bool ifetch) > -{ > - return MMU_USER_IDX; > -} > #else > > /* > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h > index 3771b2130c..be032e1a49 100644 > --- a/include/exec/cpu-common.h > +++ b/include/exec/cpu-common.h > @@ -272,24 +272,4 @@ static inline CPUState *env_cpu(CPUArchState *env) > return (CPUState *)env_cpu_const(env); > } > > -#ifndef CONFIG_USER_ONLY > -/** > - * cpu_mmu_index: > - * @env: The cpu environment > - * @ifetch: True for code access, false for data access. > - * > - * Return the core mmu index for the current translation regime. > - * This function is used by generic TCG code paths. > - * > - * The user-only version of this function is inline in cpu-all.h, > - * where it always returns MMU_USER_IDX. > - */ > -static inline int cpu_mmu_index(CPUState *cs, bool ifetch) > -{ > - int ret = cs->cc->mmu_index(cs, ifetch); > - tcg_debug_assert(ret >= 0 && ret < NB_MMU_MODES); > - return ret; > -} > -#endif /* !CONFIG_USER_ONLY */ > - > #endif /* CPU_COMMON_H */ > diff --git a/include/exec/cpu-mmu-index.h b/include/exec/cpu-mmu-index.h > new file mode 100644 > index 0000000000..b46e622048 > --- /dev/null > +++ b/include/exec/cpu-mmu-index.h > @@ -0,0 +1,39 @@ > +/* > + * cpu_mmu_index() > + * > + * Copyright (c) 2003 Fabrice Bellard > + * > + * SPDX-License-Identifier: LGPL-2.1+ > + */ > + > +#ifndef EXEC_CPU_MMU_INDEX_H > +#define EXEC_CPU_MMU_INDEX_H > + > +#include "hw/core/cpu.h" > +#include "tcg/debug-assert.h" > +#ifdef COMPILING_PER_TARGET > +#include "cpu.h" > +#endif > + > +/** > + * cpu_mmu_index: > + * @env: The cpu environment > + * @ifetch: True for code access, false for data access. > + * > + * Return the core mmu index for the current translation regime. > + * This function is used by generic TCG code paths. > + */ > +static inline int cpu_mmu_index(CPUState *cs, bool ifetch) > +{ > +#ifdef COMPILING_PER_TARGET > +# ifdef CONFIG_USER_ONLY > + return MMU_USER_IDX; > +# endif > +#endif > + > + int ret = cs->cc->mmu_index(cs, ifetch); > + tcg_debug_assert(ret >= 0 && ret < NB_MMU_MODES); > + return ret; > +} > + > +#endif /* EXEC_CPU_MMU_INDEX_H */ > diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h > index 1fbdbe59ae..0b10d840fe 100644 > --- a/include/exec/cpu_ldst.h > +++ b/include/exec/cpu_ldst.h > @@ -67,6 +67,7 @@ > #endif > > #include "exec/cpu-ldst-common.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/abi_ptr.h" > > #if defined(CONFIG_USER_ONLY) > diff --git a/semihosting/uaccess.c b/semihosting/uaccess.c > index 382a366ce3..a957891166 100644 > --- a/semihosting/uaccess.c > +++ b/semihosting/uaccess.c > @@ -9,6 +9,7 @@ > > #include "qemu/osdep.h" > #include "exec/cpu-all.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/exec-all.h" > #include "semihosting/uaccess.h" > > diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c > index 1a4dbec567..a9d8352b76 100644 > --- a/target/arm/gdbstub64.c > +++ b/target/arm/gdbstub64.c > @@ -27,6 +27,9 @@ > #include <sys/prctl.h> > #include "mte_user_helper.h" > #endif > +#ifdef CONFIG_TCG > +#include "exec/cpu-mmu-index.h" > +#endif > > int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > { > diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c > index fb1d93ef1f..510786518d 100644 > --- a/target/hppa/mem_helper.c > +++ b/target/hppa/mem_helper.c > @@ -22,6 +22,7 @@ > #include "cpu.h" > #include "exec/exec-all.h" > #include "exec/cputlb.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/page-protection.h" > #include "exec/helper-proto.h" > #include "hw/core/cpu.h" > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c > index a8935f487a..20a5c69795 100644 > --- a/target/i386/tcg/translate.c > +++ b/target/i386/tcg/translate.c > @@ -20,6 +20,7 @@ > > #include "qemu/host-utils.h" > #include "cpu.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/exec-all.h" > #include "exec/translation-block.h" > #include "tcg/tcg-op.h" > diff --git a/target/loongarch/cpu_helper.c b/target/loongarch/cpu_helper.c > index 930466ca48..8662fb36ed 100644 > --- a/target/loongarch/cpu_helper.c > +++ b/target/loongarch/cpu_helper.c > @@ -8,6 +8,7 @@ > > #include "qemu/osdep.h" > #include "cpu.h" > +#include "exec/cpu-mmu-index.h" > #include "internals.h" > #include "cpu-csr.h" > > diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c > index 27fc929bee..996514ffe8 100644 > --- a/target/microblaze/helper.c > +++ b/target/microblaze/helper.c > @@ -21,6 +21,7 @@ > #include "qemu/osdep.h" > #include "cpu.h" > #include "exec/cputlb.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/page-protection.h" > #include "qemu/host-utils.h" > #include "exec/log.h" > diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c > index f8587d5ac4..987ac9e3a7 100644 > --- a/target/microblaze/mmu.c > +++ b/target/microblaze/mmu.c > @@ -22,6 +22,7 @@ > #include "qemu/log.h" > #include "cpu.h" > #include "exec/cputlb.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/page-protection.h" > > static unsigned int tlb_decode_size(unsigned int f) > diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c > index 7a6af183ae..5b437959ac 100644 > --- a/target/openrisc/translate.c > +++ b/target/openrisc/translate.c > @@ -20,6 +20,7 @@ > > #include "qemu/osdep.h" > #include "cpu.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/exec-all.h" > #include "tcg/tcg-op.h" > #include "qemu/log.h" > diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c > index 5716120117..1bf00407af 100644 > --- a/target/sparc/cpu.c > +++ b/target/sparc/cpu.c > @@ -22,6 +22,7 @@ > #include "cpu.h" > #include "qemu/module.h" > #include "qemu/qemu-print.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/exec-all.h" > #include "exec/translation-block.h" > #include "hw/qdev-properties.h" > diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c > index 7548d01777..4a0cedd9e2 100644 > --- a/target/sparc/mmu_helper.c > +++ b/target/sparc/mmu_helper.c > @@ -21,6 +21,7 @@ > #include "qemu/log.h" > #include "cpu.h" > #include "exec/cputlb.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/page-protection.h" > #include "qemu/qemu-print.h" > #include "trace.h" > diff --git a/target/tricore/helper.c b/target/tricore/helper.c > index a64412e6bd..be3d97af78 100644 > --- a/target/tricore/helper.c > +++ b/target/tricore/helper.c > @@ -20,6 +20,7 @@ > #include "hw/registerfields.h" > #include "cpu.h" > #include "exec/cputlb.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/page-protection.h" > #include "fpu/softfloat-helpers.h" > #include "qemu/qemu-print.h" > diff --git a/target/xtensa/mmu_helper.c b/target/xtensa/mmu_helper.c > index 63be741a42..96140c89c7 100644 > --- a/target/xtensa/mmu_helper.c > +++ b/target/xtensa/mmu_helper.c > @@ -33,6 +33,7 @@ > #include "exec/helper-proto.h" > #include "qemu/host-utils.h" > #include "exec/cputlb.h" > +#include "exec/cpu-mmu-index.h" > #include "exec/exec-all.h" > #include "exec/page-protection.h" > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
On 3/18/25 17:02, Pierrick Bouvier wrote: > On 3/18/25 14:31, Richard Henderson wrote: >> The implementation of cpu_mmu_index was split between cpu-common.h >> and cpu-all.h, depending on CONFIG_USER_ONLY. We already have the >> plumbing common to user and system mode. Using MMU_USER_IDX >> requires the cpu.h for a specific target, and so is restricted to >> when we're compiling per-target. >> > > A side question: Why is MMU_USER_IDX different depending on architecture? > I'm trying to understand why (and by what) previous indexes are reserved when MMU_USER_IDX > is not zero. Depends on the translator, but often: cpu_mmu_index may be encoded into tb_flags, and the translator *also* uses this encoding to determine the priv state. So if, in user-only mode, we fail to encode MMU_USER_IDX into tb_flags, we'll get incorrect priv checks in the translator and fail to raise SIGILL for privledged operations. Depending on the target, the mmu_index space may be quite complicated, with various meanings assigned to various bits. Thus "0" may not be reasonable for MMU_USER_IDX. See, for instance, enum ARMMMUIdx or ppc hreg_compute_hflags_value(). (Both of which, amusingly, use MMU_USER_IDX 0; neither here nor there.) r~
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index 902ca1f3c7..6108351f58 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -48,8 +48,6 @@ CPUArchState *cpu_copy(CPUArchState *env); #ifdef CONFIG_USER_ONLY -static inline int cpu_mmu_index(CPUState *cs, bool ifetch); - /* * Allow some level of source compatibility with softmmu. We do not * support any of the more exotic features, so only invalid pages may @@ -59,10 +57,6 @@ static inline int cpu_mmu_index(CPUState *cs, bool ifetch); #define TLB_MMIO (1 << (TARGET_PAGE_BITS_MIN - 2)) #define TLB_WATCHPOINT 0 -static inline int cpu_mmu_index(CPUState *cs, bool ifetch) -{ - return MMU_USER_IDX; -} #else /* diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h index 3771b2130c..be032e1a49 100644 --- a/include/exec/cpu-common.h +++ b/include/exec/cpu-common.h @@ -272,24 +272,4 @@ static inline CPUState *env_cpu(CPUArchState *env) return (CPUState *)env_cpu_const(env); } -#ifndef CONFIG_USER_ONLY -/** - * cpu_mmu_index: - * @env: The cpu environment - * @ifetch: True for code access, false for data access. - * - * Return the core mmu index for the current translation regime. - * This function is used by generic TCG code paths. - * - * The user-only version of this function is inline in cpu-all.h, - * where it always returns MMU_USER_IDX. - */ -static inline int cpu_mmu_index(CPUState *cs, bool ifetch) -{ - int ret = cs->cc->mmu_index(cs, ifetch); - tcg_debug_assert(ret >= 0 && ret < NB_MMU_MODES); - return ret; -} -#endif /* !CONFIG_USER_ONLY */ - #endif /* CPU_COMMON_H */ diff --git a/include/exec/cpu-mmu-index.h b/include/exec/cpu-mmu-index.h new file mode 100644 index 0000000000..b46e622048 --- /dev/null +++ b/include/exec/cpu-mmu-index.h @@ -0,0 +1,39 @@ +/* + * cpu_mmu_index() + * + * Copyright (c) 2003 Fabrice Bellard + * + * SPDX-License-Identifier: LGPL-2.1+ + */ + +#ifndef EXEC_CPU_MMU_INDEX_H +#define EXEC_CPU_MMU_INDEX_H + +#include "hw/core/cpu.h" +#include "tcg/debug-assert.h" +#ifdef COMPILING_PER_TARGET +#include "cpu.h" +#endif + +/** + * cpu_mmu_index: + * @env: The cpu environment + * @ifetch: True for code access, false for data access. + * + * Return the core mmu index for the current translation regime. + * This function is used by generic TCG code paths. + */ +static inline int cpu_mmu_index(CPUState *cs, bool ifetch) +{ +#ifdef COMPILING_PER_TARGET +# ifdef CONFIG_USER_ONLY + return MMU_USER_IDX; +# endif +#endif + + int ret = cs->cc->mmu_index(cs, ifetch); + tcg_debug_assert(ret >= 0 && ret < NB_MMU_MODES); + return ret; +} + +#endif /* EXEC_CPU_MMU_INDEX_H */ diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h index 1fbdbe59ae..0b10d840fe 100644 --- a/include/exec/cpu_ldst.h +++ b/include/exec/cpu_ldst.h @@ -67,6 +67,7 @@ #endif #include "exec/cpu-ldst-common.h" +#include "exec/cpu-mmu-index.h" #include "exec/abi_ptr.h" #if defined(CONFIG_USER_ONLY) diff --git a/semihosting/uaccess.c b/semihosting/uaccess.c index 382a366ce3..a957891166 100644 --- a/semihosting/uaccess.c +++ b/semihosting/uaccess.c @@ -9,6 +9,7 @@ #include "qemu/osdep.h" #include "exec/cpu-all.h" +#include "exec/cpu-mmu-index.h" #include "exec/exec-all.h" #include "semihosting/uaccess.h" diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c index 1a4dbec567..a9d8352b76 100644 --- a/target/arm/gdbstub64.c +++ b/target/arm/gdbstub64.c @@ -27,6 +27,9 @@ #include <sys/prctl.h> #include "mte_user_helper.h" #endif +#ifdef CONFIG_TCG +#include "exec/cpu-mmu-index.h" +#endif int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) { diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c index fb1d93ef1f..510786518d 100644 --- a/target/hppa/mem_helper.c +++ b/target/hppa/mem_helper.c @@ -22,6 +22,7 @@ #include "cpu.h" #include "exec/exec-all.h" #include "exec/cputlb.h" +#include "exec/cpu-mmu-index.h" #include "exec/page-protection.h" #include "exec/helper-proto.h" #include "hw/core/cpu.h" diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index a8935f487a..20a5c69795 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -20,6 +20,7 @@ #include "qemu/host-utils.h" #include "cpu.h" +#include "exec/cpu-mmu-index.h" #include "exec/exec-all.h" #include "exec/translation-block.h" #include "tcg/tcg-op.h" diff --git a/target/loongarch/cpu_helper.c b/target/loongarch/cpu_helper.c index 930466ca48..8662fb36ed 100644 --- a/target/loongarch/cpu_helper.c +++ b/target/loongarch/cpu_helper.c @@ -8,6 +8,7 @@ #include "qemu/osdep.h" #include "cpu.h" +#include "exec/cpu-mmu-index.h" #include "internals.h" #include "cpu-csr.h" diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c index 27fc929bee..996514ffe8 100644 --- a/target/microblaze/helper.c +++ b/target/microblaze/helper.c @@ -21,6 +21,7 @@ #include "qemu/osdep.h" #include "cpu.h" #include "exec/cputlb.h" +#include "exec/cpu-mmu-index.h" #include "exec/page-protection.h" #include "qemu/host-utils.h" #include "exec/log.h" diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c index f8587d5ac4..987ac9e3a7 100644 --- a/target/microblaze/mmu.c +++ b/target/microblaze/mmu.c @@ -22,6 +22,7 @@ #include "qemu/log.h" #include "cpu.h" #include "exec/cputlb.h" +#include "exec/cpu-mmu-index.h" #include "exec/page-protection.h" static unsigned int tlb_decode_size(unsigned int f) diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c index 7a6af183ae..5b437959ac 100644 --- a/target/openrisc/translate.c +++ b/target/openrisc/translate.c @@ -20,6 +20,7 @@ #include "qemu/osdep.h" #include "cpu.h" +#include "exec/cpu-mmu-index.h" #include "exec/exec-all.h" #include "tcg/tcg-op.h" #include "qemu/log.h" diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c index 5716120117..1bf00407af 100644 --- a/target/sparc/cpu.c +++ b/target/sparc/cpu.c @@ -22,6 +22,7 @@ #include "cpu.h" #include "qemu/module.h" #include "qemu/qemu-print.h" +#include "exec/cpu-mmu-index.h" #include "exec/exec-all.h" #include "exec/translation-block.h" #include "hw/qdev-properties.h" diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c index 7548d01777..4a0cedd9e2 100644 --- a/target/sparc/mmu_helper.c +++ b/target/sparc/mmu_helper.c @@ -21,6 +21,7 @@ #include "qemu/log.h" #include "cpu.h" #include "exec/cputlb.h" +#include "exec/cpu-mmu-index.h" #include "exec/page-protection.h" #include "qemu/qemu-print.h" #include "trace.h" diff --git a/target/tricore/helper.c b/target/tricore/helper.c index a64412e6bd..be3d97af78 100644 --- a/target/tricore/helper.c +++ b/target/tricore/helper.c @@ -20,6 +20,7 @@ #include "hw/registerfields.h" #include "cpu.h" #include "exec/cputlb.h" +#include "exec/cpu-mmu-index.h" #include "exec/page-protection.h" #include "fpu/softfloat-helpers.h" #include "qemu/qemu-print.h" diff --git a/target/xtensa/mmu_helper.c b/target/xtensa/mmu_helper.c index 63be741a42..96140c89c7 100644 --- a/target/xtensa/mmu_helper.c +++ b/target/xtensa/mmu_helper.c @@ -33,6 +33,7 @@ #include "exec/helper-proto.h" #include "qemu/host-utils.h" #include "exec/cputlb.h" +#include "exec/cpu-mmu-index.h" #include "exec/exec-all.h" #include "exec/page-protection.h"
The implementation of cpu_mmu_index was split between cpu-common.h and cpu-all.h, depending on CONFIG_USER_ONLY. We already have the plumbing common to user and system mode. Using MMU_USER_IDX requires the cpu.h for a specific target, and so is restricted to when we're compiling per-target. Include the new header only where needed. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/exec/cpu-all.h | 6 ------ include/exec/cpu-common.h | 20 ------------------ include/exec/cpu-mmu-index.h | 39 +++++++++++++++++++++++++++++++++++ include/exec/cpu_ldst.h | 1 + semihosting/uaccess.c | 1 + target/arm/gdbstub64.c | 3 +++ target/hppa/mem_helper.c | 1 + target/i386/tcg/translate.c | 1 + target/loongarch/cpu_helper.c | 1 + target/microblaze/helper.c | 1 + target/microblaze/mmu.c | 1 + target/openrisc/translate.c | 1 + target/sparc/cpu.c | 1 + target/sparc/mmu_helper.c | 1 + target/tricore/helper.c | 1 + target/xtensa/mmu_helper.c | 1 + 16 files changed, 54 insertions(+), 26 deletions(-) create mode 100644 include/exec/cpu-mmu-index.h