[PATCH 00/10] target/misc: Header cleanups around "cpu.h"

[PATCH 00/10] target/misc: Header cleanups around "cpu.h"

[Philippe Mathieu-Daudé]

These patches are part of a big refactor cleanup
around "cpu.h". Most changes should be trivial IMHO.

Philippe Mathieu-Daudé (10):
  target/hppa/helper: Include missing "cpu.h" header
  target/hexagon: Declare hexagon_regnames[] in "cpu.h"
  target/loongarch/cpu: Remove unused "sysbus.h" header
  target/loongarch/cpu: Restrict "memory.h" header to sysemu
  target/m68k/helper: Include missing "cpu.h" header
  target/ppc/helper: Include missing "cpu.h" header
  target/ppc/internal: Restrict MMU declarations to sysemu
  target/riscv/cpu: Restrict sysemu-specific fields from CPUArchState
  target/sparc/sysemu: Remove pointless CONFIG_USER_ONLY guard
  target/xtensa/cpu: Include missing "memory.h" header

 target/hexagon/cpu.h      |  2 ++
 target/hexagon/internal.h |  2 --
 target/hppa/helper.h      |  2 ++
 target/loongarch/cpu.h    |  3 ++-
 target/m68k/helper.h      |  2 ++
 target/ppc/helper.h       |  2 ++
 target/ppc/internal.h     |  5 +++++
 target/riscv/cpu.h        | 34 ++++++++++++++++++----------------
 target/sparc/mmu_helper.c |  2 --
 target/xtensa/cpu.c       |  3 +++
 10 files changed, 36 insertions(+), 21 deletions(-)

-- 
2.38.1

[PATCH 01/10] target/hppa/helper: Include missing "cpu.h" header

[Philippe Mathieu-Daudé]

'dh_ctype_tr' is defined as 'target_ureg', itself declared in "cpu.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/hppa/helper.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/hppa/helper.h b/target/hppa/helper.h
index c7e35ce8c7..621bc9738e 100644
--- a/target/hppa/helper.h
+++ b/target/hppa/helper.h
@@ -1,3 +1,5 @@
+#include "cpu.h"
+
 #if TARGET_REGISTER_BITS == 64
 # define dh_alias_tr     i64
 # define dh_typecode_tr  dh_typecode_i64
-- 
2.38.1

Re: [PATCH 01/10] target/hppa/helper: Include missing "cpu.h" header

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> 'dh_ctype_tr' is defined as 'target_ureg', itself declared in "cpu.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/hppa/helper.h | 2 ++
>   1 file changed, 2 insertions(+)

No, helper.h is multiply included.
It should probably be renamed across all target/*/.


r~

[PATCH 02/10] target/hexagon: Declare hexagon_regnames[] in "cpu.h"

[Philippe Mathieu-Daudé]

The hexagon_regnames array has TOTAL_PER_THREAD_REGS elements.
TOTAL_PER_THREAD_REGS is defined in "cpu.h". Instead of pulling
"cpu.h" in "internal.h", move the hexagon_regnames[] declaration
to "cpu.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/hexagon/cpu.h      | 2 ++
 target/hexagon/internal.h | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h
index 2a65a57bab..9e8cd5a257 100644
--- a/target/hexagon/cpu.h
+++ b/target/hexagon/cpu.h
@@ -178,6 +178,8 @@ typedef HexagonCPU ArchCPU;
 
 void hexagon_translate_init(void);
 
+extern const char * const hexagon_regnames[TOTAL_PER_THREAD_REGS];
+
 #include "exec/cpu-all.h"
 
 #endif /* HEXAGON_CPU_H */
diff --git a/target/hexagon/internal.h b/target/hexagon/internal.h
index b1bfadc3f5..1a6386e0ac 100644
--- a/target/hexagon/internal.h
+++ b/target/hexagon/internal.h
@@ -38,6 +38,4 @@ void hexagon_debug_vreg(CPUHexagonState *env, int regnum);
 void hexagon_debug_qreg(CPUHexagonState *env, int regnum);
 void hexagon_debug(CPUHexagonState *env);
 
-extern const char * const hexagon_regnames[TOTAL_PER_THREAD_REGS];
-
 #endif
-- 
2.38.1

RE: [PATCH 02/10] target/hexagon: Declare hexagon_regnames[] in "cpu.h"

