[PATCH 3/3] meson: Enable -Wvla

Thomas Huth posted 3 patches 9 months, 1 week ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>
[PATCH 3/3] meson: Enable -Wvla
Posted by Thomas Huth 9 months, 1 week ago
From: Peter Maydell <peter.maydell@linaro.org>

QEMU has historically used variable length arrays only very rarely.
Variable length arrays are a potential security issue where an
on-stack dynamic allocation isn't correctly size-checked, especially
when the size comes from the guest.  (An example problem of this kind
from the past is CVE-2021-3527).  Forbidding them entirely is a
defensive measure against further bugs of this kind.

Enable -Wvla to prevent any new uses from sneaking into the codebase.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20240125173211.1786196-3-peter.maydell@linaro.org>
[thuth: rebased to current master branch]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index c1dc83e4c0..0ef1654e86 100644
--- a/meson.build
+++ b/meson.build
@@ -592,6 +592,7 @@ warn_flags = [
   '-Wstrict-prototypes',
   '-Wtype-limits',
   '-Wundef',
+  '-Wvla',
   '-Wwrite-strings',
 
   # Then disable some undesirable warnings
-- 
2.43.2
Re: [PATCH 3/3] meson: Enable -Wvla
Posted by Thomas Huth 9 months, 1 week ago
On 21/02/2024 17.26, Thomas Huth wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
> 
> QEMU has historically used variable length arrays only very rarely.
> Variable length arrays are a potential security issue where an
> on-stack dynamic allocation isn't correctly size-checked, especially
> when the size comes from the guest.  (An example problem of this kind
> from the past is CVE-2021-3527).  Forbidding them entirely is a
> defensive measure against further bugs of this kind.
> 
> Enable -Wvla to prevent any new uses from sneaking into the codebase.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Message-ID: <20240125173211.1786196-3-peter.maydell@linaro.org>
> [thuth: rebased to current master branch]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   meson.build | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/meson.build b/meson.build
> index c1dc83e4c0..0ef1654e86 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -592,6 +592,7 @@ warn_flags = [
>     '-Wstrict-prototypes',
>     '-Wtype-limits',
>     '-Wundef',
> +  '-Wvla',
>     '-Wwrite-strings',
>   
>     # Then disable some undesirable warnings

Sigh, there's a new warning in the latest master branch:

  https://gitlab.com/thuth/qemu/-/jobs/6225992174

Caused by commit d65aba828 ("hw/sparc/leon3: implement multiprocessor")...
Clément, Philippe, could this maybe be written in a different way that does 
not trigger a -Wvla warning?

  Thomas


Re: [PATCH 3/3] meson: Enable -Wvla
Posted by Thomas Huth 9 months, 1 week ago
On 21/02/2024 17.59, Thomas Huth wrote:
> On 21/02/2024 17.26, Thomas Huth wrote:
>> From: Peter Maydell <peter.maydell@linaro.org>
>>
>> QEMU has historically used variable length arrays only very rarely.
>> Variable length arrays are a potential security issue where an
>> on-stack dynamic allocation isn't correctly size-checked, especially
>> when the size comes from the guest.  (An example problem of this kind
>> from the past is CVE-2021-3527).  Forbidding them entirely is a
>> defensive measure against further bugs of this kind.
>>
>> Enable -Wvla to prevent any new uses from sneaking into the codebase.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> Message-ID: <20240125173211.1786196-3-peter.maydell@linaro.org>
>> [thuth: rebased to current master branch]
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   meson.build | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/meson.build b/meson.build
>> index c1dc83e4c0..0ef1654e86 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -592,6 +592,7 @@ warn_flags = [
>>     '-Wstrict-prototypes',
>>     '-Wtype-limits',
>>     '-Wundef',
>> +  '-Wvla',
>>     '-Wwrite-strings',
>>     # Then disable some undesirable warnings
> 
> Sigh, there's a new warning in the latest master branch:
> 
>   https://gitlab.com/thuth/qemu/-/jobs/6225992174
> 
> Caused by commit d65aba828 ("hw/sparc/leon3: implement multiprocessor")...
> Clément, Philippe, could this maybe be written in a different way that does 
> not trigger a -Wvla warning?

I think the DO_UPCAST is wrong here - if I got that right, DO_UPCAST is 
supposed to check that the second parameter is at the same location as the 
first type later points to. This is not the case here. I think we rather 
want container_of() here, so this patch is likely a simple fix:

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -150,7 +150,7 @@ static void leon3_cpu_reset(void *opaque)
  {
      struct CPUResetData *info = (struct CPUResetData *) opaque;
      int id = info->id;
-    ResetData *s = (ResetData *)DO_UPCAST(ResetData, info[id], info);
+    ResetData *s = container_of(info, ResetData, info[id]);
      CPUState *cpu = CPU(s->info[id].cpu);
      CPUSPARCState *env = cpu_env(cpu);

I can send it as a proper patch, too.

  Thomas


Re: [PATCH 3/3] meson: Enable -Wvla
Posted by Philippe Mathieu-Daudé 9 months, 1 week ago
On 21/2/24 17:59, Thomas Huth wrote:
> On 21/02/2024 17.26, Thomas Huth wrote:
>> From: Peter Maydell <peter.maydell@linaro.org>
>>
>> QEMU has historically used variable length arrays only very rarely.
>> Variable length arrays are a potential security issue where an
>> on-stack dynamic allocation isn't correctly size-checked, especially
>> when the size comes from the guest.  (An example problem of this kind
>> from the past is CVE-2021-3527).  Forbidding them entirely is a
>> defensive measure against further bugs of this kind.
>>
>> Enable -Wvla to prevent any new uses from sneaking into the codebase.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> Message-ID: <20240125173211.1786196-3-peter.maydell@linaro.org>
>> [thuth: rebased to current master branch]
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   meson.build | 1 +
>>   1 file changed, 1 insertion(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>> diff --git a/meson.build b/meson.build
>> index c1dc83e4c0..0ef1654e86 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -592,6 +592,7 @@ warn_flags = [
>>     '-Wstrict-prototypes',
>>     '-Wtype-limits',
>>     '-Wundef',
>> +  '-Wvla',
>>     '-Wwrite-strings',
>>     # Then disable some undesirable warnings
> 
> Sigh, there's a new warning in the latest master branch:
> 
>   https://gitlab.com/thuth/qemu/-/jobs/6225992174
> 
> Caused by commit d65aba828 ("hw/sparc/leon3: implement multiprocessor")...
> Clément, Philippe, could this maybe be written in a different way that 
> does not trigger a -Wvla warning?

Clément, ResetData::entry isn't used, so we can simplify removing
the whole ResetData structure, but I'm not sure this is intended:

-- >8 --
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 4873b59b6c..1ff6b5d63d 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -68,14 +68,6 @@
  #define LEON3_APB_PNP_OFFSET (0x800FF000)
  #define LEON3_AHB_PNP_OFFSET (0xFFFFF000)

-typedef struct ResetData {
-    struct CPUResetData {
-        int id;
-        SPARCCPU *cpu;
-    } info[MAX_CPUS];
-    uint32_t entry;             /* save kernel entry in case of reset */
-} ResetData;
-
  static uint32_t *gen_store_u32(uint32_t *code, hwaddr addr, uint32_t val)
  {
      stl_p(code++, 0x82100000); /* mov %g0, %g1                */
@@ -148,17 +140,14 @@ static void write_bootloader(void *ptr, hwaddr 
kernel_addr)

  static void leon3_cpu_reset(void *opaque)
  {
-    struct CPUResetData *info = (struct CPUResetData *) opaque;
-    int id = info->id;
-    ResetData *s = (ResetData *)DO_UPCAST(ResetData, info[id], info);
-    CPUState *cpu = CPU(s->info[id].cpu);
+    CPUState *cpu = opaque;
      CPUSPARCState *env = cpu_env(cpu);

      cpu_reset(cpu);

      cpu->halted = cpu->cpu_index != 0;
-    env->pc = s->entry;
-    env->npc = s->entry + 4;
+    env->pc = LEON3_PROM_OFFSET;
+    env->npc = LEON3_PROM_OFFSET + 4;
  }

  static void leon3_cache_control_int(CPUSPARCState *env)
@@ -259,7 +248,7 @@ static void leon3_generic_hw_init(MachineState *machine)
      ram_addr_t ram_size = machine->ram_size;
      const char *bios_name = machine->firmware ?: LEON3_PROM_FILENAME;
      const char *kernel_filename = machine->kernel_filename;
-    SPARCCPU *cpu;
+    SPARCCPU *cpu[MAX_CPUS];
      CPUSPARCState   *env;
      MemoryRegion *address_space_mem = get_system_memory();
      MemoryRegion *prom = g_new(MemoryRegion, 1);
@@ -267,28 +256,22 @@ static void leon3_generic_hw_init(MachineState 
*machine)
      char       *filename;
      int         bios_size;
      int         prom_size;
-    ResetData  *reset_info;
      DeviceState *dev, *irqmpdev;
      int i;
      AHBPnp *ahb_pnp;
      APBPnp *apb_pnp;

-    reset_info = g_malloc0(sizeof(ResetData));
-
      for (i = 0; i < machine->smp.cpus; i++) {
          /* Init CPU */
-        cpu = SPARC_CPU(object_new(machine->cpu_type));
-        qdev_init_gpio_in_named(DEVICE(cpu), leon3_start_cpu, 
"start_cpu", 1);
-        qdev_init_gpio_in_named(DEVICE(cpu), leon3_set_pil_in, "pil", 1);
-        qdev_realize(DEVICE(cpu), NULL, &error_fatal);
-        env = &cpu->env;
+        cpu[i] = SPARC_CPU(object_new(machine->cpu_type));
+        qdev_init_gpio_in_named(DEVICE(cpu[i]), leon3_start_cpu, 
"start_cpu", 1);
+        qdev_init_gpio_in_named(DEVICE(cpu[i]), leon3_set_pil_in, 
"pil", 1);
+        qdev_realize(DEVICE(cpu[i]), NULL, &error_fatal);
+        env = &cpu[i]->env;

          cpu_sparc_set_id(env, i);

-        /* Reset data */
-        reset_info->info[i].id = i;
-        reset_info->info[i].cpu = cpu;
-        qemu_register_reset(leon3_cpu_reset, &reset_info->info[i]);
+        qemu_register_reset(leon3_cpu_reset, cpu[i]);
      }

      ahb_pnp = GRLIB_AHB_PNP(qdev_new(TYPE_GRLIB_AHB_PNP));
@@ -312,13 +295,12 @@ static void leon3_generic_hw_init(MachineState 
*machine)
      sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);

      for (i = 0; i < machine->smp.cpus; i++) {
-        cpu = reset_info->info[i].cpu;
-        env = &cpu->env;
+        env = &cpu[i]->env;
          qdev_connect_gpio_out_named(irqmpdev, "grlib-start-cpu", i,
-                                    qdev_get_gpio_in_named(DEVICE(cpu),
+                                    qdev_get_gpio_in_named(DEVICE(cpu[i]),
 
"start_cpu", 0));
          qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", i,
-                                    qdev_get_gpio_in_named(DEVICE(cpu),
+                                    qdev_get_gpio_in_named(DEVICE(cpu[i]),
                                                             "pil", 0));
          env->irq_manager = irqmpdev;
          env->qemu_irq_ack = leon3_irq_manager;
@@ -396,11 +378,6 @@ static void leon3_generic_hw_init(MachineState 
*machine)
               * bootloader.
               */
              write_bootloader(memory_region_get_ram_ptr(prom), entry);
-            reset_info->entry = LEON3_PROM_OFFSET;
-            for (i = 0; i < machine->smp.cpus; i++) {
-                reset_info->info[i].cpu->env.pc = LEON3_PROM_OFFSET;
-                reset_info->info[i].cpu->env.npc = LEON3_PROM_OFFSET + 4;
-            }
          }
      }
---

Regards,

Phil.

Re: [PATCH 3/3] meson: Enable -Wvla
Posted by Clément Chigot 9 months, 1 week ago
On Wed, Feb 21, 2024 at 6:27 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 21/2/24 17:59, Thomas Huth wrote:
> > On 21/02/2024 17.26, Thomas Huth wrote:
> >> From: Peter Maydell <peter.maydell@linaro.org>
> >>
> >> QEMU has historically used variable length arrays only very rarely.
> >> Variable length arrays are a potential security issue where an
> >> on-stack dynamic allocation isn't correctly size-checked, especially
> >> when the size comes from the guest.  (An example problem of this kind
> >> from the past is CVE-2021-3527).  Forbidding them entirely is a
> >> defensive measure against further bugs of this kind.
> >>
> >> Enable -Wvla to prevent any new uses from sneaking into the codebase.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> Message-ID: <20240125173211.1786196-3-peter.maydell@linaro.org>
> >> [thuth: rebased to current master branch]
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>   meson.build | 1 +
> >>   1 file changed, 1 insertion(+)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> >> diff --git a/meson.build b/meson.build
> >> index c1dc83e4c0..0ef1654e86 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -592,6 +592,7 @@ warn_flags = [
> >>     '-Wstrict-prototypes',
> >>     '-Wtype-limits',
> >>     '-Wundef',
> >> +  '-Wvla',
> >>     '-Wwrite-strings',
> >>     # Then disable some undesirable warnings
> >
> > Sigh, there's a new warning in the latest master branch:
> >
> >   https://gitlab.com/thuth/qemu/-/jobs/6225992174
> >
> > Caused by commit d65aba828 ("hw/sparc/leon3: implement multiprocessor")...
> > Clément, Philippe, could this maybe be written in a different way that
> > does not trigger a -Wvla warning?
>
> Clément, ResetData::entry isn't used, so we can simplify removing
> the whole ResetData structure, but I'm not sure this is intended:

AFAICT, it used to take the kernel entry point before the bootloader
part was implemented. But I'm wondering if it could not be one of the
issues being the avocado failure. I do want to resurrect it but I
missed the time at the moment...
I'll do some testing with your patch and see how it goes anyway.

> -- >8 --
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 4873b59b6c..1ff6b5d63d 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -68,14 +68,6 @@
>   #define LEON3_APB_PNP_OFFSET (0x800FF000)
>   #define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
>
> -typedef struct ResetData {
> -    struct CPUResetData {
> -        int id;
> -        SPARCCPU *cpu;
> -    } info[MAX_CPUS];
> -    uint32_t entry;             /* save kernel entry in case of reset */
> -} ResetData;
> -
>   static uint32_t *gen_store_u32(uint32_t *code, hwaddr addr, uint32_t val)
>   {
>       stl_p(code++, 0x82100000); /* mov %g0, %g1                */
> @@ -148,17 +140,14 @@ static void write_bootloader(void *ptr, hwaddr
> kernel_addr)
>
>   static void leon3_cpu_reset(void *opaque)
>   {
> -    struct CPUResetData *info = (struct CPUResetData *) opaque;
> -    int id = info->id;
> -    ResetData *s = (ResetData *)DO_UPCAST(ResetData, info[id], info);
> -    CPUState *cpu = CPU(s->info[id].cpu);
> +    CPUState *cpu = opaque;
>       CPUSPARCState *env = cpu_env(cpu);
>
>       cpu_reset(cpu);
>
>       cpu->halted = cpu->cpu_index != 0;
> -    env->pc = s->entry;
> -    env->npc = s->entry + 4;
> +    env->pc = LEON3_PROM_OFFSET;
> +    env->npc = LEON3_PROM_OFFSET + 4;
>   }
>
>   static void leon3_cache_control_int(CPUSPARCState *env)
> @@ -259,7 +248,7 @@ static void leon3_generic_hw_init(MachineState *machine)
>       ram_addr_t ram_size = machine->ram_size;
>       const char *bios_name = machine->firmware ?: LEON3_PROM_FILENAME;
>       const char *kernel_filename = machine->kernel_filename;
> -    SPARCCPU *cpu;
> +    SPARCCPU *cpu[MAX_CPUS];
>       CPUSPARCState   *env;
>       MemoryRegion *address_space_mem = get_system_memory();
>       MemoryRegion *prom = g_new(MemoryRegion, 1);
> @@ -267,28 +256,22 @@ static void leon3_generic_hw_init(MachineState
> *machine)
>       char       *filename;
>       int         bios_size;
>       int         prom_size;
> -    ResetData  *reset_info;
>       DeviceState *dev, *irqmpdev;
>       int i;
>       AHBPnp *ahb_pnp;
>       APBPnp *apb_pnp;
>
> -    reset_info = g_malloc0(sizeof(ResetData));
> -
>       for (i = 0; i < machine->smp.cpus; i++) {
>           /* Init CPU */
> -        cpu = SPARC_CPU(object_new(machine->cpu_type));
> -        qdev_init_gpio_in_named(DEVICE(cpu), leon3_start_cpu,
> "start_cpu", 1);
> -        qdev_init_gpio_in_named(DEVICE(cpu), leon3_set_pil_in, "pil", 1);
> -        qdev_realize(DEVICE(cpu), NULL, &error_fatal);
> -        env = &cpu->env;
> +        cpu[i] = SPARC_CPU(object_new(machine->cpu_type));
> +        qdev_init_gpio_in_named(DEVICE(cpu[i]), leon3_start_cpu,
> "start_cpu", 1);
> +        qdev_init_gpio_in_named(DEVICE(cpu[i]), leon3_set_pil_in,
> "pil", 1);
> +        qdev_realize(DEVICE(cpu[i]), NULL, &error_fatal);
> +        env = &cpu[i]->env;
>
>           cpu_sparc_set_id(env, i);
>
> -        /* Reset data */
> -        reset_info->info[i].id = i;
> -        reset_info->info[i].cpu = cpu;
> -        qemu_register_reset(leon3_cpu_reset, &reset_info->info[i]);
> +        qemu_register_reset(leon3_cpu_reset, cpu[i]);
>       }
>
>       ahb_pnp = GRLIB_AHB_PNP(qdev_new(TYPE_GRLIB_AHB_PNP));
> @@ -312,13 +295,12 @@ static void leon3_generic_hw_init(MachineState
> *machine)
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);
>
>       for (i = 0; i < machine->smp.cpus; i++) {
> -        cpu = reset_info->info[i].cpu;
> -        env = &cpu->env;
> +        env = &cpu[i]->env;
>           qdev_connect_gpio_out_named(irqmpdev, "grlib-start-cpu", i,
> -                                    qdev_get_gpio_in_named(DEVICE(cpu),
> +                                    qdev_get_gpio_in_named(DEVICE(cpu[i]),
>
> "start_cpu", 0));
>           qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", i,
> -                                    qdev_get_gpio_in_named(DEVICE(cpu),
> +                                    qdev_get_gpio_in_named(DEVICE(cpu[i]),
>                                                              "pil", 0));
>           env->irq_manager = irqmpdev;
>           env->qemu_irq_ack = leon3_irq_manager;
> @@ -396,11 +378,6 @@ static void leon3_generic_hw_init(MachineState
> *machine)
>                * bootloader.
>                */
>               write_bootloader(memory_region_get_ram_ptr(prom), entry);
> -            reset_info->entry = LEON3_PROM_OFFSET;
> -            for (i = 0; i < machine->smp.cpus; i++) {
> -                reset_info->info[i].cpu->env.pc = LEON3_PROM_OFFSET;
> -                reset_info->info[i].cpu->env.npc = LEON3_PROM_OFFSET + 4;
> -            }
>           }
>       }
> ---
>
> Regards,
>
> Phil.
Re: [PATCH 3/3] meson: Enable -Wvla
Posted by Philippe Mathieu-Daudé 9 months, 1 week ago
On 21/2/24 18:27, Philippe Mathieu-Daudé wrote:

> Clément, ResetData::entry isn't used, so we can simplify removing
> the whole ResetData structure, but I'm not sure this is intended:
> 
> -- >8 --
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 4873b59b6c..1ff6b5d63d 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -68,14 +68,6 @@
>   #define LEON3_APB_PNP_OFFSET (0x800FF000)
>   #define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
> 
> -typedef struct ResetData {
> -    struct CPUResetData {
> -        int id;
> -        SPARCCPU *cpu;
> -    } info[MAX_CPUS];
> -    uint32_t entry;             /* save kernel entry in case of reset */
> -} ResetData;
> -
>   static uint32_t *gen_store_u32(uint32_t *code, hwaddr addr, uint32_t val)
>   {
>       stl_p(code++, 0x82100000); /* mov %g0, %g1                */
> @@ -148,17 +140,14 @@ static void write_bootloader(void *ptr, hwaddr 
> kernel_addr)
> 
>   static void leon3_cpu_reset(void *opaque)
>   {
> -    struct CPUResetData *info = (struct CPUResetData *) opaque;
> -    int id = info->id;
> -    ResetData *s = (ResetData *)DO_UPCAST(ResetData, info[id], info);
> -    CPUState *cpu = CPU(s->info[id].cpu);
> +    CPUState *cpu = opaque;
>       CPUSPARCState *env = cpu_env(cpu);
> 
>       cpu_reset(cpu);
> 
>       cpu->halted = cpu->cpu_index != 0;
> -    env->pc = s->entry;
> -    env->npc = s->entry + 4;
> +    env->pc = LEON3_PROM_OFFSET;
> +    env->npc = LEON3_PROM_OFFSET + 4;
>   }
> 
>   static void leon3_cache_control_int(CPUSPARCState *env)
> @@ -259,7 +248,7 @@ static void leon3_generic_hw_init(MachineState 
> *machine)
>       ram_addr_t ram_size = machine->ram_size;
>       const char *bios_name = machine->firmware ?: LEON3_PROM_FILENAME;
>       const char *kernel_filename = machine->kernel_filename;
> -    SPARCCPU *cpu;
> +    SPARCCPU *cpu[MAX_CPUS];
>       CPUSPARCState   *env;
>       MemoryRegion *address_space_mem = get_system_memory();
>       MemoryRegion *prom = g_new(MemoryRegion, 1);
> @@ -267,28 +256,22 @@ static void leon3_generic_hw_init(MachineState 
> *machine)
>       char       *filename;
>       int         bios_size;
>       int         prom_size;
> -    ResetData  *reset_info;
>       DeviceState *dev, *irqmpdev;
>       int i;
>       AHBPnp *ahb_pnp;
>       APBPnp *apb_pnp;
> 
> -    reset_info = g_malloc0(sizeof(ResetData));
> -
>       for (i = 0; i < machine->smp.cpus; i++) {
>           /* Init CPU */
> -        cpu = SPARC_CPU(object_new(machine->cpu_type));
> -        qdev_init_gpio_in_named(DEVICE(cpu), leon3_start_cpu, 
> "start_cpu", 1);
> -        qdev_init_gpio_in_named(DEVICE(cpu), leon3_set_pil_in, "pil", 1);
> -        qdev_realize(DEVICE(cpu), NULL, &error_fatal);
> -        env = &cpu->env;
> +        cpu[i] = SPARC_CPU(object_new(machine->cpu_type));
> +        qdev_init_gpio_in_named(DEVICE(cpu[i]), leon3_start_cpu, 
> "start_cpu", 1);
> +        qdev_init_gpio_in_named(DEVICE(cpu[i]), leon3_set_pil_in, 
> "pil", 1);
> +        qdev_realize(DEVICE(cpu[i]), NULL, &error_fatal);
> +        env = &cpu[i]->env;
> 
>           cpu_sparc_set_id(env, i);
> 
> -        /* Reset data */
> -        reset_info->info[i].id = i;
> -        reset_info->info[i].cpu = cpu;
> -        qemu_register_reset(leon3_cpu_reset, &reset_info->info[i]);
> +        qemu_register_reset(leon3_cpu_reset, cpu[i]);

           qemu_register_reset(leon3_cpu_reset, CPU(cpu[i]));

>       }
> 
>       ahb_pnp = GRLIB_AHB_PNP(qdev_new(TYPE_GRLIB_AHB_PNP));
> @@ -312,13 +295,12 @@ static void leon3_generic_hw_init(MachineState 
> *machine)
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);
> 
>       for (i = 0; i < machine->smp.cpus; i++) {
> -        cpu = reset_info->info[i].cpu;
> -        env = &cpu->env;
> +        env = &cpu[i]->env;
>           qdev_connect_gpio_out_named(irqmpdev, "grlib-start-cpu", i,
> -                                    qdev_get_gpio_in_named(DEVICE(cpu),
> +                                    qdev_get_gpio_in_named(DEVICE(cpu[i]),
> 
> "start_cpu", 0));
>           qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", i,
> -                                    qdev_get_gpio_in_named(DEVICE(cpu),
> +                                    qdev_get_gpio_in_named(DEVICE(cpu[i]),
>                                                              "pil", 0));
>           env->irq_manager = irqmpdev;
>           env->qemu_irq_ack = leon3_irq_manager;
> @@ -396,11 +378,6 @@ static void leon3_generic_hw_init(MachineState 
> *machine)
>                * bootloader.
>                */
>               write_bootloader(memory_region_get_ram_ptr(prom), entry);
> -            reset_info->entry = LEON3_PROM_OFFSET;
> -            for (i = 0; i < machine->smp.cpus; i++) {
> -                reset_info->info[i].cpu->env.pc = LEON3_PROM_OFFSET;
> -                reset_info->info[i].cpu->env.npc = LEON3_PROM_OFFSET + 4;
> -            }
>           }
>       }
> ---
> 
> Regards,
> 
> Phil.