| Message ID | 20240129233043.34558-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/core: Introduce CPUClass hook for mmu_index | expand |
Hi Richard, On 30/1/24 00:30, Richard Henderson wrote: > To be used after all targets have populated the hook. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/hw/core/cpu.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h > index 2c284d6397..4385ce54c9 100644 > --- a/include/hw/core/cpu.h > +++ b/include/hw/core/cpu.h > @@ -103,6 +103,8 @@ struct SysemuCPUOps; > * @parse_features: Callback to parse command line arguments. > * @reset_dump_flags: #CPUDumpFlags to use for reset logging. > * @has_work: Callback for checking if there is work to do. > + * @mmu_index: Callback for choosing softmmu mmu index; > + * may be used internally by memory_rw_debug without TCG. > * @memory_rw_debug: Callback for GDB memory access. > * @dump_state: Callback for dumping state. > * @query_cpu_fast: > @@ -150,6 +152,7 @@ struct CPUClass { > void (*parse_features)(const char *typename, char *str, Error **errp); > > bool (*has_work)(CPUState *cpu); > + int (*mmu_index)(CPUState *cpu, bool ifetch); Can the index ever be negative? Would it be useful to also have a mmu_index_max() so we could check mmu_index() is in range in the final cpu_mmu_index()? > int (*memory_rw_debug)(CPUState *cpu, vaddr addr, > uint8_t *buf, int len, bool is_write); > void (*dump_state)(CPUState *cpu, FILE *, int flags);
On 1/30/24 17:46, Philippe Mathieu-Daudé wrote: > Hi Richard, > > On 30/1/24 00:30, Richard Henderson wrote: >> To be used after all targets have populated the hook. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> include/hw/core/cpu.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h >> index 2c284d6397..4385ce54c9 100644 >> --- a/include/hw/core/cpu.h >> +++ b/include/hw/core/cpu.h >> @@ -103,6 +103,8 @@ struct SysemuCPUOps; >> * @parse_features: Callback to parse command line arguments. >> * @reset_dump_flags: #CPUDumpFlags to use for reset logging. >> * @has_work: Callback for checking if there is work to do. >> + * @mmu_index: Callback for choosing softmmu mmu index; >> + * may be used internally by memory_rw_debug without TCG. >> * @memory_rw_debug: Callback for GDB memory access. >> * @dump_state: Callback for dumping state. >> * @query_cpu_fast: >> @@ -150,6 +152,7 @@ struct CPUClass { >> void (*parse_features)(const char *typename, char *str, Error **errp); >> bool (*has_work)(CPUState *cpu); >> + int (*mmu_index)(CPUState *cpu, bool ifetch); > > Can the index ever be negative? No. > Would it be useful to also have a mmu_index_max() so we could > check mmu_index() is in range in the final cpu_mmu_index()? We have NB_MMU_MODES, a target-independent constant. r~
On 30/1/24 08:51, Richard Henderson wrote: > On 1/30/24 17:46, Philippe Mathieu-Daudé wrote: >> Hi Richard, >> >> On 30/1/24 00:30, Richard Henderson wrote: >>> To be used after all targets have populated the hook. >>> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> include/hw/core/cpu.h | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h >>> index 2c284d6397..4385ce54c9 100644 >>> --- a/include/hw/core/cpu.h >>> +++ b/include/hw/core/cpu.h >>> @@ -103,6 +103,8 @@ struct SysemuCPUOps; >>> * @parse_features: Callback to parse command line arguments. >>> * @reset_dump_flags: #CPUDumpFlags to use for reset logging. >>> * @has_work: Callback for checking if there is work to do. >>> + * @mmu_index: Callback for choosing softmmu mmu index; >>> + * may be used internally by memory_rw_debug without TCG. >>> * @memory_rw_debug: Callback for GDB memory access. >>> * @dump_state: Callback for dumping state. >>> * @query_cpu_fast: >>> @@ -150,6 +152,7 @@ struct CPUClass { >>> void (*parse_features)(const char *typename, char *str, Error >>> **errp); >>> bool (*has_work)(CPUState *cpu); >>> + int (*mmu_index)(CPUState *cpu, bool ifetch); >> >> Can the index ever be negative? > > No. Then why not return an unsigned type? >> Would it be useful to also have a mmu_index_max() so we could >> check mmu_index() is in range in the final cpu_mmu_index()? > > We have NB_MMU_MODES, a target-independent constant. Ah right.
On 1/30/24 18:20, Philippe Mathieu-Daudé wrote: >>> Can the index ever be negative? >> >> No. > > Then why not return an unsigned type? The existing function didn't, and I didn't think about it. Does it really matter? r~
On 30/1/24 12:02, Richard Henderson wrote: > On 1/30/24 18:20, Philippe Mathieu-Daudé wrote: >>>> Can the index ever be negative? >>> >>> No. >> >> Then why not return an unsigned type? > > The existing function didn't, and I didn't think about it. > Does it really matter? No :)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 2c284d6397..4385ce54c9 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -103,6 +103,8 @@ struct SysemuCPUOps; * @parse_features: Callback to parse command line arguments. * @reset_dump_flags: #CPUDumpFlags to use for reset logging. * @has_work: Callback for checking if there is work to do. + * @mmu_index: Callback for choosing softmmu mmu index; + * may be used internally by memory_rw_debug without TCG. * @memory_rw_debug: Callback for GDB memory access. * @dump_state: Callback for dumping state. * @query_cpu_fast: @@ -150,6 +152,7 @@ struct CPUClass { void (*parse_features)(const char *typename, char *str, Error **errp); bool (*has_work)(CPUState *cpu); + int (*mmu_index)(CPUState *cpu, bool ifetch); int (*memory_rw_debug)(CPUState *cpu, vaddr addr, uint8_t *buf, int len, bool is_write); void (*dump_state)(CPUState *cpu, FILE *, int flags);
To be used after all targets have populated the hook. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/hw/core/cpu.h | 3 +++ 1 file changed, 3 insertions(+)