diff mbox series

[1/3] hw/mips/jazz: Fix modifying QOM class internal state from instance

Message ID 20230523064408.57941-2-philmd@linaro.org
State New
Headers show
Series hw: Fix abuse of QOM class internals modified by their instances | expand

Commit Message

Philippe Mathieu-Daudé May 23, 2023, 6:44 a.m. UTC 
QOM object instance should not modify its class state (because
all other objects instanciated from this class get affected).

Instead of modifying the MIPSCPUClass 'no_data_aborts' field
in the instance machine_init() handler, set it in the machine
class_init handler. Since 2 machines require this, share the
common code in a new machine_class_ignore_data_abort() helper.

Inspired-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/mips/jazz.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

Comments

Richard Henderson May 23, 2023, 6:26 p.m. UTC | #1 
On 5/22/23 23:44, Philippe Mathieu-Daudé wrote:
> QOM object instance should not modify its class state (because
> all other objects instanciated from this class get affected).
> 
> Instead of modifying the MIPSCPUClass 'no_data_aborts' field
> in the instance machine_init() handler, set it in the machine
> class_init handler. Since 2 machines require this, share the
> common code in a new machine_class_ignore_data_abort() helper.
> 
> Inspired-by: Bernhard Beschow<shentey@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/mips/jazz.c | 41 +++++++++++++++++++++++------------------
>   1 file changed, 23 insertions(+), 18 deletions(-)

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

r~
diff mbox series

Patch

diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index ca4426a92c..de2e827bf8 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -128,7 +128,6 @@  static void mips_jazz_init(MachineState *machine,
     int bios_size, n, big_endian;
     Clock *cpuclk;
     MIPSCPU *cpu;
-    MIPSCPUClass *mcc;
     CPUMIPSState *env;
     qemu_irq *i8259;
     rc4030_dma *dmas;
@@ -177,23 +176,6 @@  static void mips_jazz_init(MachineState *machine,
     env = &cpu->env;
     qemu_register_reset(main_cpu_reset, cpu);
 
-    /*
-     * Chipset returns 0 in invalid reads and do not raise data exceptions.
-     * However, we can't simply add a global memory region to catch
-     * everything, as this would make all accesses including instruction
-     * accesses be ignored and not raise exceptions.
-     *
-     * NOTE: this behaviour of raising exceptions for bad instruction
-     * fetches but not bad data accesses was added in commit 54e755588cf1e9
-     * to restore behaviour broken by c658b94f6e8c206, but it is not clear
-     * whether the real hardware behaves this way. It is possible that
-     * real hardware ignores bad instruction fetches as well -- if so then
-     * we could replace this hijacking of CPU methods with a simple global
-     * memory region that catches all memory accesses, as we do on Malta.
-     */
-    mcc = MIPS_CPU_GET_CLASS(cpu);
-    mcc->no_data_aborts = true;
-
     /* allocate RAM */
     memory_region_add_subregion(address_space, 0, machine->ram);
 
@@ -414,6 +396,27 @@  void mips_pica61_init(MachineState *machine)
     mips_jazz_init(machine, JAZZ_PICA61);
 }
 
+static void machine_class_ignore_data_abort(MachineClass *mc)
+{
+    MIPSCPUClass *mcc = MIPS_CPU_CLASS(mc);
+
+    /*
+     * Chipset returns 0 in invalid reads and do not raise data exceptions.
+     * However, we can't simply add a global memory region to catch
+     * everything, as this would make all accesses including instruction
+     * accesses be ignored and not raise exceptions.
+     *
+     * NOTE: this behaviour of raising exceptions for bad instruction
+     * fetches but not bad data accesses was added in commit 54e755588cf1e9
+     * to restore behaviour broken by c658b94f6e8c206, but it is not clear
+     * whether the real hardware behaves this way. It is possible that
+     * real hardware ignores bad instruction fetches as well -- if so then
+     * we could replace this hijacking of CPU methods with a simple global
+     * memory region that catches all memory accesses, as we do on Malta.
+     */
+    mcc->no_data_aborts = true;
+}
+
 static void mips_magnum_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -423,6 +426,7 @@  static void mips_magnum_class_init(ObjectClass *oc, void *data)
     mc->block_default_type = IF_SCSI;
     mc->default_cpu_type = MIPS_CPU_TYPE_NAME("R4000");
     mc->default_ram_id = "mips_jazz.ram";
+    machine_class_ignore_data_abort(mc);
 }
 
 static const TypeInfo mips_magnum_type = {
@@ -440,6 +444,7 @@  static void mips_pica61_class_init(ObjectClass *oc, void *data)
     mc->block_default_type = IF_SCSI;
     mc->default_cpu_type = MIPS_CPU_TYPE_NAME("R4000");
     mc->default_ram_id = "mips_jazz.ram";
+    machine_class_ignore_data_abort(mc);
 }
 
 static const TypeInfo mips_pica61_type = {