From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VqLHs-00020q-Od for qemu-devel@nongnu.org; Tue, 10 Dec 2013 06:15:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VqLHm-0007q8-Fr for qemu-devel@nongnu.org; Tue, 10 Dec 2013 06:15:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:16969) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VqLHl-0007py-Ve for qemu-devel@nongnu.org; Tue, 10 Dec 2013 06:15:30 -0500 Date: Tue, 10 Dec 2013 12:15:17 +0100 From: Igor Mammedov Message-ID: <20131210121517.5646870d@thinkpad> In-Reply-To: <1381762577-12526-43-git-send-email-mst@redhat.com> References: <1381762577-12526-1-git-send-email-mst@redhat.com> <1381762577-12526-43-git-send-email-mst@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 42/43] piix4: add acpi pci hotplug support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: peter.maydell@linaro.org, marcel.a@redhat.com, qemu-devel@nongnu.org, Anthony Liguori , kraxel@redhat.com On Mon, 14 Oct 2013 18:01:20 +0300 "Michael S. Tsirkin" wrote: > Add support for acpi pci hotplug using the > new infrastructure. > PIIX4 legacy interface is maintained as is for > machine types 1.6 and older. > > Signed-off-by: Michael S. Tsirkin > --- > include/hw/i386/pc.h | 5 ++++ > hw/acpi/piix4.c | 75 +++++++++++++++++++++++++++++++++++++++++++++------- > 2 files changed, 70 insertions(+), 10 deletions(-) > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 39db8cb..6865972 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -249,6 +249,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > > #define PC_COMPAT_1_6 \ > {\ > + .driver = "PIIX4_PM",\ > + .property = "acpi-pci-hotplug-with-bridge-support",\ > + .value = "off",\ > + }, \ > + {\ > .driver = "e1000",\ > .property = "mitigation",\ > .value = "off",\ > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 3bcd890..d516033 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -30,6 +30,7 @@ > #include "hw/nvram/fw_cfg.h" > #include "exec/address-spaces.h" > #include "hw/acpi/piix4.h" > +#include "hw/acpi/pcihp.h" > > //#define DEBUG > > @@ -73,7 +74,6 @@ typedef struct PIIX4PMState { > uint32_t io_base; > > MemoryRegion io_gpe; > - MemoryRegion io_pci; > MemoryRegion io_cpu; > ACPIREGS ar; > > @@ -88,11 +88,16 @@ typedef struct PIIX4PMState { > Notifier machine_ready; > Notifier powerdown_notifier; > > - /* for pci hotplug */ > + /* for legacy pci hotplug (compatible with qemu 1.6 and older) */ > + MemoryRegion io_pci; > struct pci_status pci0_status; > uint32_t pci0_hotplug_enable; > uint32_t pci0_slot_device_present; > > + /* for new pci hotplug (with PCI2PCI bridge support) */ > + AcpiPciHpState acpi_pci_hotplug; pcihp therm collides with linux's pcihp driver that is used with native PCIE hotplug, which makes code a bit confusing at first. perhaps replacing "pcihp" through out with more verbose "pci_hotplug" would be better? > + bool use_acpi_pci_hotplug; > + > uint8_t disable_s3; > uint8_t disable_s4; > uint8_t s4_val; > @@ -282,6 +287,18 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int version_id) > return ret; > } > > +static bool vmstate_test_use_acpi_pci_hotplug(void *opaque, int version_id) > +{ > + PIIX4PMState *s = opaque; > + return s->use_acpi_pci_hotplug; > +} > + > +static bool vmstate_test_no_use_acpi_pci_hotplug(void *opaque, int version_id) > +{ > + PIIX4PMState *s = opaque; > + return !s->use_acpi_pci_hotplug; > +} > + > /* qemu-kvm 1.2 uses version 3 but advertised as 2 > * To support incoming qemu-kvm 1.2 migration, change version_id > * and minimum_version_id to 2 below (which breaks migration from > @@ -304,8 +321,12 @@ static const VMStateDescription vmstate_acpi = { > VMSTATE_TIMER(ar.tmr.timer, PIIX4PMState), > VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState), > VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE), > - VMSTATE_STRUCT(pci0_status, PIIX4PMState, 2, vmstate_pci_status, > - struct pci_status), > + VMSTATE_STRUCT_TEST(pci0_status, PIIX4PMState, > + vmstate_test_no_use_acpi_pci_hotplug, > + 2, vmstate_pci_status, > + struct pci_status), > + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState, > + vmstate_test_use_acpi_pci_hotplug), > VMSTATE_END_OF_LIST() > } > }; > @@ -383,7 +404,11 @@ static void piix4_reset(void *opaque) > pci_conf[0x5B] = 0x02; > } > pm_io_space_update(s); > - piix4_update_hotplug(s); > + if (s->use_acpi_pci_hotplug) { > + acpi_pcihp_reset(&s->acpi_pci_hotplug); > + } else { > + piix4_update_hotplug(s); > + } > } > > static void piix4_pm_powerdown_req(Notifier *n, void *opaque) > @@ -394,6 +419,26 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) > acpi_pm1_evt_power_down(&s->ar); > } > > +static int piix4_acpi_pci_hotplug(DeviceState *qdev, PCIDevice *dev, > + PCIHotplugState state) > +{ > + PIIX4PMState *s = PIIX4_PM(qdev); > + int ret = acpi_pcihp_device_hotplug(&s->acpi_pci_hotplug, dev, state); > + if (ret < 0) { > + return ret; > + } > + s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS; > + > + pm_update_sci(s); > + return 0; > +} > + > +static void piix4_update_bus_hotplug(PCIBus *bus, void *opaque) > +{ > + PIIX4PMState *s = opaque; > + pci_bus_hotplug(bus, piix4_acpi_pci_hotplug, DEVICE(s)); > +} > + > static void piix4_pm_machine_ready(Notifier *n, void *opaque) > { > PIIX4PMState *s = container_of(n, PIIX4PMState, machine_ready); > @@ -407,6 +452,10 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque) > pci_conf[0x63] = 0x60; > pci_conf[0x67] = (memory_region_present(io_as, 0x3f8) ? 0x08 : 0) | > (memory_region_present(io_as, 0x2f8) ? 0x90 : 0); > + > + if (s->use_acpi_pci_hotplug) { > + pci_for_each_bus(d->bus, piix4_update_bus_hotplug, s); why do you need to do it dinamically? > + } > } > > static void piix4_pm_add_propeties(PIIX4PMState *s) > @@ -528,6 +577,8 @@ static Property piix4_pm_properties[] = { > DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0), > DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState, disable_s4, 0), > DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2), > + DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState, > + use_acpi_pci_hotplug, true), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -716,11 +767,15 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, > "acpi-gpe0", GPE_LEN); > memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe); > > - memory_region_init_io(&s->io_pci, OBJECT(s), &piix4_pci_ops, s, > - "acpi-pci-hotplug", PCI_HOTPLUG_SIZE); > - memory_region_add_subregion(parent, PCI_HOTPLUG_ADDR, > - &s->io_pci); > - pci_bus_hotplug(bus, piix4_device_hotplug, DEVICE(s)); > + if (s->use_acpi_pci_hotplug) { > + acpi_pcihp_init(&s->acpi_pci_hotplug, bus, parent); > + } else { > + memory_region_init_io(&s->io_pci, OBJECT(s), &piix4_pci_ops, s, > + "acpi-pci-hotplug", PCI_HOTPLUG_SIZE); > + memory_region_add_subregion(parent, PCI_HOTPLUG_ADDR, > + &s->io_pci); > + pci_bus_hotplug(bus, piix4_device_hotplug, DEVICE(s)); > + } > > CPU_FOREACH(cpu) { > CPUClass *cc = CPU_GET_CLASS(cpu); > -- > MST > > -- Regards, Igor