[Taylor Simpson]

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Friday, December 16, 2022 4:06 PM
> To: qemu-devel@nongnu.org
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>; David Gibson
> <david@gibson.dropbear.id.au>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Mark Cave-Ayland <mark.cave-
> ayland@ilande.co.uk>; Bin Meng <bin.meng@windriver.com>; Richard
> Henderson <richard.henderson@linaro.org>; Artyom Tarasenko
> <atar4qemu@gmail.com>; Cédric Le Goater <clg@kaod.org>; Song Gao
> <gaosong@loongson.cn>; Alistair Francis <alistair.francis@wdc.com>; qemu-
> ppc@nongnu.org; Laurent Vivier <laurent@vivier.eu>; Palmer Dabbelt
> <palmer@dabbelt.com>; Taylor Simpson <tsimpson@quicinc.com>; Greg
> Kurz <groug@kaod.org>; qemu-riscv@nongnu.org; Max Filippov
> <jcmvbkbc@gmail.com>; Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH 02/10] target/hexagon: Declare hexagon_regnames[] in
> "cpu.h"
> 
> The hexagon_regnames array has TOTAL_PER_THREAD_REGS elements.
> TOTAL_PER_THREAD_REGS is defined in "cpu.h". Instead of pulling "cpu.h" in
> "internal.h", move the hexagon_regnames[] declaration to "cpu.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/hexagon/cpu.h      | 2 ++
>  target/hexagon/internal.h | 2 --
>  2 files changed, 2 insertions(+), 2 deletions(-)

Why is this needed?  internal.h doesn't directly include cpu.h.

If the goal is to reduce the number of places where internal.h is included, then there would be additional changes.

If the concern is the dependence on TOTAL_PER_THREAD_REGS in internal.h, just change it to
extern const char * const hexagon_regnames[];

Thanks,
Taylor

Re: [PATCH 02/10] target/hexagon: Declare hexagon_regnames[] in "cpu.h"

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> The hexagon_regnames array has TOTAL_PER_THREAD_REGS elements.
> TOTAL_PER_THREAD_REGS is defined in "cpu.h". Instead of pulling
> "cpu.h" in "internal.h", move the hexagon_regnames[] declaration
> to "cpu.h".

I don't think this is correct.
I would have expected cpu.h to already be included by all users of internal.h, thus any 
formal re-inclusion of cpu.h at the top of internal.h should be a no-op.


r~

[PATCH 03/10] target/loongarch/cpu: Remove unused "sysbus.h" header

[Philippe Mathieu-Daudé]

Nothing requires SysBus declarations here.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/loongarch/cpu.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index e15c633b0b..c8612f5466 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -13,7 +13,6 @@
 #include "hw/registerfields.h"
 #include "qemu/timer.h"
 #include "exec/memory.h"
-#include "hw/sysbus.h"
 #include "cpu-csr.h"
 
 #define IOCSRF_TEMP             0
-- 
2.38.1

Re: [PATCH 03/10] target/loongarch/cpu: Remove unused "sysbus.h" header

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> Nothing requires SysBus declarations here.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/loongarch/cpu.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 03/10] target/loongarch/cpu: Remove unused "sysbus.h" header

[Bernhard Beschow]

Am 16. Dezember 2022 22:05:32 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Nothing requires SysBus declarations here.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> target/loongarch/cpu.h | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
>index e15c633b0b..c8612f5466 100644
>--- a/target/loongarch/cpu.h
>+++ b/target/loongarch/cpu.h
>@@ -13,7 +13,6 @@
> #include "hw/registerfields.h"
> #include "qemu/timer.h"
> #include "exec/memory.h"
>-#include "hw/sysbus.h"
> #include "cpu-csr.h"
> 
> #define IOCSRF_TEMP             0

https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg02628.html

\o/

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

Re: [PATCH 03/10] target/loongarch/cpu: Remove unused "sysbus.h" header

[Philippe Mathieu-Daudé]

On 17/12/22 13:26, Bernhard Beschow wrote:
> 
> 
> Am 16. Dezember 2022 22:05:32 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> Nothing requires SysBus declarations here.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> target/loongarch/cpu.h | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
>> index e15c633b0b..c8612f5466 100644
>> --- a/target/loongarch/cpu.h
>> +++ b/target/loongarch/cpu.h
>> @@ -13,7 +13,6 @@
>> #include "hw/registerfields.h"
>> #include "qemu/timer.h"
>> #include "exec/memory.h"
>> -#include "hw/sysbus.h"
>> #include "cpu-csr.h"
>>
>> #define IOCSRF_TEMP             0
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg02628.html
> 
> \o/

