Re: [Qemu-devel] [PATCH 10/21] acpi_piix4: add infrastructure to send CPU hot-plug GPE to guest

[Juan Quintela]

Igor Mammedov <address@hidden> wrote:
> * introduce processor status bitmask visible to guest at 0xaf00 addr,
>   where ACPI asl code expects it
> * set bit corresponding to APIC ID in processor status bitmask on
>   receiving CPU hot-plug notification
> * trigger CPU hot-plug SCI, to notify guest about CPU hot-plug event
>
> Signed-off-by: Igor Mammedov <address@hidden>

This is wrong (or at least supperfluous)

> +static int piix4_init_cpu_status(Object *obj, void *opaque)
> +{
> +    struct cpu_status *g = (struct cpu_status *)opaque;
> +    Object *cpu_obj = object_dynamic_cast(obj, TYPE_CPU);
> +
> +    if (cpu_obj) {
> +        struct Error *error = NULL;

we set error to NULL

> +        CPUClass *k = CPU_GET_CLASS(cpu_obj);
> +        int64_t id = k->get_arch_id(CPU(cpu_obj));
> +
> +        if (error) {

and without touching error we test if it is != NULL.


something is missing here?

> +            fprintf(stderr, "failed to initilize CPU status for ACPI: %s\n",
> +                    error_get_pretty(error));
> +            error_free(error);
> +            abort();

Can't we return an error code at this point?  I guess no,  but asking
will not hurt.

> +        }
> +        g_assert((id / 8) < PIIX4_PROC_LEN);
> +        g->sts[id / 8] |= (1 << (id % 8));
> +    }
> +    return object_child_foreach(obj, piix4_init_cpu_status, opaque);
> +}
> +
>  static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
>                                  PCIHotplugState state);
>  
> @@ -600,6 +704,13 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion 
> *parent,
>      memory_region_add_subregion(parent, PCI_HOTPLUG_ADDR,
>                                  &s->io_pci);
>      pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
> +
> +    piix4_init_cpu_status(qdev_get_machine(), &s->gpe_cpu);
> +    memory_region_init_io(&s->io_cpu, &cpu_hotplug_ops, s, 
> "apci-cpu-hotplug",
> +                          PIIX4_PROC_LEN);
> +    memory_region_add_subregion(parent, PIIX4_PROC_BASE, &s->io_cpu);
> +    s->cpu_added_notifier.notify = piix4_cpu_added_req;
> +    qemu_register_cpu_added_notifier(&s->cpu_added_notifier);
>  }
>  
>  static void enable_device(PIIX4PMState *s, int slot)