From patchwork Fri Feb 24 23:21:08 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 142997 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 72A93B6EE7 for ; Sat, 25 Feb 2012 10:21:22 +1100 (EST) Received: from localhost ([::1]:33491 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S14S0-0007KT-CG for incoming@patchwork.ozlabs.org; Fri, 24 Feb 2012 18:21:20 -0500 Received: from eggs.gnu.org ([208.118.235.92]:41273) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S14Rt-0007KD-K5 for qemu-devel@nongnu.org; Fri, 24 Feb 2012 18:21:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S14Rr-0007Up-Q4 for qemu-devel@nongnu.org; Fri, 24 Feb 2012 18:21:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:64320) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S14Rr-0007Uj-FE for qemu-devel@nongnu.org; Fri, 24 Feb 2012 18:21:11 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q1ONL9U4014396 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 24 Feb 2012 18:21:09 -0500 Received: from bling.home ([10.3.113.14]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q1ONL8Z3029798; Fri, 24 Feb 2012 18:21:08 -0500 From: Alex Williamson To: qemu-devel@nongnu.org Date: Fri, 24 Feb 2012 16:21:08 -0700 Message-ID: <20120224231833.17817.98736.stgit@bling.home> User-Agent: StGIT/0.14.3 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: alex.williamson@redhat.com, ddutile@redhat.com, qemu-devel@nongnu.org, gleb@redhat.com, mst@redhat.com Subject: [Qemu-devel] [PATCH] acpi_piix4: Add _STA support and use it to acknowledge hotplug X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Our current PCI hotplug model assumes that guests are hotplug capable. Adding a device always works and we can ask for it back with device_del. This isn't always the case. If the guest doesn't support hotplug, adding a device renders the device lost until the guest is shut down. This is particularly annoying when the device is an assigned device. The Status method (_STA) is a standard way to evaluate a device in ACPI. By adding this to the slot object, we get a nice interaction point where we can determine whether the OSPM is responding to a PCI hotplug. If the OSPM does not trigger this callback in response to a hot add, we can assume it is not hotplug capable, clear the pending add, and allow device_del to skip the ACPI path and remove the device directly. In addition to non-hotplug aware guests, this also enables regression testing where the remove is issued before the guest is able to respond to the add request. With a small enough delay between add/del, the device is removed without the guest ever touching it. We don't currently have paths that make use of guest write of the up/down fields or guest read of the eject field, so we can re-use these and avoid adding new I/O port regions for PCI hotplug. Write to the "up" port becomes an acknowledgment of the up bit and thus clears it. Read from the "eject" port becomes a bitmap of present slots allowing _STA to determine it's return value. Write to the "down" port is unused. _STA is also evaluated when ACPI namespace is initialized by the guest. Using this, we can determine if the guest is running a bios with _STA support and only enable support when it is. This allows new qemu to work with old bios. New bios code is also able to probe for this support by testing whether the "present" register reports any slots with present devices. If not, it can dynamically disable _STA, triggering the above compatibility support. Signed-off-by: Alex Williamson --- hw/acpi_piix4.c | 177 +++++++++++++++++++++++++++++++++++++------------------ 1 files changed, 118 insertions(+), 59 deletions(-) diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 30d37f9..ea568b8 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -41,9 +41,10 @@ #define GPE_BASE 0xafe0 #define PROC_BASE 0xaf00 #define GPE_LEN 4 -#define PCI_BASE 0xae00 -#define PCI_EJ_BASE 0xae08 -#define PCI_RMV_BASE 0xae0c +#define PCI_UP_ACK 0xae00 +#define PCI_DOWN 0xae04 +#define PCI_PRES_EJ 0xae08 +#define PCI_HOTPLUG 0xae0c #define PIIX4_CPU_HOTPLUG_STATUS 4 #define PIIX4_PCI_HOTPLUG_STATUS 2 @@ -80,6 +81,7 @@ typedef struct PIIX4PMState { struct gpe_regs gpe_cpu; struct pci_status pci0_status; uint32_t pci0_hotplug_enable; + bool bios_has_STA; } PIIX4PMState; static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s); @@ -256,6 +258,24 @@ static const VMStateDescription vmstate_pci_status = { } }; +static bool bios_has_STA_needed(void *opaque) +{ + PIIX4PMState *s = opaque; + + return s->bios_has_STA; +} + +static const VMStateDescription vmstate_bios_has_STA = { + .name = "piix4_pm/bios_has_STA", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField []) { + VMSTATE_BOOL(bios_has_STA, PIIX4PMState), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_acpi = { .name = "piix4_pm", .version_id = 2, @@ -274,6 +294,14 @@ static const VMStateDescription vmstate_acpi = { VMSTATE_STRUCT(pci0_status, PIIX4PMState, 2, vmstate_pci_status, struct pci_status), VMSTATE_END_OF_LIST() + }, + .subsections = (VMStateSubsection []) { + { + .vmsd = &vmstate_bios_has_STA, + .needed = bios_has_STA_needed, + }, { + /* empty */ + } } }; @@ -472,49 +500,45 @@ static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val) PIIX4_DPRINTF("gpe write %x <== %d\n", addr, val); } -static uint32_t pcihotplug_read(void *opaque, uint32_t addr) +static uint32_t pci_up_read(void *opaque, uint32_t addr) { - uint32_t val = 0; - struct pci_status *g = opaque; - switch (addr) { - case PCI_BASE: - val = g->up; - break; - case PCI_BASE + 4: - val = g->down; - break; - default: - break; - } + PIIX4PMState *s = opaque; + uint32_t val; + + val = s->pci0_status.up; - PIIX4_DPRINTF("pcihotplug read %x == %x\n", addr, val); + PIIX4_DPRINTF("pci up read %x\n", val); return val; } -static void pcihotplug_write(void *opaque, uint32_t addr, uint32_t val) +static void pci_ack_write(void *opaque, uint32_t addr, uint32_t val) { - struct pci_status *g = opaque; - switch (addr) { - case PCI_BASE: - g->up = val; - break; - case PCI_BASE + 4: - g->down = val; - break; - } + PIIX4PMState *s = opaque; - PIIX4_DPRINTF("pcihotplug write %x <== %d\n", addr, val); + s->pci0_status.up &= ~val; + if (!s->bios_has_STA) { + s->bios_has_STA = true; + } + + PIIX4_DPRINTF("pci ack write %x\n", val); } -static uint32_t pciej_read(void *opaque, uint32_t addr) +static uint32_t pci_down_read(void *opaque, uint32_t addr) { - PIIX4_DPRINTF("pciej read %x\n", addr); - return 0; + PIIX4PMState *s = opaque; + uint32_t val; + + val = s->pci0_status.down; + + PIIX4_DPRINTF("pci down read %x\n", val); + return val; } -static void pciej_write(void *opaque, uint32_t addr, uint32_t val) +static void pci_ej_write(void *opaque, uint32_t addr, uint32_t val) { - BusState *bus = opaque; + PIIX4PMState *s = opaque; + PCIDevice *dev = &s->dev; + BusState *bus = qdev_get_parent_bus(&dev->qdev); DeviceState *qdev, *next; int slot = ffs(val) - 1; @@ -523,23 +547,35 @@ static void pciej_write(void *opaque, uint32_t addr, uint32_t val) PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); if (PCI_SLOT(dev->devfn) == slot && !pc->no_hotplug) { qdev_free(qdev); + s->pci0_status.down &= ~val; } } - - PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val); + PIIX4_DPRINTF("pci ej write %x\n", val); } - -static uint32_t pcirmv_read(void *opaque, uint32_t addr) +static uint32_t pci_present_read(void *opaque, uint32_t addr) { PIIX4PMState *s = opaque; + PCIDevice *dev = &s->dev; + BusState *bus = qdev_get_parent_bus(&dev->qdev); + DeviceState *qdev, *next; + uint32_t val = 0; - return s->pci0_hotplug_enable; + QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) { + PCIDevice *dev = PCI_DEVICE(qdev); + val |= (1 << PCI_SLOT(dev->devfn)); + } + + PIIX4_DPRINTF("pci present read %x\n", val); + + return val; } -static void pcirmv_write(void *opaque, uint32_t addr, uint32_t val) +static uint32_t pci_hotpluggable_read(void *opaque, uint32_t addr) { - return; + PIIX4PMState *s = opaque; + + return s->pci0_hotplug_enable; } extern const char *global_cpu_model; @@ -549,7 +585,6 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s) { - struct pci_status *pci0_status = &s->pci0_status; int i = 0, cpus = smp_cpus; while (cpus > 0) { @@ -564,14 +599,15 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s) register_ioport_write(PROC_BASE, 32, 1, gpe_writeb, s); register_ioport_read(PROC_BASE, 32, 1, gpe_readb, s); - register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, pci0_status); - register_ioport_read(PCI_BASE, 8, 4, pcihotplug_read, pci0_status); + register_ioport_read(PCI_UP_ACK, 4, 4, pci_up_read, s); + register_ioport_write(PCI_UP_ACK, 4, 4, pci_ack_write, s); - register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus); - register_ioport_read(PCI_EJ_BASE, 4, 4, pciej_read, bus); + register_ioport_read(PCI_DOWN, 4, 4, pci_down_read, s); - register_ioport_write(PCI_RMV_BASE, 4, 4, pcirmv_write, s); - register_ioport_read(PCI_RMV_BASE, 4, 4, pcirmv_read, s); + register_ioport_read(PCI_PRES_EJ, 4, 4, pci_present_read, s); + register_ioport_write(PCI_PRES_EJ, 4, 4, pci_ej_write, s); + + register_ioport_read(PCI_HOTPLUG, 4, 4, pci_hotpluggable_read, s); pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev); } @@ -618,16 +654,45 @@ void qemu_system_cpu_hot_add(int cpu, int state) } #endif -static void enable_device(PIIX4PMState *s, int slot) +static int enable_device(PIIX4PMState *s, int slot) { + uint32_t mask = 1U << slot; + + if ((s->pci0_status.up | s->pci0_status.down) & mask) { + return -1; + } + s->gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS; - s->pci0_status.up |= (1 << slot); + s->pci0_status.up |= mask; + + pm_update_sci(s); + return 0; } -static void disable_device(PIIX4PMState *s, int slot) +static int disable_device(PIIX4PMState *s, int slot) { + uint32_t mask = 1U << slot; + + if (s->pci0_status.up & mask) { + if (s->bios_has_STA) { + PIIX4_DPRINTF("slot %d never acknowledged, removing directly\n", + slot); + s->pci0_status.up &= ~mask; + pci_ej_write(s, 0, mask); + if (!(s->pci0_status.up | s->pci0_status.down)) { + s->gpe.sts[0] &= ~PIIX4_PCI_HOTPLUG_STATUS; + } + return 0; + } else { + s->pci0_status.up &= ~mask; + } + } + s->gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS; - s->pci0_status.down |= (1 << slot); + s->pci0_status.down |= mask; + + pm_update_sci(s); + return 0; } static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, @@ -644,15 +709,9 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, return 0; } - s->pci0_status.up = 0; - s->pci0_status.down = 0; if (state == PCI_HOTPLUG_ENABLED) { - enable_device(s, slot); + return enable_device(s, slot); } else { - disable_device(s, slot); + return disable_device(s, slot); } - - pm_update_sci(s); - - return 0; }