=)

Please ping me on your flying series, I'm trying to catch up /o\

[PATCH 04/10] target/loongarch/cpu: Restrict "memory.h" header to sysemu

[Philippe Mathieu-Daudé]

Missed in 0093b9a5ee ("target/loongarch: Adjust functions
and structure to support user-mode") while cleaning commit
f84a2aacf5 ("target/loongarch: Add LoongArch IOCSR instruction").

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/loongarch/cpu.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index c8612f5466..2f17ac6b47 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -12,7 +12,9 @@
 #include "fpu/softfloat-types.h"
 #include "hw/registerfields.h"
 #include "qemu/timer.h"
+#ifndef CONFIG_USER_ONLY
 #include "exec/memory.h"
+#endif
 #include "cpu-csr.h"
 
 #define IOCSRF_TEMP             0
-- 
2.38.1

Re: [PATCH 04/10] target/loongarch/cpu: Restrict "memory.h" header to sysemu

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> Missed in 0093b9a5ee ("target/loongarch: Adjust functions
> and structure to support user-mode") while cleaning commit
> f84a2aacf5 ("target/loongarch: Add LoongArch IOCSR instruction").
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/loongarch/cpu.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
> index c8612f5466..2f17ac6b47 100644
> --- a/target/loongarch/cpu.h
> +++ b/target/loongarch/cpu.h
> @@ -12,7 +12,9 @@
>   #include "fpu/softfloat-types.h"
>   #include "hw/registerfields.h"
>   #include "qemu/timer.h"
> +#ifndef CONFIG_USER_ONLY
>   #include "exec/memory.h"
> +#endif
>   #include "cpu-csr.h"
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 05/10] target/m68k/helper: Include missing "cpu.h" header

[Philippe Mathieu-Daudé]

'dh_ctype_fp' is defined as 'FPReg *', itself declared in "cpu.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/m68k/helper.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index c9bed2b884..aaf0e1668e 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -1,3 +1,5 @@
+#include "cpu.h"
+
 DEF_HELPER_1(bitrev, i32, i32)
 DEF_HELPER_1(ff1, i32, i32)
 DEF_HELPER_FLAGS_2(sats, TCG_CALL_NO_RWG_SE, i32, i32, i32)
-- 
2.38.1

Re: [PATCH 05/10] target/m68k/helper: Include missing "cpu.h" header

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> 'dh_ctype_fp' is defined as 'FPReg *', itself declared in "cpu.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/m68k/helper.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/m68k/helper.h b/target/m68k/helper.h
> index c9bed2b884..aaf0e1668e 100644
> --- a/target/m68k/helper.h
> +++ b/target/m68k/helper.h
> @@ -1,3 +1,5 @@
> +#include "cpu.h"
> +
>   DEF_HELPER_1(bitrev, i32, i32)
>   DEF_HELPER_1(ff1, i32, i32)
>   DEF_HELPER_FLAGS_2(sats, TCG_CALL_NO_RWG_SE, i32, i32, i32)

No, see patch 1.

r~

[PATCH 06/10] target/ppc/helper: Include missing "cpu.h" header

[Philippe Mathieu-Daudé]

'dh_ctype_*' are defined as 'ppc_avr_t/ppc_vsr_t/ppc_acc_t',
themselves declared in "cpu.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/helper.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 8dd22a35e4..a02db8d893 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -1,3 +1,5 @@
+#include "cpu.h"
+
 DEF_HELPER_FLAGS_3(raise_exception_err, TCG_CALL_NO_WG, noreturn, env, i32, i32)
 DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, i32)
 DEF_HELPER_FLAGS_4(tw, TCG_CALL_NO_WG, void, env, tl, tl, i32)
-- 
2.38.1

Re: [PATCH 06/10] target/ppc/helper: Include missing "cpu.h" header

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> 'dh_ctype_*' are defined as 'ppc_avr_t/ppc_vsr_t/ppc_acc_t',
> themselves declared in "cpu.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/ppc/helper.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 8dd22a35e4..a02db8d893 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -1,3 +1,5 @@
> +#include "cpu.h"

No, see patch 1.


r~

[PATCH 07/10] target/ppc/internal: Restrict MMU declarations to sysemu

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/internal.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 337a362205..901bae6d39 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -242,9 +242,12 @@ static inline int prot_for_access_type(MMUAccessType access_type)
     g_assert_not_reached();
 }
 
+#ifndef CONFIG_USER_ONLY
+
 /* PowerPC MMU emulation */
 
 typedef struct mmu_ctx_t mmu_ctx_t;
