[PATCH-for-10.1 0/4] target/sparc: Spring cleanups around CPU features & LEON3

[PATCH-for-10.1 0/4] target/sparc: Spring cleanups around CPU features & LEON3

[Philippe Mathieu-Daudé]

Some LEON3 specific flags seems to be disableable, so list them.

Move HW specific code from CPU ENV one layer upper.

Philippe Mathieu-Daudé (4):
  target/sparc: Expose more CPU features
  target/sparc: Restrict SPARC64 specific features
  target/sparc: Log unimplemented ASI load/store accesses
  target/sparc: Move hardware fields from CPUSPARCState to SPARCCPU

 target/sparc/cpu.h             | 10 +++++-----
 target/sparc/cpu-feature.h.inc | 20 +++++++++++--------
 hw/sparc/leon3.c               | 35 +++++++++++++++++-----------------
 target/sparc/cpu.c             |  6 +++++-
 target/sparc/int32_helper.c    |  8 ++++++--
 target/sparc/ldst_helper.c     | 20 +++++++++++++------
 target/sparc/translate.c       | 10 ++++++++--
 7 files changed, 68 insertions(+), 41 deletions(-)

-- 
2.47.1

[PATCH-for-10.1 1/4] target/sparc: Expose more CPU features

[Philippe Mathieu-Daudé]

Various features were not displayed or settable from command line.

