"Michael S. Tsirkin" <m...@redhat.com> writes: > This adds support for device hotplug behind > pci bridges. Bridge devices themselves need > to be pre-configured on qemu command line. > > One of the goals of this project was to > demonstrate the advantages of generating > ACPI tables in QEMU. > In my opinion, it does this successfully.
Since you've gone out of your way to make this difficult to actually review... > > * This saves the need to provide a new interface for firmware > to discover bus number to pci brige mapping Do you mean fw_cfg? The BIOS normally does bus numbering. I see no reason for it not to. > * No need for yet another interface to bios to detect qemu version > to check if it's safe to activate new code, > and to ship multiple table versions: We only care about supporting one version of SeaBIOS. SeaBIOS should only care about supporting one version of QEMU. We're not asking it to support multiple versions. > we generated code so we know this is new qemu > * Use of glib's GArray makes it much easier to build > up tables in code without need for iasl and code patching Adding a dynamic array to SeaBIOS isn't rocket science. > > I have also tried to address the concerns that > Anthony has raised with this approach, please see below. > > Design: > - each bus gets assigned a number 0-255 > - generated ACPI code writes this number > to a new BSEL register, then uses existing > UP/DOWN registers to probe slot status; > to eject, write number to BSEL register, > then slot into existing EJ > > This is to address the ACPI spec requirement to > avoid config cycle access to any bus except PCI roots. > > Portability: > - Non x86 (or any Linux) platforms don't need any of this code. > They can keep happily using SHPC the way > they always did. > > Things to note: > - this is on top of acpi patchset posted a month ago, > with a small patch adding a core function to walk all > pci buses, on top. > Can also be found in my git tree > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi > > - Extensive use of glib completely removes > pointer math in new code: we use > g_array_append_vals exclusively. > There's no code patching in new code. > This is in response to comments about security > concerns with adding code to qemu. > In this sense it is more secure than existing > code in hw/acpi/core.c - that can be switched to > glib if desired. > > - New code (not imported from seabios) > uses glib to reduce dependency on iasl. > With time all code can be rewritted to > use glib and avoid iasl, if desired. > > - As was the case previously, > systems that lack working iasl are detected at configure time, > pre-generated hex files in source tree are used in this case. > This addresses the concern with iasl/big-endian > systems. > > - Compile tested only. Migration is known to be > broken - there are a bunch of TODO tags in code. > It was agreed on the last qemu conf meeting that > this code is posted without looking at migration, > with understanding that it is addressed before > being merged. Please do not mistake this > limitation for a fundamental one - I have a > very good idea how to fix it, including > cross-version migration. > > - Cross version migration: when running with -M 1.5 > and older, all ACPI table generation should be disabled. > We'll present FW_CFG interface compatible with 1.5. So TL;DR, you break a bunch of stuff and introduce a mess of code. It would be less code and wouldn't break anything to add this logic to SeaBIOS. How is this even a discussion? Regards, Anthony Liguori > > Cc: Jordan Justen <jljus...@gmail.com> > Cc: Anthony Liguori <anth...@codemonkey.ws> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: seabios@seabios.org > Cc: David Woodhouse <dw...@infradead.org> > Cc: Kevin O'Connor <ke...@koconnor.net> > Cc: Peter Maydell <peter.mayd...@linaro.org> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > --- > docs/specs/acpi_pci_hotplug.txt | 8 + > include/hw/i386/pc.h | 4 +- > hw/i386/acpi-dsdt.dsl | 36 +++- > hw/i386/ssdt-pcihp.dsl | 51 ----- > hw/acpi/piix4.c | 145 ++++++++++---- > hw/i386/acpi-build.c | 411 > ++++++++++++++++++++++++++++++++++----- > hw/i386/Makefile.objs | 2 +- > hw/i386/ssdt-pcihp.hex.generated | 108 ---------- > 8 files changed, 510 insertions(+), 255 deletions(-) > delete mode 100644 hw/i386/ssdt-pcihp.dsl > delete mode 100644 hw/i386/ssdt-pcihp.hex.generated > > diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt > index a839434..951b99a 100644 > --- a/docs/specs/acpi_pci_hotplug.txt > +++ b/docs/specs/acpi_pci_hotplug.txt > @@ -43,3 +43,11 @@ PCI removability status (IO port 0xae0c-0xae0f, 4-byte > access): > > Used by ACPI BIOS _RMV method to indicate removability status to OS. One > bit per slot. Read-only > + > +PCI bus selector (IO port 0xae10-0xae13, 4-byte access): > +----------------------------------------------- > + > +Used by ACPI BIOS methods to select the current PCI bus. > +When written, makes all the other PCI registers (0xae00 - 0xae0f) > +to refer to the appropriate bus. > +0 selects PCI root bus (default). > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index f8d0871..66ec787 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -38,7 +38,9 @@ struct PcGuestInfo { > bool s3_disabled; > bool s4_disabled; > uint8_t s4_val; > - DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX); > +#define GUEST_INFO_MAX_HOTPLUG_BUS 256 > + PCIBus *hotplug_buses[GUEST_INFO_MAX_HOTPLUG_BUS]; > + DECLARE_BITMAP(hotplug_enable[GUEST_INFO_MAX_HOTPLUG_BUS], PCI_SLOT_MAX); > uint16_t sci_int; > uint8_t acpi_enable_cmd; > uint8_t acpi_disable_cmd; > diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl > index 90efce0..dc05668 100644 > --- a/hw/i386/acpi-dsdt.dsl > +++ b/hw/i386/acpi-dsdt.dsl > @@ -133,11 +133,18 @@ DefinitionBlock ( > B0EJ, 32, > } > > + OperationRegion(SEL, SystemIO, 0xae10, 0x04) > + Field(SEL, DWordAcc, NoLock, WriteAsZeros) { > + BSEL, 32, > + } > + > /* Methods called by bulk generated PCI devices below */ > + Name(BNUM, 0x00) > > /* Methods called by hotplug devices */ > - Method(PCEJ, 1, NotSerialized) { > + Method(PCEJ, 2, NotSerialized) { > // _EJ0 method - eject callback > + Store(Arg1, BSEL) > Store(ShiftLeft(1, Arg0), B0EJ) > Return (0x0) > } > @@ -147,16 +154,25 @@ DefinitionBlock ( > > /* PCI hotplug notify method */ > Method(PCNF, 0) { > - // Local0 = iterator > + // Local0 = bridge selector > Store(Zero, Local0) > - While (LLess(Local0, 31)) { > - Increment(Local0) > - If (And(PCIU, ShiftLeft(1, Local0))) { > - PCNT(Local0, 1) > - } > - If (And(PCID, ShiftLeft(1, Local0))) { > - PCNT(Local0, 3) > - } > + While (LLess(Local0, 255)) { > + Store(Local0, BSEL) > + // Local1 = slot iterator > + Store(Zero, Local1) > + > + Store(PCIU, Local2) > + Store(PCID, Local3) > + While (LLess(Local1, 31)) { > + Increment(Local1) > + If (And(Local2, ShiftLeft(1, Local1))) { > + PCNT(Add(Local1, Multiply(Local0, 32)), 1) > + } > + If (And(Local3, ShiftLeft(1, Local1))) { > + PCNT(Add(Local1, Multiply(Local0, 32)), 3) > + } > + } > + Increment(Local0) > } > } > } > diff --git a/hw/i386/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl > deleted file mode 100644 > index d29a5b9..0000000 > --- a/hw/i386/ssdt-pcihp.dsl > +++ /dev/null > @@ -1,51 +0,0 @@ > -/* > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License as published by > - * the Free Software Foundation; either version 2 of the License, or > - * (at your option) any later version. > - > - * This program is distributed in the hope that it will be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > - * GNU General Public License for more details. > - > - * You should have received a copy of the GNU General Public License along > - * with this program; if not, see <http://www.gnu.org/licenses/>. > - */ > - > -ACPI_EXTRACT_ALL_CODE ssdp_pcihp_aml > - > -DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1) > -{ > - > -/**************************************************************** > - * PCI hotplug > - ****************************************************************/ > - > - /* Objects supplied by DSDT */ > - External(\_SB.PCI0, DeviceObj) > - External(\_SB.PCI0.PCEJ, MethodObj) > - > - Scope(\_SB.PCI0) { > - > - /* Bulk generated PCI hotplug devices */ > - ACPI_EXTRACT_DEVICE_START ssdt_pcihp_start > - ACPI_EXTRACT_DEVICE_END ssdt_pcihp_end > - ACPI_EXTRACT_DEVICE_STRING ssdt_pcihp_name > - > - // Method _EJ0 can be patched by BIOS to EJ0_ > - // at runtime, if the slot is detected to not support hotplug. > - // Extract the offset of the address dword and the > - // _EJ0 name to allow this patching. > - Device(SAA) { > - ACPI_EXTRACT_NAME_BYTE_CONST ssdt_pcihp_id > - Name(_SUN, 0xAA) > - ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcihp_adr > - Name(_ADR, 0xAA0000) > - ACPI_EXTRACT_METHOD_STRING ssdt_pcihp_ej0 > - Method(_EJ0, 1) { > - Return (PCEJ(_SUN)) > - } > - } > - } > -} > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 19a26f5..10106fd 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -29,6 +29,7 @@ > #include "exec/ioport.h" > #include "hw/nvram/fw_cfg.h" > #include "exec/address-spaces.h" > +#include "hw/pci/pci_bus.h" > > //#define DEBUG > > @@ -42,11 +43,12 @@ > #define GPE_LEN 4 > > #define PCI_HOTPLUG_ADDR 0xae00 > -#define PCI_HOTPLUG_SIZE 0x000f > +#define PCI_HOTPLUG_SIZE 0x0014 > #define PCI_UP_BASE 0xae00 > #define PCI_DOWN_BASE 0xae04 > #define PCI_EJ_BASE 0xae08 > #define PCI_RMV_BASE 0xae0c > +#define PCI_SEL_BASE 0xae10 > > #define PIIX4_PROC_BASE 0xaf00 > #define PIIX4_PROC_LEN 32 > @@ -57,6 +59,8 @@ > struct pci_status { > uint32_t up; /* deprecated, maintained for migration compatibility */ > uint32_t down; > + uint32_t hotplug_enable; > + uint32_t device_present; > }; > > typedef struct CPUStatus { > @@ -84,9 +88,8 @@ typedef struct PIIX4PMState { > Notifier powerdown_notifier; > > /* for pci hotplug */ > - struct pci_status pci0_status; > - uint32_t pci0_hotplug_enable; > - uint32_t pci0_slot_device_present; > + struct pci_status pci_status[GUEST_INFO_MAX_HOTPLUG_BUS]; > + uint32_t hotplug_select; > > uint8_t disable_s3; > uint8_t disable_s4; > @@ -98,6 +101,17 @@ typedef struct PIIX4PMState { > PcGuestInfo *guest_info; > } PIIX4PMState; > > +static int piix4_find_hotplug_bus(PcGuestInfo *guest_info, PCIBus *bus) > +{ > + int i; > + for (i = 0; i < GUEST_INFO_MAX_HOTPLUG_BUS; ++i) { > + if (guest_info->hotplug_buses[i] == bus) { > + return i; > + } > + } > + return -1; > +} > + > static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, > PCIBus *bus, PIIX4PMState *s); > > @@ -183,6 +197,8 @@ static void pm_write_config(PCIDevice *d, > > static void vmstate_pci_status_pre_save(void *opaque) > { > +#if 0 > + /* TODO */ > struct pci_status *pci0_status = opaque; > PIIX4PMState *s = container_of(pci0_status, PIIX4PMState, pci0_status); > > @@ -190,6 +206,7 @@ static void vmstate_pci_status_pre_save(void *opaque) > * to a version that still does... of course these might get lost > * by an old buggy implementation, but we try. */ > pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable; > +#endif > } > > static int vmstate_acpi_post_load(void *opaque, int version_id) > @@ -267,7 +284,10 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int > version_id) > qemu_get_be16s(f, &temp); > } > > +#if 0 > + TODO > ret = vmstate_load_state(f, &vmstate_pci_status, &s->pci0_status, 1); > +#endif > return ret; > } > > @@ -293,21 +313,28 @@ 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), > +#if 0 > VMSTATE_STRUCT(pci0_status, PIIX4PMState, 2, vmstate_pci_status, > struct pci_status), > +#endif > VMSTATE_END_OF_LIST() > } > }; > > -static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots) > +static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned bsel, unsigned > slots) > { > BusChild *kid, *next; > - BusState *bus = qdev_get_parent_bus(&s->dev.qdev); > + BusState *bus; > int slot = ffs(slots) - 1; > bool slot_free = true; > > + if (!s->guest_info->hotplug_buses[bsel]) { > + return; > + } > + bus = &s->guest_info->hotplug_buses[bsel]->qbus; > + > /* Mark request as complete */ > - s->pci0_status.down &= ~(1U << slot); > + s->pci_status[bsel].down &= ~(1U << slot); > > QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) { > DeviceState *qdev = kid->child; > @@ -322,23 +349,26 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, > unsigned slots) > } > } > if (slot_free) { > - s->pci0_slot_device_present &= ~(1U << slot); > + s->pci_status[bsel].device_present &= ~(1U << slot); > } > } > > -static void piix4_update_hotplug(PIIX4PMState *s) > +static void piix4_update_hotplug(PIIX4PMState *s, unsigned bsel) > { > - PCIDevice *dev = &s->dev; > - BusState *bus = qdev_get_parent_bus(&dev->qdev); > BusChild *kid, *next; > + BusState *bus; > > + if (!s->guest_info->hotplug_buses[bsel]) { > + return; > + } > + bus = &s->guest_info->hotplug_buses[bsel]->qbus; > /* Execute any pending removes during reset */ > - while (s->pci0_status.down) { > - acpi_piix_eject_slot(s, s->pci0_status.down); > + while (s->pci_status[bsel].down) { > + acpi_piix_eject_slot(s, bsel, s->pci_status[bsel].down); > } > > - s->pci0_hotplug_enable = ~0; > - s->pci0_slot_device_present = 0; > + s->pci_status[bsel].hotplug_enable = ~0; > + s->pci_status[bsel].device_present = 0; > > QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) { > DeviceState *qdev = kid->child; > @@ -347,10 +377,10 @@ static void piix4_update_hotplug(PIIX4PMState *s) > int slot = PCI_SLOT(pdev->devfn); > > if (pc->no_hotplug) { > - s->pci0_hotplug_enable &= ~(1U << slot); > + s->pci_status[bsel].hotplug_enable &= ~(1U << slot); > } > > - s->pci0_slot_device_present |= (1U << slot); > + s->pci_status[bsel].device_present |= (1U << slot); > } > } > > @@ -358,6 +388,7 @@ static void piix4_reset(void *opaque) > { > PIIX4PMState *s = opaque; > uint8_t *pci_conf = s->dev.config; > + int i; > > pci_conf[0x58] = 0; > pci_conf[0x59] = 0; > @@ -371,7 +402,9 @@ static void piix4_reset(void *opaque) > /* Mark SMM as already inited (until KVM supports SMM). */ > pci_conf[0x5B] = 0x02; > } > - piix4_update_hotplug(s); > + for (i = 0; i < GUEST_INFO_MAX_HOTPLUG_BUS; ++i) { > + piix4_update_hotplug(s, i); > + } > } > > static void piix4_pm_powerdown_req(Notifier *n, void *opaque) > @@ -382,14 +415,20 @@ static void piix4_pm_powerdown_req(Notifier *n, void > *opaque) > acpi_pm1_evt_power_down(&s->ar); > } > > -static void piix4_update_guest_info(PIIX4PMState *s) > +static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, > + PCIHotplugState state); > + > +static void piix4_update_bus_guest_info(PCIBus *pci_bus, void *opaque) > { > - PCIDevice *dev = &s->dev; > - BusState *bus = qdev_get_parent_bus(&dev->qdev); > + PIIX4PMState *s = opaque; > + PcGuestInfo *guest_info = s->guest_info; > BusChild *kid, *next; > + BusState *bus = &pci_bus->qbus; > > - memset(s->guest_info->slot_hotplug_enable, 0xff, > - DIV_ROUND_UP(PCI_SLOT_MAX, BITS_PER_BYTE)); > + int i = piix4_find_hotplug_bus(guest_info, NULL); > + assert(i >= 0); > + > + guest_info->hotplug_buses[i] = pci_bus; > > QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) { > DeviceState *qdev = kid->child; > @@ -398,9 +437,22 @@ static void piix4_update_guest_info(PIIX4PMState *s) > int slot = PCI_SLOT(pdev->devfn); > > if (pc->no_hotplug) { > - clear_bit(slot, s->guest_info->slot_hotplug_enable); > + clear_bit(slot, guest_info->hotplug_enable[i]); > } > } > + > + pci_bus_hotplug(pci_bus, piix4_device_hotplug, &s->dev.qdev); > +} > + > +static void piix4_update_guest_info(PIIX4PMState *s) > +{ > + PCIDevice *dev = &s->dev; > + > + memset(s->guest_info->hotplug_enable, 0xff, > + sizeof s->guest_info->hotplug_enable); > + > + pci_for_each_bus(dev->bus, piix4_update_bus_guest_info, > + s); > } > > static void piix4_pm_machine_ready(Notifier *n, void *opaque) > @@ -589,16 +641,22 @@ static uint64_t pci_read(void *opaque, hwaddr addr, > unsigned int size) > { > PIIX4PMState *s = opaque; > uint32_t val = 0; > + int bsel = s->hotplug_select; > + > + if (bsel > GUEST_INFO_MAX_HOTPLUG_BUS) { > + return 0; > + } > > switch (addr) { > case PCI_UP_BASE - PCI_HOTPLUG_ADDR: > /* Manufacture an "up" value to cause a device check on any hotplug > * slot with a device. Extra device checks are harmless. */ > - val = s->pci0_slot_device_present & s->pci0_hotplug_enable; > + val = s->pci_status[bsel].device_present & > + s->pci_status[bsel].hotplug_enable; > PIIX4_DPRINTF("pci_up_read %x\n", val); > break; > case PCI_DOWN_BASE - PCI_HOTPLUG_ADDR: > - val = s->pci0_status.down; > + val = s->pci_status[bsel].down; > PIIX4_DPRINTF("pci_down_read %x\n", val); > break; > case PCI_EJ_BASE - PCI_HOTPLUG_ADDR: > @@ -606,8 +664,10 @@ static uint64_t pci_read(void *opaque, hwaddr addr, > unsigned int size) > PIIX4_DPRINTF("pci_features_read %x\n", val); > break; > case PCI_RMV_BASE - PCI_HOTPLUG_ADDR: > - val = s->pci0_hotplug_enable; > + val = s->pci_status[bsel].hotplug_enable; > break; > + case PCI_SEL_BASE - PCI_HOTPLUG_ADDR: > + val = s->hotplug_select; > default: > break; > } > @@ -618,12 +678,18 @@ static uint64_t pci_read(void *opaque, hwaddr addr, > unsigned int size) > static void pci_write(void *opaque, hwaddr addr, uint64_t data, > unsigned int size) > { > + PIIX4PMState *s = opaque; > switch (addr) { > case PCI_EJ_BASE - PCI_HOTPLUG_ADDR: > - acpi_piix_eject_slot(opaque, (uint32_t)data); > + if (s->hotplug_select >= GUEST_INFO_MAX_HOTPLUG_BUS) { > + break; > + } > + acpi_piix_eject_slot(s, s->hotplug_select, data); > PIIX4_DPRINTF("pciej write %" HWADDR_PRIx " <== % " PRIu64 "\n", > addr, data); > break; > + case PCI_SEL_BASE - PCI_HOTPLUG_ADDR: > + s->hotplug_select = data; > default: > break; > } > @@ -706,9 +772,6 @@ static void piix4_init_cpu_status(CPUState *cpu, void > *data) > g->sts[id / 8] |= (1 << (id % 8)); > } > > -static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, > - PCIHotplugState state); > - > static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, > PCIBus *bus, PIIX4PMState *s) > { > @@ -720,8 +783,6 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion > *parent, > PCI_HOTPLUG_SIZE); > memory_region_add_subregion(parent, PCI_HOTPLUG_ADDR, > &s->io_pci); > - pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev); > - > qemu_for_each_cpu(piix4_init_cpu_status, &s->gpe_cpu); > memory_region_init_io(&s->io_cpu, &cpu_hotplug_ops, s, > "apci-cpu-hotplug", > PIIX4_PROC_LEN); > @@ -730,16 +791,16 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion > *parent, > qemu_register_cpu_added_notifier(&s->cpu_added_notifier); > } > > -static void enable_device(PIIX4PMState *s, int slot) > +static void enable_device(PIIX4PMState *s, unsigned bsel, int slot) > { > s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS; > - s->pci0_slot_device_present |= (1U << slot); > + s->pci_status[bsel].device_present |= (1U << slot); > } > > -static void disable_device(PIIX4PMState *s, int slot) > +static void disable_device(PIIX4PMState *s, unsigned bsel, int slot) > { > s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS; > - s->pci0_status.down |= (1U << slot); > + s->pci_status[bsel].down |= (1U << slot); > } > > static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, > @@ -748,19 +809,23 @@ static int piix4_device_hotplug(DeviceState *qdev, > PCIDevice *dev, > int slot = PCI_SLOT(dev->devfn); > PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, > PCI_DEVICE(qdev)); > + int bsel = piix4_find_hotplug_bus(s->guest_info, dev->bus); > + if (bsel < 0) { > + return -1; > + } > > /* Don't send event when device is enabled during qemu machine creation: > * it is present on boot, no hotplug event is necessary. We do send an > * event when the device is disabled later. */ > if (state == PCI_COLDPLUG_ENABLED) { > - s->pci0_slot_device_present |= (1U << slot); > + s->pci_status[bsel].device_present |= (1U << slot); > return 0; > } > > if (state == PCI_HOTPLUG_ENABLED) { > - enable_device(s, slot); > + enable_device(s, bsel, slot); > } else { > - disable_device(s, slot); > + disable_device(s, bsel, slot); > } > > pm_update_sci(s); > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 72606a8..43e4988 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -34,6 +34,7 @@ > #include "hw/acpi/acpi.h" > #include "hw/nvram/fw_cfg.h" > #include "hw/i386/bios-linker-loader.h" > +#include "hw/pci/pci_bus.h" > > #define ACPI_BUILD_APPNAME "Bochs" > #define ACPI_BUILD_APPNAME6 "BOCHS " > @@ -258,23 +259,358 @@ acpi_encode_len(uint8_t *ssdt_ptr, int length, int > bytes) > #define ACPI_PROC_SIZEOF (*ssdt_proc_end - *ssdt_proc_start) > #define ACPI_PROC_AML (ssdp_proc_aml + *ssdt_proc_start) > > -/* 0x5B 0x82 DeviceOp PkgLength NameString */ > -#define ACPI_PCIHP_OFFSET_HEX (*ssdt_pcihp_name - *ssdt_pcihp_start + 1) > -#define ACPI_PCIHP_OFFSET_ID (*ssdt_pcihp_id - *ssdt_pcihp_start) > -#define ACPI_PCIHP_OFFSET_ADR (*ssdt_pcihp_adr - *ssdt_pcihp_start) > -#define ACPI_PCIHP_OFFSET_EJ0 (*ssdt_pcihp_ej0 - *ssdt_pcihp_start) > -#define ACPI_PCIHP_SIZEOF (*ssdt_pcihp_end - *ssdt_pcihp_start) > -#define ACPI_PCIHP_AML (ssdp_pcihp_aml + *ssdt_pcihp_start) > - > #define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */ > #define ACPI_SSDT_HEADER_LENGTH 36 > > #include "hw/i386/ssdt-misc.hex" > -#include "hw/i386/ssdt-pcihp.hex" > + > +static inline GArray *build_alloc_array(void) > +{ > + return g_array_new(false, true /* clear */, 1); > +} > + > +static inline void build_free_array(GArray *array) > +{ > + g_array_free(array, true); > +} > + > +static inline void build_prepend_byte(GArray *array, uint8_t val) > +{ > + g_array_prepend_val(array, val); > +} > + > +static inline void build_append_byte(GArray *array, uint8_t val) > +{ > + g_array_append_val(array, val); > +} > + > +static inline void build_prepend_array(GArray *array, GArray *val) > +{ > + g_array_prepend_vals(array, val->data, val->len); > +} > + > +static inline void build_append_array(GArray *array, GArray *val) > +{ > + g_array_append_vals(array, val->data, val->len); > +} > + > +static void build_append_nameseg(GArray *array, const char *format, ...) > +{ > + GString *s = g_string_new(""); > + va_list args; > + > + va_start(args, format); > + g_string_vprintf(s, format, args); > + va_end(args); > + > + assert(s->len == 4); > + g_array_append_vals(array, s->str, s->len); > + g_string_free(s, true); > +} > + > +static void build_prepend_nameseg(GArray *array, const char *format, ...) > +{ > + GString *s = g_string_new(""); > + va_list args; > + > + va_start(args, format); > + g_string_vprintf(s, format, args); > + va_end(args); > + > + assert(s->len == 4); > + g_array_prepend_vals(array, s->str, s->len); > + g_string_free(s, true); > +} > + > +static void build_prepend_devfn(GArray *name, uint8_t devfn) > +{ > + build_prepend_nameseg(name, "S%0.02x_", devfn); > +} > + > +static void build_append_devfn(GArray *name, uint8_t devfn) > +{ > + GArray *array = build_alloc_array(); > + build_prepend_devfn(array, devfn); > + build_append_array(name, array); > + build_free_array(array); > +} > + > +static void build_prepend_name_string(GArray *name, PCIBus *bus) > +{ > + int segcount; > + > + for (;!pci_bus_is_root(bus); bus = bus->parent_dev->bus) { > + build_prepend_devfn(name, bus->parent_dev->devfn); > + } > + g_array_prepend_vals(name, "PCI0", 4); > + g_array_prepend_vals(name, "_SB_", 4); > + /* Name segment length is a multiple of 4 */ > + assert(name->len % 4 == 0); > + segcount = name->len / 4; > + build_prepend_byte(name, segcount); > + build_prepend_byte(name, 0x2f); /* MultiNamePrefix */ > + build_prepend_byte(name, '\\'); /* RootChar */ > +} > + > +static void build_append_name_string(GArray *name, PCIBus *bus) > +{ > + GArray *append = build_alloc_array(); > + build_prepend_name_string(append, bus); > + build_append_array(name, append); > + build_free_array(append); > +} > + > +/* 5.4 Definition Block Encoding */ > +enum { > + PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ > + PACKAGE_LENGTH_2BYTE_SHIFT = 4, > + PACKAGE_LENGTH_3BYTE_SHIFT = 12, > + PACKAGE_LENGTH_4BYTE_SHIFT = 20, > +}; > + > +static void build_prepend_package_length(GArray *package) > +{ > + unsigned bytes; > + uint8_t byte; > + unsigned package_len = package->len; > + unsigned length = package_len; > + > + if (length + 1 < (1 << PACKAGE_LENGTH_1BYTE_SHIFT)) { > + byte = length; > + build_prepend_byte(package, byte); > + return; > + } > + > + if (length + 4 > (1 << PACKAGE_LENGTH_4BYTE_SHIFT)) { > + byte = length >> PACKAGE_LENGTH_4BYTE_SHIFT; > + build_prepend_byte(package, byte); > + length &= (1 << PACKAGE_LENGTH_4BYTE_SHIFT) - 1; > + } > + if (length + 3 > (1 << PACKAGE_LENGTH_3BYTE_SHIFT)) { > + byte = length >> PACKAGE_LENGTH_3BYTE_SHIFT; > + build_prepend_byte(package, byte); > + length &= (1 << PACKAGE_LENGTH_3BYTE_SHIFT) - 1; > + } > + if (length + 2 > (1 << PACKAGE_LENGTH_2BYTE_SHIFT)) { > + byte = length >> PACKAGE_LENGTH_2BYTE_SHIFT; > + build_prepend_byte(package, byte); > + length &= (1 << PACKAGE_LENGTH_2BYTE_SHIFT) - 1; > + } > + > + bytes = package->len - package_len; > + byte = (bytes << PACKAGE_LENGTH_1BYTE_SHIFT) | length; > + build_prepend_byte(package, byte); > +} > + > +static void build_package(GArray *package, uint8_t op) > +{ > + build_prepend_package_length(package); > + build_prepend_byte(package, op); > +} > + > +static void build_extop_package(GArray *package, uint8_t op) > +{ > + build_package(package, op); > + build_prepend_byte(package, 0x5B); /* ExtOpPrefix */ > +} > + > +static void build_append_notify(GArray *method, GArray *target_name, > + uint16_t value) > +{ > + GArray *notify = build_alloc_array(); > + uint8_t op = 0xA0; /* IfOp */ > + > + build_append_byte(notify, 0x93); /* LEqualOp */ > + build_append_byte(notify, 0x68); /* Arg0Op */ > + build_append_byte(notify, 0x0A); /* BytePrefix */ > + build_append_byte(notify, value); > + build_append_byte(notify, value >> 8); > + build_append_byte(notify, 0x86); /* NotifyOp */ > + build_append_array(notify, target_name); > + build_append_byte(notify, 0x69); /* Arg1Op */ > + > + /* Pack it up */ > + build_package(notify, op); > + > + build_append_array(method, notify); > + > + build_free_array(notify); > +} > + > +static void > +build_append_notify_slots(GArray *method, PCIBus *bus, > + unsigned bus_select, > + DECLARE_BITMAP(hotplug_enable, PCI_SLOT_MAX)) > +{ > + int i; > + > + if (!bus) { > + return; > + } > + > + for (i = 0; i < 32; i++) { > + GArray *target; > + if (!test_bit(i, hotplug_enable)) { > + continue; > + } > + target = build_alloc_array(); > + build_prepend_devfn(target, i); > + build_prepend_name_string(target, bus); > + /* *32 matches Multiply in PCNF */ > + build_append_notify(method, target, bus_select * 32 + i); > + build_free_array(target); > + } > +} > + > +static void build_append_notify_buses(GArray *table, PcGuestInfo *guest_info) > +{ > + int i; > + GArray *method = build_alloc_array(); > + > + uint8_t op = 0x14; /* MethodOp */ > + > + build_append_nameseg(method, "PCNT"); > + build_append_byte(method, 0x02); /* MethodFlags: ArgCount */ > + for (i = 0; i < ARRAY_SIZE(guest_info->hotplug_buses); ++i) { > + build_append_notify_slots(method, guest_info->hotplug_buses[i], i, > + guest_info->hotplug_enable[i]); > + } > + build_package(method, op); > + > + build_append_array(table, method); > + build_free_array(method); > +} > + > +static void build_append_device(GArray *table, unsigned devfn) > +{ > + uint8_t op, ej0_op; > + > + GArray *device = build_alloc_array(); > + GArray *ej0 = build_alloc_array(); > + > + op = 0x82; /* DeviceOp */ > + > + build_append_devfn(device, devfn); > + > + build_append_byte(device, 0x08); /* NameOp */ > + build_append_nameseg(device, "_SUN"); > + build_append_byte(device, 0x0A); /* BytePrefix */ > + build_append_byte(device, PCI_SLOT(devfn)); > + build_append_byte(device, 0x08); /* NameOp */ > + build_append_nameseg(device, "_ADR"); > + build_append_byte(device, 0x0C); /* DWordPrefix */ > + build_append_byte(device, PCI_FUNC(devfn)); > + build_append_byte(device, 0x00); > + build_append_byte(device, PCI_SLOT(devfn)); > + build_append_byte(device, 0x00); > + > + ej0_op = 0x14; /* MethodOp */ > + build_append_nameseg(ej0, "_EJ0"); > + build_append_byte(ej0, 0x01); /* MethodFlags: ArgCount */ > + build_append_byte(ej0, 0xA4); /* ReturnOp */ > + build_append_nameseg(ej0, "PCEJ"); > + build_append_nameseg(ej0, "_SUN"); > + build_append_nameseg(ej0, "BNUM"); > + build_package(ej0, ej0_op); > + build_append_array(device, ej0); > + > + build_extop_package(device, op); > + > + build_append_array(table, device); > + build_free_array(device); > + build_free_array(ej0); > +} > + > +static bool build_find_device_bus(PCIBus *buses[GUEST_INFO_MAX_HOTPLUG_BUS], > + PCIBus *bus, unsigned devfn) > +{ > + int i; > + for (i = 0; i < ARRAY_SIZE(buses); ++i) { > + if (buses[i] && buses[i]->parent_dev && > + buses[i]->parent_dev->bus == bus && > + buses[i]->parent_dev->devfn == devfn) { > + return true; > + } > + } > + return false; > +} > + > +static void > +build_append_bus(GArray *table, int bnum, PcGuestInfo *guest_info) > +{ > + uint8_t op; > + int slot; > + unsigned devfn; > + GArray *device; > + PCIBus *bus = guest_info->hotplug_buses[bnum]; > + > + if (!bus) { > + return; > + } > + > + device = build_alloc_array(); > + > + /* Root device described in DSDT */ > + if (bnum) { > + devfn = bus->parent_dev->devfn; > + > + op = 0x82; /* DeviceOp */ > + > + build_append_name_string(device, bus); > + > + build_append_byte(device, 0x08); /* NameOp */ > + build_append_nameseg(device, "_SUN"); > + build_append_byte(device, 0x0A); /* BytePrefix */ > + build_append_byte(device, PCI_SLOT(devfn)); > + build_append_byte(device, 0x08); /* NameOp */ > + build_append_nameseg(device, "BNUM"); > + build_append_byte(device, 0x0A); /* BytePrefix */ > + build_append_byte(device, bnum); > + build_append_byte(device, 0x08); /* NameOp */ > + build_append_nameseg(device, "_ADR"); > + build_append_byte(device, 0x0C); /* DWordPrefix */ > + build_append_byte(device, PCI_FUNC(devfn)); > + build_append_byte(device, 0x00); > + build_append_byte(device, PCI_SLOT(devfn)); > + build_append_byte(device, 0x00); > + } else { > + op = 0x10; /* ScopeOp */ > + build_append_nameseg(device, "PCI0"); > + } > + > + for (slot = 0; slot < PCI_SLOT_MAX; ++slot) { > + if (!test_bit(slot, guest_info->hotplug_enable[bnum])) { > + continue; > + } > + /* Bus device? We'll add it later */ > + if (build_find_device_bus(guest_info->hotplug_buses, > + bus, PCI_DEVFN(slot, 0))) { > + continue; > + } > + build_append_device(device, PCI_DEVFN(slot, 0)); > + } > + > + if (bnum) { > + build_extop_package(device, op); > + } else { > + build_package(device, op); > + } > + > + build_append_array(table, device); > + build_free_array(device); > +} > + > +static void build_append_device_buses(GArray *table, PcGuestInfo *guest_info) > +{ > + int i; > + for (i = 0; i < ARRAY_SIZE(guest_info->hotplug_buses); ++i) { > + build_append_bus(table, i, guest_info); > + } > +} > > static uint8_t* > build_notify(uint8_t *ssdt_ptr, const char *name, int skip, int count, > - const char *target, int ofs) > + const char *target, int ofs, int step) > { > int i; > > @@ -295,31 +631,14 @@ build_notify(uint8_t *ssdt_ptr, const char *name, int > skip, int count, > *(ssdt_ptr++) = i; > *(ssdt_ptr++) = 0x86; /* NotifyOp */ > memcpy(ssdt_ptr, target, 4); > - ssdt_ptr[ofs] = acpi_get_hex(i >> 4); > - ssdt_ptr[ofs + 1] = acpi_get_hex(i); > + ssdt_ptr[ofs] = acpi_get_hex((i * step) >> 4); > + ssdt_ptr[ofs + 1] = acpi_get_hex(i * step); > ssdt_ptr += 4; > *(ssdt_ptr++) = 0x69; /* Arg1Op */ > } > return ssdt_ptr; > } > > -static void patch_pcihp(int slot, uint8_t *ssdt_ptr, uint32_t eject) > -{ > - ssdt_ptr[ACPI_PCIHP_OFFSET_HEX] = acpi_get_hex(slot >> 4); > - ssdt_ptr[ACPI_PCIHP_OFFSET_HEX+1] = acpi_get_hex(slot); > - ssdt_ptr[ACPI_PCIHP_OFFSET_ID] = slot; > - ssdt_ptr[ACPI_PCIHP_OFFSET_ADR + 2] = slot; > - > - /* Runtime patching of ACPI_EJ0: to disable hotplug for a slot, > - * replace the method name: _EJ0 by ACPI_EJ0_. */ > - /* Sanity check */ > - assert (!memcmp(ssdt_ptr + ACPI_PCIHP_OFFSET_EJ0, "_EJ0", 4)); > - > - if (!eject) { > - memcpy(ssdt_ptr + ACPI_PCIHP_OFFSET_EJ0, "EJ0_", 4); > - } > -} > - > static void > build_ssdt(GArray *table_data, GArray *linker, > FWCfgState *fw_cfg, PcGuestInfo *guest_info) > @@ -330,11 +649,20 @@ build_ssdt(GArray *table_data, GArray *linker, > + (acpi_cpus * ACPI_PROC_SIZEOF) /* procs */ > + (1+2+5+(12*acpi_cpus)) /* NTFY */ > + (6+2+1+(1*acpi_cpus)) /* CPON */ > - + (1+3+4) /* Scope(PCI0) */ > - + ((PCI_SLOT_MAX - 1) * ACPI_PCIHP_SIZEOF) /* slots > */ > - + (1+2+5+(12*(PCI_SLOT_MAX - 1)))); /* PCNT */ > - uint8_t *ssdt = acpi_data_push(table_data, length); > - uint8_t *ssdt_ptr = ssdt; > + ); > + uint8_t *ssdt; > + uint8_t *ssdt_ptr; > + GArray *devices = build_alloc_array(); > + GArray *notify = build_alloc_array(); > + > + /* TODO: rewrite this function using glib */ > + build_append_device_buses(devices, guest_info); > + length += devices->len; > + build_append_notify_buses(notify, guest_info); > + length += notify->len; > + > + ssdt = acpi_data_push(table_data, length); > + ssdt_ptr = ssdt; > > /* Copy header and encode fwcfg values in the S3_ / S4_ / S5_ packages */ > memcpy(ssdt_ptr, ssdp_misc_aml, sizeof(ssdp_misc_aml)); > @@ -389,7 +717,7 @@ build_ssdt(GArray *table_data, GArray *linker, > > /* build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} > ...}" */ > /* Arg0 = Processor ID = APIC ID */ > - ssdt_ptr = build_notify(ssdt_ptr, "NTFY", 0, acpi_cpus, "CP00", 2); > + ssdt_ptr = build_notify(ssdt_ptr, "NTFY", 0, acpi_cpus, "CP00", 2, 1); > > /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })" */ > *(ssdt_ptr++) = 0x08; /* NameOp */ > @@ -411,15 +739,10 @@ build_ssdt(GArray *table_data, GArray *linker, > *(ssdt_ptr++) = 'I'; > *(ssdt_ptr++) = '0'; > > - /* build Device object for each slot */ > - for (i = 1; i < PCI_SLOT_MAX; i++) { > - bool eject = test_bit(i, guest_info->slot_hotplug_enable); > - memcpy(ssdt_ptr, ACPI_PCIHP_AML, ACPI_PCIHP_SIZEOF); > - patch_pcihp(i, ssdt_ptr, eject); > - ssdt_ptr += ACPI_PCIHP_SIZEOF; > - } > - > - ssdt_ptr = build_notify(ssdt_ptr, "PCNT", 1, PCI_SLOT_MAX, "S00_", 1); > + memcpy(ssdt_ptr, devices->data, devices->len); > + ssdt_ptr += devices->len; > + memcpy(ssdt_ptr, notify->data, notify->len); > + ssdt_ptr += notify->len; > > build_header(linker, table_data, > (void*)ssdt, ACPI_SSDT_SIGNATURE, ssdt_ptr - ssdt, 1); > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > index 2ab2572..c2e74e9 100644 > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -6,7 +6,7 @@ obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o > obj-y += kvmvapic.o > obj-y += acpi-build.o > obj-y += bios-linker-loader.o > -hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex > hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex > hw/i386/q35-acpi-dsdt.hex > +hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex > hw/i386/ssdt-proc.hex hw/i386/ssdt-misc.hex hw/i386/q35-acpi-dsdt.hex > hw/i386/pc_piix.o: hw/i386/pc_piix.c hw/i386/acpi-dsdt.hex > hw/i386/pc_q35.o: hw/i386/pc_q35.c hw/i386/q35-acpi-dsdt.hex > > diff --git a/hw/i386/ssdt-pcihp.hex.generated > b/hw/i386/ssdt-pcihp.hex.generated > deleted file mode 100644 > index 0d32a27..0000000 > --- a/hw/i386/ssdt-pcihp.hex.generated > +++ /dev/null > @@ -1,108 +0,0 @@ > -static unsigned char ssdt_pcihp_name[] = { > -0x33 > -}; > -static unsigned char ssdt_pcihp_adr[] = { > -0x44 > -}; > -static unsigned char ssdt_pcihp_end[] = { > -0x58 > -}; > -static unsigned char ssdp_pcihp_aml[] = { > -0x53, > -0x53, > -0x44, > -0x54, > -0x58, > -0x0, > -0x0, > -0x0, > -0x1, > -0x77, > -0x42, > -0x58, > -0x50, > -0x43, > -0x0, > -0x0, > -0x42, > -0x58, > -0x53, > -0x53, > -0x44, > -0x54, > -0x50, > -0x43, > -0x1, > -0x0, > -0x0, > -0x0, > -0x49, > -0x4e, > -0x54, > -0x4c, > -0x28, > -0x5, > -0x10, > -0x20, > -0x10, > -0x33, > -0x5c, > -0x2e, > -0x5f, > -0x53, > -0x42, > -0x5f, > -0x50, > -0x43, > -0x49, > -0x30, > -0x5b, > -0x82, > -0x26, > -0x53, > -0x41, > -0x41, > -0x5f, > -0x8, > -0x5f, > -0x53, > -0x55, > -0x4e, > -0xa, > -0xaa, > -0x8, > -0x5f, > -0x41, > -0x44, > -0x52, > -0xc, > -0x0, > -0x0, > -0xaa, > -0x0, > -0x14, > -0xf, > -0x5f, > -0x45, > -0x4a, > -0x30, > -0x1, > -0xa4, > -0x50, > -0x43, > -0x45, > -0x4a, > -0x5f, > -0x53, > -0x55, > -0x4e > -}; > -static unsigned char ssdt_pcihp_start[] = { > -0x30 > -}; > -static unsigned char ssdt_pcihp_id[] = { > -0x3d > -}; > -static unsigned char ssdt_pcihp_ej0[] = { > -0x4a > -}; > -- > MST _______________________________________________ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios