On Tue, 23 Apr 2013 15:43:51 +0200
Juan Quintela <quint...@redhat.com> wrote:
> Igor Mammedov <imamm...@redhat.com> 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 <imamm...@redhat.com>
>
> 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?
It's leftover from previous versions, all error handing should in this
function should be dropped, leaving only assert().
>
> > + 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)
>