Diff of 'qemu-system-sparc -cpu help':

 ...
   TI-SuperSparc-60     (IU 40000000 FPU 00000000 MMU 01000800 NWINS 8)
   TI-SuperSparc-61     (IU 44000000 FPU 00000000 MMU 01000000 NWINS 8)
   TI-SuperSparc-II     (IU 40000000 FPU 00000000 MMU 08000000 NWINS 8)
 - LEON2                (IU f2000000 FPU 00080000 MMU f2000000 NWINS 8)
 - LEON3                (IU f3000000 FPU 00080000 MMU f3000000 NWINS 8)
 + LEON2                (IU f2000000 FPU 00080000 MMU f2000000 NWINS 8) +shutdown
 + LEON3                (IU f3000000 FPU 00080000 MMU f3000000 NWINS 8) +shutdown +asr17 +cachectrl +powerdown +casa
  Default CPU feature flags (use '-' to remove): mul div fsmuld
 -Available CPU feature flags (use '+' to add): float128
 +Available CPU feature flags (use '+' to add): shutdown asr17 cachectrl powerdown casa float128
  Numerical features (use '=' to set): iu_version fpu_version mmu_version nwindows

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

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 57161201173..37406227cb7 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -548,6 +548,11 @@ static const sparc_def_t sparc_defs[] = {
 /* This must match sparc_cpu_properties[]. */
 static const char * const feature_name[] = {
     [CPU_FEATURE_BIT_FLOAT128] = "float128",
+    [CPU_FEATURE_BIT_TA0_SHUTDOWN] = "shutdown",
+    [CPU_FEATURE_BIT_POWERDOWN] = "powerdown",
+    [CPU_FEATURE_BIT_CACHE_CTRL] = "cachectrl",
+    [CPU_FEATURE_BIT_ASR17] = "asr17",
+    [CPU_FEATURE_BIT_CASA] = "casa",
 #ifdef TARGET_SPARC64
     [CPU_FEATURE_BIT_CMT] = "cmt",
     [CPU_FEATURE_BIT_GL] = "gl",
-- 
2.47.1

[PATCH-for-10.1 2/4] target/sparc: Restrict SPARC64 specific features

[Philippe Mathieu-Daudé]

Following commit 554abe47c7b ("target/sparc: Partition cpu
features"), avoid compiling SPARC64 specific code on 32-bit
binary.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/cpu-feature.h.inc | 20 ++++++++++++--------
 target/sparc/translate.c       | 10 ++++++++--
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/target/sparc/cpu-feature.h.inc b/target/sparc/cpu-feature.h.inc
index be810052376..7b7b94a0562 100644
--- a/target/sparc/cpu-feature.h.inc
+++ b/target/sparc/cpu-feature.h.inc
@@ -1,12 +1,8 @@
 FEATURE(FLOAT128)
-FEATURE(MUL)
-FEATURE(DIV)
-FEATURE(VIS1)
-FEATURE(VIS2)
-FEATURE(FSMULD)
-FEATURE(HYPV)
-FEATURE(CMT)
-FEATURE(GL)
+FEATURE(MUL)            /* Mandatory in v9 */
+FEATURE(DIV)            /* Mandatory in v9 */
+FEATURE(FSMULD)         /* Mandatory in v9 */
+
 FEATURE(TA0_SHUTDOWN) /* Shutdown on "ta 0x0" */
 FEATURE(ASR17)
 FEATURE(CACHE_CTRL)
@@ -16,3 +12,11 @@ FEATURE(FMAF)
 FEATURE(VIS3)
 FEATURE(IMA)
 FEATURE(VIS4)
+
+#ifdef TARGET_SPARC64
+FEATURE(HYPV)
+FEATURE(CMT)
+FEATURE(GL)
+FEATURE(VIS1)
+FEATURE(VIS2)
+#endif
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index bfe63649db2..53b145848b9 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -1850,10 +1850,12 @@ static void gen_st_asi(DisasContext *dc, DisasASI *da, TCGv src, TCGv addr)
         if (TARGET_LONG_BITS == 32) {
             gen_exception(dc, TT_ILL_INSN);
             break;
+#ifdef TARGET_SPARC64
         } else if (!(dc->def->features & CPU_FEATURE_HYPV)) {
             /* Pre OpenSPARC CPUs don't have these */
             gen_exception(dc, TT_ILL_INSN);
             break;
+#endif
         }
         /* In OpenSPARC T1+ CPUs TWINX ASIs in store are ST_BLKINIT_ ASIs */
         /* fall through */
@@ -2750,12 +2752,16 @@ static bool trans_SETHI(DisasContext *dc, arg_SETHI *a)
 static bool do_tcc(DisasContext *dc, int cond, int cc,
                    int rs1, bool imm, int rs2_or_imm)
 {
-    int mask = ((dc->def->features & CPU_FEATURE_HYPV) && supervisor(dc)
-                ? UA2005_HTRAP_MASK : V8_TRAP_MASK);
+    int mask = 0;
     DisasCompare cmp;
     TCGLabel *lab;
     TCGv_i32 trap;
 
+#ifdef TARGET_SPARC64
+    mask = ((dc->def->features & CPU_FEATURE_HYPV) && supervisor(dc))
+           ? UA2005_HTRAP_MASK : V8_TRAP_MASK;
+#endif
+
     /* Trap never.  */
     if (cond == 0) {
         return advance_pc(dc);
-- 
2.47.1

[PATCH-for-10.1 3/4] target/sparc: Log unimplemented ASI load/store accesses

[Philippe Mathieu-Daudé]

When the cache-controller feature is not implemented,
log potential ASI access as unimplemented.

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

diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index b559afc9a94..d4de32d3c48 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -600,6 +600,10 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
         case 0x0C:          /* Leon3 Date Cache config */
             if (env->def.features & CPU_FEATURE_CACHE_CTRL) {
                 ret = leon3_cache_control_ld(env, addr, size);
+            } else {
+                qemu_log_mask(LOG_UNIMP,
+                              "%08x: unimplemented access size: %d\n", addr,
+                              size);
             }
             break;
         case 0x01c00a00: /* MXCC control register */
@@ -816,6 +820,10 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
         case 0x0C:          /* Leon3 Date Cache config */
             if (env->def.features & CPU_FEATURE_CACHE_CTRL) {
                 leon3_cache_control_st(env, addr, val, size);
+            } else {
+                qemu_log_mask(LOG_UNIMP,
+                              "%08x: unimplemented access size: %d\n", addr,
+                              size);
             }
             break;
 
-- 
2.47.1

[PATCH-for-10.1 4/4] target/sparc: Move hardware fields from CPUSPARCState to SPARCCPU

[Philippe Mathieu-Daudé]

Keep CPUSPARCState for architectural fields, move Leon3
hardware specific fields to SPARCCPU.

Reset the Leon3 specific 'cache_control' field in
leon3_cpu_reset() instead of sparc_cpu_reset_hold().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/cpu.h          | 10 +++++-----
 hw/sparc/leon3.c            | 35 ++++++++++++++++++-----------------
 target/sparc/cpu.c          |  1 -
 target/sparc/int32_helper.c |  8 ++++++--
 target/sparc/ldst_helper.c  | 12 ++++++------
 5 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index 462bcb6c0e6..abb71c314dc 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -543,11 +543,6 @@ struct CPUArchState {
 #define SOFTINT_REG_MASK (SOFTINT_STIMER|SOFTINT_INTRMASK|SOFTINT_TIMER)
 #endif
     sparc_def_t def;
-
-    /* Leon3 */
-    DeviceState *irq_manager;
-    void (*qemu_irq_ack)(CPUSPARCState *env, int intno);
-    uint32_t cache_control;
 };
 
 /**
@@ -560,6 +555,11 @@ struct ArchCPU {
     CPUState parent_obj;
 
     CPUSPARCState env;
+
+    /* Leon3 */
+    DeviceState *irq_manager;
+    void (*qemu_irq_ack)(SPARCCPU *cpu, int intno);
+    uint32_t cache_control;
 };
 
 /**
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 0aeaad3becc..06966861744 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -152,6 +152,7 @@ static void leon3_cpu_reset(void *opaque)
     int id = info->id;
     ResetData *s = container_of(info, ResetData, info[id]);
     CPUState *cpu = CPU(s->info[id].cpu);
+    SPARCCPU *scpu = SPARC_CPU(cpu);
     CPUSPARCState *env = cpu_env(cpu);
 
     cpu_reset(cpu);
@@ -159,41 +160,41 @@ static void leon3_cpu_reset(void *opaque)
     cpu->halted = cpu->cpu_index != 0;
     env->pc = s->entry;
     env->npc = s->entry + 4;
+    scpu->cache_control = 0;
 }
 
-static void leon3_cache_control_int(CPUSPARCState *env)
+static void leon3_cache_control_int(SPARCCPU *cpu)
 {
     uint32_t state = 0;
 
-    if (env->cache_control & CACHE_CTRL_IF) {
+    if (cpu->cache_control & CACHE_CTRL_IF) {
         /* Instruction cache state */
-        state = env->cache_control & CACHE_STATE_MASK;
+        state = cpu->cache_control & CACHE_STATE_MASK;
         if (state == CACHE_ENABLED) {
             state = CACHE_FROZEN;
             trace_int_helper_icache_freeze();
         }
 
-        env->cache_control &= ~CACHE_STATE_MASK;
-        env->cache_control |= state;
+        cpu->cache_control &= ~CACHE_STATE_MASK;
+        cpu->cache_control |= state;
     }
 
-    if (env->cache_control & CACHE_CTRL_DF) {
+    if (cpu->cache_control & CACHE_CTRL_DF) {
         /* Data cache state */
-        state = (env->cache_control >> 2) & CACHE_STATE_MASK;
+        state = (cpu->cache_control >> 2) & CACHE_STATE_MASK;
         if (state == CACHE_ENABLED) {
             state = CACHE_FROZEN;
             trace_int_helper_dcache_freeze();
         }
 
-        env->cache_control &= ~(CACHE_STATE_MASK << 2);
-        env->cache_control |= (state << 2);
+        cpu->cache_control &= ~(CACHE_STATE_MASK << 2);
+        cpu->cache_control |= (state << 2);
     }
 }
 
-static void leon3_irq_ack(CPUSPARCState *env, int intno)
+static void leon3_irq_ack(SPARCCPU *cpu, int intno)
 {
-    CPUState *cpu = CPU(env_cpu(env));
-    grlib_irqmp_ack(env->irq_manager, cpu->cpu_index, intno);
+    grlib_irqmp_ack(cpu->irq_manager, CPU(cpu)->cpu_index, intno);
 }
 
 /*
@@ -248,10 +249,10 @@ static void leon3_start_cpu(void *opaque, int n, int level)
     async_run_on_cpu(cs, leon3_start_cpu_async_work, RUN_ON_CPU_NULL);
 }
 
-static void leon3_irq_manager(CPUSPARCState *env, int intno)
+static void leon3_irq_manager(SPARCCPU *cpu, int intno)
 {
-    leon3_irq_ack(env, intno);
-    leon3_cache_control_int(env);
+    leon3_irq_ack(cpu, intno);
+    leon3_cache_control_int(cpu);
 }
 
 static void leon3_generic_hw_init(MachineState *machine)
@@ -320,8 +321,8 @@ static void leon3_generic_hw_init(MachineState *machine)
         qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", i,
                                     qdev_get_gpio_in_named(DEVICE(cpu),
                                                            "pil", 0));
-        env->irq_manager = irqmpdev;
-        env->qemu_irq_ack = leon3_irq_manager;
+        cpu->irq_manager = irqmpdev;
+        cpu->qemu_irq_ack = leon3_irq_manager;
     }
 
     sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 37406227cb7..d62ad6c4db2 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -78,7 +78,6 @@ static void sparc_cpu_reset_hold(Object *obj, ResetType type)
     env->pc = 0;
     env->npc = env->pc + 4;
 #endif
-    env->cache_control = 0;
     cpu_put_fsr(env, 0);
 }
 
diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
index f0266061023..a902702559d 100644
--- a/target/sparc/int32_helper.c
+++ b/target/sparc/int32_helper.c
@@ -168,8 +168,12 @@ void sparc_cpu_do_interrupt(CPUState *cs)
 
 #if !defined(CONFIG_USER_ONLY)
     /* IRQ acknowledgment */
-    if ((intno & ~15) == TT_EXTINT && env->qemu_irq_ack != NULL) {
-        env->qemu_irq_ack(env, intno);
+    if ((intno & ~15) == TT_EXTINT) {
+        SPARCCPU *cpu = env_archcpu(env);
+
+        if (cpu->qemu_irq_ack != NULL) {
+            cpu->qemu_irq_ack(cpu, intno);
+        }
     }
 #endif
 }
diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index d4de32d3c48..0a11360ccaf 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -513,7 +513,7 @@ static void sparc_raise_mmu_fault(CPUState *cs, hwaddr addr,
 
 /* Leon3 cache control */
 
-static void leon3_cache_control_st(CPUSPARCState *env, target_ulong addr,
+static void leon3_cache_control_st(SPARCCPU *cpu, target_ulong addr,
                                    uint64_t val, int size)
 {
     DPRINTF_CACHE_CONTROL("st addr:%08x, val:%" PRIx64 ", size:%d\n",
@@ -534,7 +534,7 @@ static void leon3_cache_control_st(CPUSPARCState *env, target_ulong addr,
         val &= ~CACHE_CTRL_IP;
         val &= ~CACHE_CTRL_DP;
 
-        env->cache_control = val;
+        cpu->cache_control = val;
         break;
     case 0x04:              /* Instruction cache configuration */
     case 0x08:              /* Data cache configuration */
@@ -546,7 +546,7 @@ static void leon3_cache_control_st(CPUSPARCState *env, target_ulong addr,
     };
 }
 
-static uint64_t leon3_cache_control_ld(CPUSPARCState *env, target_ulong addr,
+static uint64_t leon3_cache_control_ld(SPARCCPU *cpu, target_ulong addr,
                                        int size)
 {
     uint64_t ret = 0;
@@ -558,7 +558,7 @@ static uint64_t leon3_cache_control_ld(CPUSPARCState *env, target_ulong addr,
 
     switch (addr) {
     case 0x00:              /* Cache control */
-        ret = env->cache_control;
+        ret = cpu->cache_control;
         break;
 
         /* Configuration registers are read and only always keep those
@@ -599,7 +599,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
         case 0x08:          /* Leon3 Instruction Cache config */
         case 0x0C:          /* Leon3 Date Cache config */
             if (env->def.features & CPU_FEATURE_CACHE_CTRL) {
-                ret = leon3_cache_control_ld(env, addr, size);
+                ret = leon3_cache_control_ld(env_archcpu(env), addr, size);
             } else {
                 qemu_log_mask(LOG_UNIMP,
                               "%08x: unimplemented access size: %d\n", addr,
@@ -819,7 +819,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
         case 0x08:          /* Leon3 Instruction Cache config */
         case 0x0C:          /* Leon3 Date Cache config */
             if (env->def.features & CPU_FEATURE_CACHE_CTRL) {
-                leon3_cache_control_st(env, addr, val, size);
+                leon3_cache_control_st(env_archcpu(env), addr, val, size);
             } else {
                 qemu_log_mask(LOG_UNIMP,
                               "%08x: unimplemented access size: %d\n", addr,
-- 
2.47.1

Re: [PATCH-for-10.1 2/4] target/sparc: Restrict SPARC64 specific features

[Richard Henderson]

On 3/25/25 05:39, Philippe Mathieu-Daudé wrote:
> @@ -2750,12 +2752,16 @@ static bool trans_SETHI(DisasContext *dc, arg_SETHI *a)
>   static bool do_tcc(DisasContext *dc, int cond, int cc,
>                      int rs1, bool imm, int rs2_or_imm)
>   {
> -    int mask = ((dc->def->features & CPU_FEATURE_HYPV) && supervisor(dc)
> -                ? UA2005_HTRAP_MASK : V8_TRAP_MASK);
> +    int mask = 0;
>       DisasCompare cmp;
>       TCGLabel *lab;
>       TCGv_i32 trap;
>   
> +#ifdef TARGET_SPARC64
> +    mask = ((dc->def->features & CPU_FEATURE_HYPV) && supervisor(dc))
> +           ? UA2005_HTRAP_MASK : V8_TRAP_MASK;
> +#endif

This is wrong.  The conversion could have been

   int mask = V8_TRAP_MASK;
#ifdef
   if (HYPV && super) {
     mask = UA2005_HTRAP_MASK;
   }
#endif

but that's an excellent reason not to have the ifdef at all.

If you want to hide the features from sparc32, so they don't show up on the command-line, 
fine.  But I think you want to introduce

#define CPU_FEATURE_HYPV 0

etc for sparc32 to automatically fail such tests as these without further modification.


r~

Re: [PATCH-for-10.1 1/4] target/sparc: Expose more CPU features

[Richard Henderson]

On 3/25/25 05:39, Philippe Mathieu-Daudé wrote:
> Various features were not displayed or settable from command line.
> 
> Diff of 'qemu-system-sparc -cpu help':
> 
>   ...
>     TI-SuperSparc-60     (IU 40000000 FPU 00000000 MMU 01000800 NWINS 8)
>     TI-SuperSparc-61     (IU 44000000 FPU 00000000 MMU 01000000 NWINS 8)
>     TI-SuperSparc-II     (IU 40000000 FPU 00000000 MMU 08000000 NWINS 8)
>   - LEON2                (IU f2000000 FPU 00080000 MMU f2000000 NWINS 8)
>   - LEON3                (IU f3000000 FPU 00080000 MMU f3000000 NWINS 8)
>   + LEON2                (IU f2000000 FPU 00080000 MMU f2000000 NWINS 8) +shutdown
>   + LEON3                (IU f3000000 FPU 00080000 MMU f3000000 NWINS 8) +shutdown +asr17 +cachectrl +powerdown +casa
>    Default CPU feature flags (use '-' to remove): mul div fsmuld
>   -Available CPU feature flags (use '+' to add): float128
>   +Available CPU feature flags (use '+' to add): shutdown asr17 cachectrl powerdown casa float128
>    Numerical features (use '=' to set): iu_version fpu_version mmu_version nwindows
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/sparc/cpu.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 57161201173..37406227cb7 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -548,6 +548,11 @@ static const sparc_def_t sparc_defs[] = {
>   /* This must match sparc_cpu_properties[]. */
>   static const char * const feature_name[] = {
>       [CPU_FEATURE_BIT_FLOAT128] = "float128",
> +    [CPU_FEATURE_BIT_TA0_SHUTDOWN] = "shutdown",
> +    [CPU_FEATURE_BIT_POWERDOWN] = "powerdown",
> +    [CPU_FEATURE_BIT_CACHE_CTRL] = "cachectrl",
> +    [CPU_FEATURE_BIT_ASR17] = "asr17",
> +    [CPU_FEATURE_BIT_CASA] = "casa",
>   #ifdef TARGET_SPARC64
>       [CPU_FEATURE_BIT_CMT] = "cmt",
>       [CPU_FEATURE_BIT_GL] = "gl",

I'm not convinced this is a good idea.

The only effect of disabling these is to break LEON kernels. These features are not 
exposed for kernel discovery, so you can't just turn them on and have them used on 
supersparc either.  They will certainly break anything sparc64.


r~

Re: [PATCH-for-10.1 2/4] target/sparc: Restrict SPARC64 specific features

[Philippe Mathieu-Daudé]

On 25/3/25 15:07, Richard Henderson wrote:
> On 3/25/25 05:39, Philippe Mathieu-Daudé wrote:
>> @@ -2750,12 +2752,16 @@ static bool trans_SETHI(DisasContext *dc, 
>> arg_SETHI *a)
>>   static bool do_tcc(DisasContext *dc, int cond, int cc,
>>                      int rs1, bool imm, int rs2_or_imm)
>>   {
>> -    int mask = ((dc->def->features & CPU_FEATURE_HYPV) && supervisor(dc)
>> -                ? UA2005_HTRAP_MASK : V8_TRAP_MASK);
>> +    int mask = 0;
>>       DisasCompare cmp;
>>       TCGLabel *lab;
>>       TCGv_i32 trap;
>> +#ifdef TARGET_SPARC64
>> +    mask = ((dc->def->features & CPU_FEATURE_HYPV) && supervisor(dc))
>> +           ? UA2005_HTRAP_MASK : V8_TRAP_MASK;
>> +#endif
> 
> This is wrong.  The conversion could have been
> 
>    int mask = V8_TRAP_MASK;
> #ifdef
>    if (HYPV && super) {
>      mask = UA2005_HTRAP_MASK;
>    }
> #endif

Oh indeed. I guess I got confused by the parenthesis.

> but that's an excellent reason not to have the ifdef at all.
> 
> If you want to hide the features from sparc32, so they don't show up on 
> the command-line, fine.  But I think you want to introduce
> 
> #define CPU_FEATURE_HYPV 0
> 
> etc for sparc32 to automatically fail such tests as these without 
> further modification.

Hmm maybe not a big win after all since you said sparc32 and sparc64
will likely be considered as distinct architectures (in terms of QEMU
target implementations).

Re: [PATCH-for-10.1 1/4] target/sparc: Expose more CPU features

[Philippe Mathieu-Daudé]

On 25/3/25 15:11, Richard Henderson wrote:
> On 3/25/25 05:39, Philippe Mathieu-Daudé wrote:
>> Various features were not displayed or settable from command line.
>>
>> Diff of 'qemu-system-sparc -cpu help':
>>
>>   ...
>>     TI-SuperSparc-60     (IU 40000000 FPU 00000000 MMU 01000800 NWINS 8)
>>     TI-SuperSparc-61     (IU 44000000 FPU 00000000 MMU 01000000 NWINS 8)
>>     TI-SuperSparc-II     (IU 40000000 FPU 00000000 MMU 08000000 NWINS 8)
>>   - LEON2                (IU f2000000 FPU 00080000 MMU f2000000 NWINS 8)
>>   - LEON3                (IU f3000000 FPU 00080000 MMU f3000000 NWINS 8)
>>   + LEON2                (IU f2000000 FPU 00080000 MMU f2000000 NWINS 
>> 8) +shutdown
>>   + LEON3                (IU f3000000 FPU 00080000 MMU f3000000 NWINS 
>> 8) +shutdown +asr17 +cachectrl +powerdown +casa
>>    Default CPU feature flags (use '-' to remove): mul div fsmuld
>>   -Available CPU feature flags (use '+' to add): float128
>>   +Available CPU feature flags (use '+' to add): shutdown asr17 
>> cachectrl powerdown casa float128
>>    Numerical features (use '=' to set): iu_version fpu_version 
>> mmu_version nwindows
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/sparc/cpu.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
>> index 57161201173..37406227cb7 100644
>> --- a/target/sparc/cpu.c
>> +++ b/target/sparc/cpu.c
>> @@ -548,6 +548,11 @@ static const sparc_def_t sparc_defs[] = {
>>   /* This must match sparc_cpu_properties[]. */
>>   static const char * const feature_name[] = {
>>       [CPU_FEATURE_BIT_FLOAT128] = "float128",
>> +    [CPU_FEATURE_BIT_TA0_SHUTDOWN] = "shutdown",
>> +    [CPU_FEATURE_BIT_POWERDOWN] = "powerdown",
>> +    [CPU_FEATURE_BIT_CACHE_CTRL] = "cachectrl",
>> +    [CPU_FEATURE_BIT_ASR17] = "asr17",
>> +    [CPU_FEATURE_BIT_CASA] = "casa",
>>   #ifdef TARGET_SPARC64
>>       [CPU_FEATURE_BIT_CMT] = "cmt",
>>       [CPU_FEATURE_BIT_GL] = "gl",
> 
> I'm not convinced this is a good idea.
> 
> The only effect of disabling these is to break LEON kernels. These 
> features are not exposed for kernel discovery, so you can't just turn 
> them on and have them used on supersparc either.  They will certainly 
> break anything sparc64.

OK. It seems there is a lot of hidden knowledge behind this features
array :)

Re: [PATCH-for-10.1 3/4] target/sparc: Log unimplemented ASI load/store accesses

[Clément Chigot]

On Tue, Mar 25, 2025 at 1:39 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> When the cache-controller feature is not implemented,
> log potential ASI access as unimplemented.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/sparc/ldst_helper.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index b559afc9a94..d4de32d3c48 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -600,6 +600,10 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
>          case 0x0C:          /* Leon3 Date Cache config */
>              if (env->def.features & CPU_FEATURE_CACHE_CTRL) {
>                  ret = leon3_cache_control_ld(env, addr, size);
> +            } else {
> +                qemu_log_mask(LOG_UNIMP,
> +                              "%08x: unimplemented access size: %d\n", addr,

This should rather mimic the line in "default", as the issue is not
just the size value.
Same below.

Otherwise,
Reviewed-by: Clément Chigot <chigot@adacore.com>


> +                              size);
>              }
>              break;
>          case 0x01c00a00: /* MXCC control register */
> @@ -816,6 +820,10 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>          case 0x0C:          /* Leon3 Date Cache config */
>              if (env->def.features & CPU_FEATURE_CACHE_CTRL) {
>                  leon3_cache_control_st(env, addr, val, size);
> +            } else {
> +                qemu_log_mask(LOG_UNIMP,
> +                              "%08x: unimplemented access size: %d\n", addr,
> +                              size);
>              }
>              break;
>
> --
> 2.47.1
>

Re: [PATCH-for-10.1 4/4] target/sparc: Move hardware fields from CPUSPARCState to SPARCCPU

[Clément Chigot]

Reviewed-by: Clément Chigot <chigot@adacore.com>


On Tue, Mar 25, 2025 at 1:39 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Keep CPUSPARCState for architectural fields, move Leon3
> hardware specific fields to SPARCCPU.
>
> Reset the Leon3 specific 'cache_control' field in
> leon3_cpu_reset() instead of sparc_cpu_reset_hold().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/sparc/cpu.h          | 10 +++++-----
>  hw/sparc/leon3.c            | 35 ++++++++++++++++++-----------------
>  target/sparc/cpu.c          |  1 -
>  target/sparc/int32_helper.c |  8 ++++++--
>  target/sparc/ldst_helper.c  | 12 ++++++------
>  5 files changed, 35 insertions(+), 31 deletions(-)
>
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index 462bcb6c0e6..abb71c314dc 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -543,11 +543,6 @@ struct CPUArchState {
>  #define SOFTINT_REG_MASK (SOFTINT_STIMER|SOFTINT_INTRMASK|SOFTINT_TIMER)
>  #endif
>      sparc_def_t def;
> -
> -    /* Leon3 */
> -    DeviceState *irq_manager;
> -    void (*qemu_irq_ack)(CPUSPARCState *env, int intno);
> -    uint32_t cache_control;
>  };
>
>  /**
> @@ -560,6 +555,11 @@ struct ArchCPU {
>      CPUState parent_obj;
>
>      CPUSPARCState env;
> +
> +    /* Leon3 */
> +    DeviceState *irq_manager;
> +    void (*qemu_irq_ack)(SPARCCPU *cpu, int intno);
> +    uint32_t cache_control;
>  };
>
>  /**
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 0aeaad3becc..06966861744 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -152,6 +152,7 @@ static void leon3_cpu_reset(void *opaque)
>      int id = info->id;
>      ResetData *s = container_of(info, ResetData, info[id]);
>      CPUState *cpu = CPU(s->info[id].cpu);
> +    SPARCCPU *scpu = SPARC_CPU(cpu);
>      CPUSPARCState *env = cpu_env(cpu);
>
>      cpu_reset(cpu);
> @@ -159,41 +160,41 @@ static void leon3_cpu_reset(void *opaque)
>      cpu->halted = cpu->cpu_index != 0;
>      env->pc = s->entry;
>      env->npc = s->entry + 4;
> +    scpu->cache_control = 0;
>  }
>
> -static void leon3_cache_control_int(CPUSPARCState *env)
> +static void leon3_cache_control_int(SPARCCPU *cpu)
>  {
>      uint32_t state = 0;
>
> -    if (env->cache_control & CACHE_CTRL_IF) {
> +    if (cpu->cache_control & CACHE_CTRL_IF) {
>          /* Instruction cache state */
> -        state = env->cache_control & CACHE_STATE_MASK;
> +        state = cpu->cache_control & CACHE_STATE_MASK;
>          if (state == CACHE_ENABLED) {
>              state = CACHE_FROZEN;
>              trace_int_helper_icache_freeze();
>          }
>
> -        env->cache_control &= ~CACHE_STATE_MASK;
> -        env->cache_control |= state;
> +        cpu->cache_control &= ~CACHE_STATE_MASK;
> +        cpu->cache_control |= state;
>      }
>
> -    if (env->cache_control & CACHE_CTRL_DF) {
> +    if (cpu->cache_control & CACHE_CTRL_DF) {
>          /* Data cache state */
> -        state = (env->cache_control >> 2) & CACHE_STATE_MASK;
> +        state = (cpu->cache_control >> 2) & CACHE_STATE_MASK;
>          if (state == CACHE_ENABLED) {
>              state = CACHE_FROZEN;
>              trace_int_helper_dcache_freeze();
>          }
>
> -        env->cache_control &= ~(CACHE_STATE_MASK << 2);
> -        env->cache_control |= (state << 2);
> +        cpu->cache_control &= ~(CACHE_STATE_MASK << 2);
> +        cpu->cache_control |= (state << 2);
>      }
>  }
>
> -static void leon3_irq_ack(CPUSPARCState *env, int intno)
> +static void leon3_irq_ack(SPARCCPU *cpu, int intno)
>  {
> -    CPUState *cpu = CPU(env_cpu(env));
> -    grlib_irqmp_ack(env->irq_manager, cpu->cpu_index, intno);
> +    grlib_irqmp_ack(cpu->irq_manager, CPU(cpu)->cpu_index, intno);
>  }
>
>  /*
> @@ -248,10 +249,10 @@ static void leon3_start_cpu(void *opaque, int n, int level)
>      async_run_on_cpu(cs, leon3_start_cpu_async_work, RUN_ON_CPU_NULL);
>  }
>
> -static void leon3_irq_manager(CPUSPARCState *env, int intno)
> +static void leon3_irq_manager(SPARCCPU *cpu, int intno)
>  {
> -    leon3_irq_ack(env, intno);
> -    leon3_cache_control_int(env);
> +    leon3_irq_ack(cpu, intno);
> +    leon3_cache_control_int(cpu);
>  }
>
>  static void leon3_generic_hw_init(MachineState *machine)
> @@ -320,8 +321,8 @@ static void leon3_generic_hw_init(MachineState *machine)
>          qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", i,
>                                      qdev_get_gpio_in_named(DEVICE(cpu),
>                                                             "pil", 0));
> -        env->irq_manager = irqmpdev;
> -        env->qemu_irq_ack = leon3_irq_manager;
> +        cpu->irq_manager = irqmpdev;
> +        cpu->qemu_irq_ack = leon3_irq_manager;
>      }
>
>      sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 37406227cb7..d62ad6c4db2 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -78,7 +78,6 @@ static void sparc_cpu_reset_hold(Object *obj, ResetType type)
>      env->pc = 0;
>      env->npc = env->pc + 4;
>  #endif
> -    env->cache_control = 0;
>      cpu_put_fsr(env, 0);
>  }
>
> diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
> index f0266061023..a902702559d 100644
> --- a/target/sparc/int32_helper.c
> +++ b/target/sparc/int32_helper.c
> @@ -168,8 +168,12 @@ void sparc_cpu_do_interrupt(CPUState *cs)
>
>  #if !defined(CONFIG_USER_ONLY)
>      /* IRQ acknowledgment */
> -    if ((intno & ~15) == TT_EXTINT && env->qemu_irq_ack != NULL) {
> -        env->qemu_irq_ack(env, intno);
> +    if ((intno & ~15) == TT_EXTINT) {
> +        SPARCCPU *cpu = env_archcpu(env);
> +
> +        if (cpu->qemu_irq_ack != NULL) {
> +            cpu->qemu_irq_ack(cpu, intno);
> +        }
>      }
>  #endif
>  }
> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index d4de32d3c48..0a11360ccaf 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -513,7 +513,7 @@ static void sparc_raise_mmu_fault(CPUState *cs, hwaddr addr,
>
>  /* Leon3 cache control */
>
> -static void leon3_cache_control_st(CPUSPARCState *env, target_ulong addr,
> +static void leon3_cache_control_st(SPARCCPU *cpu, target_ulong addr,
>                                     uint64_t val, int size)
>  {
>      DPRINTF_CACHE_CONTROL("st addr:%08x, val:%" PRIx64 ", size:%d\n",
> @@ -534,7 +534,7 @@ static void leon3_cache_control_st(CPUSPARCState *env, target_ulong addr,
>          val &= ~CACHE_CTRL_IP;
>          val &= ~CACHE_CTRL_DP;
>
> -        env->cache_control = val;
> +        cpu->cache_control = val;
>          break;
>      case 0x04:              /* Instruction cache configuration */
>      case 0x08:              /* Data cache configuration */
> @@ -546,7 +546,7 @@ static void leon3_cache_control_st(CPUSPARCState *env, target_ulong addr,
>      };
>  }
>
> -static uint64_t leon3_cache_control_ld(CPUSPARCState *env, target_ulong addr,
> +static uint64_t leon3_cache_control_ld(SPARCCPU *cpu, target_ulong addr,
>                                         int size)
>  {
>      uint64_t ret = 0;
> @@ -558,7 +558,7 @@ static uint64_t leon3_cache_control_ld(CPUSPARCState *env, target_ulong addr,
>
>      switch (addr) {
>      case 0x00:              /* Cache control */
> -        ret = env->cache_control;
> +        ret = cpu->cache_control;
>          break;
>
>          /* Configuration registers are read and only always keep those
> @@ -599,7 +599,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
>          case 0x08:          /* Leon3 Instruction Cache config */
>          case 0x0C:          /* Leon3 Date Cache config */
>              if (env->def.features & CPU_FEATURE_CACHE_CTRL) {
> -                ret = leon3_cache_control_ld(env, addr, size);
> +                ret = leon3_cache_control_ld(env_archcpu(env), addr, size);
>              } else {
>                  qemu_log_mask(LOG_UNIMP,
>                                "%08x: unimplemented access size: %d\n", addr,
> @@ -819,7 +819,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>          case 0x08:          /* Leon3 Instruction Cache config */
>          case 0x0C:          /* Leon3 Date Cache config */
>              if (env->def.features & CPU_FEATURE_CACHE_CTRL) {
> -                leon3_cache_control_st(env, addr, val, size);
> +                leon3_cache_control_st(env_archcpu(env), addr, val, size);
>              } else {
>                  qemu_log_mask(LOG_UNIMP,
>                                "%08x: unimplemented access size: %d\n", addr,
> --
> 2.47.1
>