Re: [RFC PATCH 2/2] hw/sparc: Allow building without the leon3 machine

2021-04-28 Thread Fred Konrad

Hi Philippe,

Le 4/27/21 à 9:26 PM, Philippe Mathieu-Daudé a écrit :

When building without the leon3 machine, we get this link failure:

   /usr/bin/ld: target_sparc_int32_helper.c.o: in function `leon3_irq_manager':
   target/sparc/int32_helper.c:172: undefined reference to `leon3_irq_ack'

This is because the leon3_irq_ack() is declared in hw/sparc/leon3.c,
which is only build when CONFIG_LEON3 is selected.

Fix by moving the leon3_cache_control_int() / leon3_irq_manager()
(which are specific to the leon3 machine) to hw/sparc/leon3.c.
Move the trace events along (but don't rename them).

leon3_irq_ack() is now locally used, declare it static to reduce
its scope.

Signed-off-by: Philippe Mathieu-Daudé 


Reviewed-by: KONRAD Frederic 
Tested-by: KONRAD Frederic 

Thanks for the fix!


---
RFC: The problem is we have hardware specific code in the
architectural translation code. I wish there was a better
alternative rather than moving this code to hw/sparc/.
---
  target/sparc/cpu.h  |  6 --
  hw/sparc/leon3.c| 37 -
  target/sparc/int32_helper.c | 37 -
  hw/sparc/trace-events   |  2 ++
  target/sparc/trace-events   |  4 
  5 files changed, 38 insertions(+), 48 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index 4b2290650be..ff8ae73002a 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -615,15 +615,9 @@ int cpu_cwp_inc(CPUSPARCState *env1, int cwp);
  int cpu_cwp_dec(CPUSPARCState *env1, int cwp);
  void cpu_set_cwp(CPUSPARCState *env1, int new_cwp);
  
-/* int_helper.c */

-void leon3_irq_manager(CPUSPARCState *env, void *irq_manager, int intno);
-
  /* sun4m.c, sun4u.c */
  void cpu_check_irqs(CPUSPARCState *env);
  
-/* leon3.c */

-void leon3_irq_ack(void *irq_manager, int intno);
-
  #if defined (TARGET_SPARC64)
  
  static inline int compare_masked(uint64_t x, uint64_t y, uint64_t mask)

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 7e16eea9e67..98e3789cf84 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -137,7 +137,36 @@ static void main_cpu_reset(void *opaque)
  env->regbase[6] = s->sp;
  }
  
-void leon3_irq_ack(void *irq_manager, int intno)

+static void leon3_cache_control_int(CPUSPARCState *env)
+{
+uint32_t state = 0;
+
+if (env->cache_control & CACHE_CTRL_IF) {
+/* Instruction cache state */
+state = env->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;
+}
+
+if (env->cache_control & CACHE_CTRL_DF) {
+/* Data cache state */
+state = (env->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);
+}
+}
+
+static void leon3_irq_ack(void *irq_manager, int intno)
  {
  grlib_irqmp_ack((DeviceState *)irq_manager, intno);
  }
@@ -181,6 +210,12 @@ static void leon3_set_pil_in(void *opaque, int n, int 
level)
  }
  }
  
+static void leon3_irq_manager(CPUSPARCState *env, void *irq_manager, int intno)

+{
+leon3_irq_ack(irq_manager, intno);
+leon3_cache_control_int(env);
+}
+
  static void leon3_generic_hw_init(MachineState *machine)
  {
  ram_addr_t ram_size = machine->ram_size;
diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
index 817a463a179..d008dbdb65c 100644
--- a/target/sparc/int32_helper.c
+++ b/target/sparc/int32_helper.c
@@ -136,40 +136,3 @@ void sparc_cpu_do_interrupt(CPUState *cs)
  }
  #endif
  }
-
-#if !defined(CONFIG_USER_ONLY)
-static void leon3_cache_control_int(CPUSPARCState *env)
-{
-uint32_t state = 0;
-
-if (env->cache_control & CACHE_CTRL_IF) {
-/* Instruction cache state */
-state = env->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;
-}
-
-if (env->cache_control & CACHE_CTRL_DF) {
-/* Data cache state */
-state = (env->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);
-}
-}
-
-void leon3_irq_manager(CPUSPARCState *env, void *irq_manager, int intno)
-{
-leon3_irq_ack(irq_manager, intno);
-leon3_cache_control_int(env);
-}
-#endif
diff --git a/hw/sparc/trace-events b/hw/sparc/trace-events
index 355b07ae057..dfb53dc1a24 100644
--- a

Re: [RFC PATCH 2/2] hw/sparc: Allow building without the leon3 machine

2021-04-27 Thread Richard Henderson

On 4/27/21 12:26 PM, Philippe Mathieu-Daudé wrote:

When building without the leon3 machine, we get this link failure:

   /usr/bin/ld: target_sparc_int32_helper.c.o: in function `leon3_irq_manager':
   target/sparc/int32_helper.c:172: undefined reference to `leon3_irq_ack'

This is because the leon3_irq_ack() is declared in hw/sparc/leon3.c,
which is only build when CONFIG_LEON3 is selected.

