From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33249) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUdVu-0004FH-6J for qemu-devel@nongnu.org; Tue, 23 Apr 2013 09:44:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UUdVr-0007gw-Rv for qemu-devel@nongnu.org; Tue, 23 Apr 2013 09:44:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48235) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUdVr-0007gc-JW for qemu-devel@nongnu.org; Tue, 23 Apr 2013 09:44:03 -0400 From: Juan Quintela In-Reply-To: <1366705795-24732-11-git-send-email-imammedo@redhat.com> (Igor Mammedov's message of "Tue, 23 Apr 2013 10:29:44 +0200") References: <1366705795-24732-1-git-send-email-imammedo@redhat.com> <1366705795-24732-11-git-send-email-imammedo@redhat.com> Date: Tue, 23 Apr 2013 15:43:51 +0200 Message-ID: <87txmxuzu0.fsf@elfo.elfo> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 10/21] acpi_piix4: add infrastructure to send CPU hot-plug GPE to guest Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: peter.maydell@linaro.org, gleb@redhat.com, mst@redhat.com, jan.kiszka@siemens.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, blauwirbel@gmail.com, kraxel@redhat.com, armbru@redhat.com, yang.z.zhang@intel.com, ehabkost@redhat.com, stefano.stabellini@eu.citrix.com, aderumier@odiso.com, anthony.perard@citrix.com, alex.williamson@redhat.com, rth@twiddle.net, kwolf@redhat.com, aliguori@us.ibm.com, claudio.fontana@huawei.com, pbonzini@redhat.com, afaerber@suse.de Igor Mammedov 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 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)