+
 bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
                       hwaddr *raddrp, int *psizep, int *protp,
                       int mmu_idx, bool guest_visible);
@@ -266,6 +269,8 @@ struct mmu_ctx_t {
     int nx;                        /* Non-execute area          */
 };
 
+#endif /* !CONFIG_USER_ONLY */
+
 /* Common routines used by software and hardware TLBs emulation */
 static inline int pte_is_valid(target_ulong pte0)
 {
-- 
2.38.1

Re: [PATCH 07/10] target/ppc/internal: Restrict MMU declarations to sysemu

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/ppc/internal.h | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index 337a362205..901bae6d39 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -242,9 +242,12 @@ static inline int prot_for_access_type(MMUAccessType access_type)
>       g_assert_not_reached();
>   }
>   
> +#ifndef CONFIG_USER_ONLY
> +
>   /* PowerPC MMU emulation */
>   
>   typedef struct mmu_ctx_t mmu_ctx_t;
> +
>   bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>                         hwaddr *raddrp, int *psizep, int *protp,
>                         int mmu_idx, bool guest_visible);
> @@ -266,6 +269,8 @@ struct mmu_ctx_t {
>       int nx;                        /* Non-execute area          */
>   };
>   
> +#endif /* !CONFIG_USER_ONLY */

Eh.  I suppose.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 07/10] target/ppc/internal: Restrict MMU declarations to sysemu

[Philippe Mathieu-Daudé]

On 17/12/22 01:38, Richard Henderson wrote:
> On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/ppc/internal.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
>> index 337a362205..901bae6d39 100644
>> --- a/target/ppc/internal.h
>> +++ b/target/ppc/internal.h
>> @@ -242,9 +242,12 @@ static inline int 
>> prot_for_access_type(MMUAccessType access_type)
>>       g_assert_not_reached();
>>   }
>> +#ifndef CONFIG_USER_ONLY
>> +
>>   /* PowerPC MMU emulation */
>>   typedef struct mmu_ctx_t mmu_ctx_t;
>> +
>>   bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>>                         hwaddr *raddrp, int *psizep, int *protp,
>>                         int mmu_idx, bool guest_visible);
>> @@ -266,6 +269,8 @@ struct mmu_ctx_t {
>>       int nx;                        /* Non-execute area          */
>>   };
>> +#endif /* !CONFIG_USER_ONLY */
> 
> Eh.  I suppose.

I forgot the justification:

'''
The 'hwaddr' type is only available / meaningful on system emulation.
'''

> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!

[PATCH 08/10] target/riscv/cpu: Restrict sysemu-specific fields from CPUArchState

[Philippe Mathieu-Daudé]

The 'hwaddr' type is only available / meaningful on system emulation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/cpu.h | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index fc1f72e5c3..bd4e16d946 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -368,16 +368,6 @@ struct CPUArchState {
     uint64_t menvcfg;
     target_ulong senvcfg;
     uint64_t henvcfg;
-#endif
-    target_ulong cur_pmmask;
-    target_ulong cur_pmbase;
-
-    float_status fp_status;
-
-    /* Fields from here on are preserved across CPU reset. */
-    QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
-    QEMUTimer *vstimer; /* Internal timer for VS-mode interrupt */
-    bool vstime_irq;
 
     hwaddr kernel_addr;
     hwaddr fdt_addr;
@@ -388,6 +378,16 @@ struct CPUArchState {
     uint64_t kvm_timer_compare;
     uint64_t kvm_timer_state;
     uint64_t kvm_timer_frequency;
+#endif
+    target_ulong cur_pmmask;
+    target_ulong cur_pmbase;
+
+    float_status fp_status;
+
+    /* Fields from here on are preserved across CPU reset. */
+    QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
+    QEMUTimer *vstimer; /* Internal timer for VS-mode interrupt */
+    bool vstime_irq;
 };
 
 OBJECT_DECLARE_CPU_TYPE(RISCVCPU, RISCVCPUClass, RISCV_CPU)
@@ -553,12 +553,20 @@ bool riscv_cpu_virt_enabled(CPURISCVState *env);
 void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
 bool riscv_cpu_two_stage_lookup(int mmu_idx);
 int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
+#ifndef CONFIG_USER_ONLY
+hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 G_NORETURN void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                                MMUAccessType access_type, int mmu_idx,
                                                uintptr_t retaddr);
 bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                         MMUAccessType access_type, int mmu_idx,
                         bool probe, uintptr_t retaddr);