Fix by moving the leon3_cache_control_int() / leon3_irq_manager()
(which are specific to the leon3 machine) to hw/sparc/leon3.c.
Move the trace events along (but don't rename them).

leon3_irq_ack() is now locally used, declare it static to reduce
its scope.

Signed-off-by: Philippe Mathieu-Daudé
---
RFC: The problem is we have hardware specific code in the
architectural translation code. I wish there was a better
alternative rather than moving this code to hw/sparc/.
---


This one seems dead obvious.  I think this code should have been in 
hw/sparc/leon3.c to begin with.


Reviewed-by: Richard Henderson 

r~



[RFC PATCH 2/2] hw/sparc: Allow building without the leon3 machine

2021-04-27 Thread Philippe Mathieu-Daudé
When building without the leon3 machine, we get this link failure:

  /usr/bin/ld: target_sparc_int32_helper.c.o: in function `leon3_irq_manager':
  target/sparc/int32_helper.c:172: undefined reference to `leon3_irq_ack'

This is because the leon3_irq_ack() is declared in hw/sparc/leon3.c,
which is only build when CONFIG_LEON3 is selected.

Fix by moving the leon3_cache_control_int() / leon3_irq_manager()
(which are specific to the leon3 machine) to hw/sparc/leon3.c.
Move the trace events along (but don't rename them).

leon3_irq_ack() is now locally used, declare it static to reduce
its scope.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: The problem is we have hardware specific code in the
architectural translation code. I wish there was a better
alternative rather than moving this code to hw/sparc/.
---
 target/sparc/cpu.h  |  6 --
 hw/sparc/leon3.c| 37 -
 target/sparc/int32_helper.c | 37 -
 hw/sparc/trace-events   |  2 ++
 target/sparc/trace-events   |  4 
 5 files changed, 38 insertions(+), 48 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index 4b2290650be..ff8ae73002a 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -615,15 +615,9 @@ int cpu_cwp_inc(CPUSPARCState *env1, int cwp);
 int cpu_cwp_dec(CPUSPARCState *env1, int cwp);
 void cpu_set_cwp(CPUSPARCState *env1, int new_cwp);
 
-/* int_helper.c */
-void leon3_irq_manager(CPUSPARCState *env, void *irq_manager, int intno);
-
 /* sun4m.c, sun4u.c */
 void cpu_check_irqs(CPUSPARCState *env);
 
-/* leon3.c */
-void leon3_irq_ack(void *irq_manager, int intno);
-
 #if defined (TARGET_SPARC64)
 
 static inline int compare_masked(uint64_t x, uint64_t y, uint64_t mask)
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 7e16eea9e67..98e3789cf84 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -137,7 +137,36 @@ static void main_cpu_reset(void *opaque)
 env->regbase[6] = s->sp;
 }
 
-void leon3_irq_ack(void *irq_manager, int intno)
+static void leon3_cache_control_int(CPUSPARCState *env)
+{
+uint32_t state = 0;
+
+if (env->cache_control & CACHE_CTRL_IF) {
+/* Instruction cache state */
+state = env->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;
+}
+
+if (env->cache_control & CACHE_CTRL_DF) {
+/* Data cache state */
+state = (env->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);
+}
+}
+
+static void leon3_irq_ack(void *irq_manager, int intno)
 {
 grlib_irqmp_ack((DeviceState *)irq_manager, intno);
 }
@@ -181,6 +210,12 @@ static void leon3_set_pil_in(void *opaque, int n, int 
level)
 }
 }
 
+static void leon3_irq_manager(CPUSPARCState *env, void *irq_manager, int intno)
+{
+leon3_irq_ack(irq_manager, intno);
+leon3_cache_control_int(env);
+}
+
 static void leon3_generic_hw_init(MachineState *machine)
 {
 ram_addr_t ram_size = machine->ram_size;
diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
index 817a463a179..d008dbdb65c 100644
--- a/target/sparc/int32_helper.c
+++ b/target/sparc/int32_helper.c
@@ -136,40 +136,3 @@ void sparc_cpu_do_interrupt(CPUState *cs)
 }
 #endif
 }
-
-#if !defined(CONFIG_USER_ONLY)
-static void leon3_cache_control_int(CPUSPARCState *env)
-{
-uint32_t state = 0;
-
-if (env->cache_control & CACHE_CTRL_IF) {
-/* Instruction cache state */
-state = env->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;
-}
-
-if (env->cache_control & CACHE_CTRL_DF) {
-/* Data cache state */
-state = (env->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);
-}
-}
-
-void leon3_irq_manager(CPUSPARCState *env, void *irq_manager, int intno)
-{
-leon3_irq_ack(irq_manager, intno);
-leon3_cache_control_int(env);
-}
-#endif
diff --git a/hw/sparc/trace-events b/hw/sparc/trace-events
index 355b07ae057..dfb53dc1a24 100644
--- a/hw/sparc/trace-events
+++ b/hw/sparc/trace-events
@@ -19,3 +19,5 @@ sun4m_iommu_bad_addr(uint64_t addr) "bad addr 0x%"PRIx64
 # leon3.c
 leon3_set_irq(int intno) "Set CPU IRQ %d"
 leon3_rese