+void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
+                                     vaddr addr, unsigned size,
+                                     MMUAccessType access_type,
+                                     int mmu_idx, MemTxAttrs attrs,
+                                     MemTxResult response, uintptr_t retaddr);
+#endif
 char *riscv_isa_string(RISCVCPU *cpu);
 void riscv_cpu_list(void);
 
@@ -566,12 +574,6 @@ void riscv_cpu_list(void);
 #define cpu_mmu_index riscv_cpu_mmu_index
 
 #ifndef CONFIG_USER_ONLY
-void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
-                                     vaddr addr, unsigned size,
-                                     MMUAccessType access_type,
-                                     int mmu_idx, MemTxAttrs attrs,
-                                     MemTxResult response, uintptr_t retaddr);
-hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
 void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env);
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts);
-- 
2.38.1

Re: [PATCH 08/10] target/riscv/cpu: Restrict sysemu-specific fields from CPUArchState

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> The 'hwaddr' type is only available / meaningful on system emulation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/riscv/cpu.h | 34 ++++++++++++++++++----------------
>   1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index fc1f72e5c3..bd4e16d946 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -368,16 +368,6 @@ struct CPUArchState {
>       uint64_t menvcfg;
>       target_ulong senvcfg;
>       uint64_t henvcfg;
> -#endif
> -    target_ulong cur_pmmask;
> -    target_ulong cur_pmbase;
> -
> -    float_status fp_status;
> -
> -    /* Fields from here on are preserved across CPU reset. */
> -    QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
> -    QEMUTimer *vstimer; /* Internal timer for VS-mode interrupt */
> -    bool vstime_irq;
>   
>       hwaddr kernel_addr;
>       hwaddr fdt_addr;
> @@ -388,6 +378,16 @@ struct CPUArchState {
>       uint64_t kvm_timer_compare;
>       uint64_t kvm_timer_state;
>       uint64_t kvm_timer_frequency;
> +#endif
> +    target_ulong cur_pmmask;
> +    target_ulong cur_pmbase;
> +
> +    float_status fp_status;
> +
> +    /* Fields from here on are preserved across CPU reset. */
> +    QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
> +    QEMUTimer *vstimer; /* Internal timer for VS-mode interrupt */
> +    bool vstime_irq;
>   };

Except for fp_status, all of the other fields that you move are sysemu specific.


r~

[PATCH 09/10] target/sparc/sysemu: Remove pointless CONFIG_USER_ONLY guard

[Philippe Mathieu-Daudé]

Commit caac44a52a ("target/sparc: Make sparc_cpu_tlb_fill sysemu
only") restricted mmu_helper.c to system emulation. Checking
whether CONFIG_USER_ONLY is defined is now pointless.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/mmu_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
index 919448a494..a7e51e4b7d 100644
--- a/target/sparc/mmu_helper.c
+++ b/target/sparc/mmu_helper.c
@@ -924,7 +924,6 @@ hwaddr sparc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
     return phys_addr;
 }
 
-#ifndef CONFIG_USER_ONLY
 G_NORETURN void sparc_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                               MMUAccessType access_type,
                                               int mmu_idx,
@@ -942,4 +941,3 @@ G_NORETURN void sparc_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
 
     cpu_raise_exception_ra(env, TT_UNALIGNED, retaddr);
 }
-#endif /* !CONFIG_USER_ONLY */
-- 
2.38.1

[PATCH 10/10] target/xtensa/cpu: Include missing "memory.h" header

[Philippe Mathieu-Daudé]

Under system emulation, xtensa_cpu_initfn() calls
memory_region_init_io(), itself declared in "exec/memory.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/xtensa/cpu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 09923301c4..879710f8d1 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -35,6 +35,9 @@
 #include "qemu/module.h"
 #include "migration/vmstate.h"
 #include "hw/qdev-clock.h"
+#ifndef CONFIG_USER_ONLY
+#include "exec/memory.h"
+#endif
 
 
 static void xtensa_cpu_set_pc(CPUState *cs, vaddr value)
-- 
2.38.1

Re: [PATCH 10/10] target/xtensa/cpu: Include missing "memory.h" header

[Richard Henderson]

On 12/16/22 14:05, Philippe Mathieu-Daudé wrote:
> Under system emulation, xtensa_cpu_initfn() calls
> memory_region_init_io(), itself declared in "exec/memory.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/xtensa/cpu.c | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~