| Message ID | 1391174830-5866-4-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jan 31, 2014 at 02:27:08PM +0100, Igor Mammedov wrote: > due to recent change introduced by: > "pcihp: reduce number of device check events" > > 'up' field is cleared right after it's read. > This is incompatible with legacy BIOS ACPI code > where PCNF ACPI method reads this field 32 times. > > To make pci_read mmio callback compatible with legacy > 'up' behavior, pcihp code will need to know in which > mode it runs so move 'use_acpi_pci_hotplug' into > AcpiPciHpState structure and alter register behavior > accordingly. > > PS: > And since field 'use_acpi_pci_hotplug' is touched > anyway rename it to reflect its meaning more crearly > to 'legacy_piix' and corresponding property to > 'legacy-acpi-pci-hotplug'. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/acpi/pcihp.c | 4 +++- > hw/acpi/piix4.c | 15 +++++++-------- > include/hw/acpi/pcihp.h | 1 + > include/hw/i386/pc.h | 4 ++-- > 4 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index 64c8cf2..a7353f6 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -215,7 +215,9 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size) > switch (addr) { > case PCI_UP_BASE: > val = s->acpi_pcihp_pci_status[bsel].up; > - s->acpi_pcihp_pci_status[bsel].up = 0; > + if (!s->legacy_piix) { > + s->acpi_pcihp_pci_status[bsel].up = 0; > + } > ACPI_PCIHP_DPRINTF("pci_up_read %" PRIu32 "\n", val); > break; > case PCI_DOWN_BASE: > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 5d55a3c..33c9516 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -88,7 +88,6 @@ typedef struct PIIX4PMState { > > /* for new pci hotplug (with PCI2PCI bridge support) */ > AcpiPciHpState acpi_pci_hotplug; > - bool use_acpi_pci_hotplug; > > uint8_t disable_s3; > uint8_t disable_s4; > @@ -263,13 +262,13 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int version_id) > static bool vmstate_test_use_acpi_pci_hotplug(void *opaque, int version_id) > { > PIIX4PMState *s = opaque; > - return s->use_acpi_pci_hotplug; > + return !s->acpi_pci_hotplug.legacy_piix; > } > > static bool vmstate_test_no_use_acpi_pci_hotplug(void *opaque, int version_id) > { > PIIX4PMState *s = opaque; > - return !s->use_acpi_pci_hotplug; > + return s->acpi_pci_hotplug.legacy_piix; > } > > /* qemu-kvm 1.2 uses version 3 but advertised as 2 > @@ -377,7 +376,7 @@ static void piix4_reset(void *opaque) > pci_conf[0x5B] = 0x02; > } > pm_io_space_update(s); > - if (s->use_acpi_pci_hotplug) { > + if (!s->acpi_pci_hotplug.legacy_piix) { > acpi_pcihp_reset(&s->acpi_pci_hotplug); > } else { > piix4_update_hotplug(s); > @@ -426,7 +425,7 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque) > 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) { > + if (!s->acpi_pci_hotplug.legacy_piix) { > pci_for_each_bus(d->bus, piix4_update_bus_hotplug, s); > } > } > @@ -550,8 +549,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_BOOL("legacy-acpi-pci-hotplug", PIIX4PMState, > + acpi_pci_hotplug.legacy_piix, false), That's a worse less descriptive name for user property. I also think it's not nice that piix pokes at acpi_pci_hotplug structure internals without an API, here and below. Let's pass a bool flag to the init function, have acpi maintain it internally. > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -694,7 +693,7 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, > "acpi-gpe0", GPE_LEN); > memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe); > > - if (s->use_acpi_pci_hotplug) { > + if (!s->acpi_pci_hotplug.legacy_piix) { > acpi_pcihp_init(&s->acpi_pci_hotplug, bus, parent); > } else { > memory_region_init_io(&s->io_pci, OBJECT(s), &piix4_pci_ops, s, > diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h > index aa297c2..6877892 100644 > --- a/include/hw/acpi/pcihp.h > +++ b/include/hw/acpi/pcihp.h > @@ -46,6 +46,7 @@ typedef struct AcpiPciHpState { > uint32_t hotplug_select; > PCIBus *root; > MemoryRegion io; > + bool legacy_piix; > } AcpiPciHpState; > > void acpi_pcihp_init(AcpiPciHpState *, PCIBus *root, > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 3e1e81b..68d75d8 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -268,8 +268,8 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > },\ > {\ > .driver = "PIIX4_PM",\ > - .property = "acpi-pci-hotplug-with-bridge-support",\ > - .value = "off",\ > + .property = "legacy-acpi-pci-hotplug",\ > + .value = "on",\ ? > } > > #define PC_COMPAT_1_6 \ > -- > 1.7.1
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c index 64c8cf2..a7353f6 100644 --- a/hw/acpi/pcihp.c +++ b/hw/acpi/pcihp.c @@ -215,7 +215,9 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size) switch (addr) { case PCI_UP_BASE: val = s->acpi_pcihp_pci_status[bsel].up; - s->acpi_pcihp_pci_status[bsel].up = 0; + if (!s->legacy_piix) { + s->acpi_pcihp_pci_status[bsel].up = 0; + } ACPI_PCIHP_DPRINTF("pci_up_read %" PRIu32 "\n", val); break; case PCI_DOWN_BASE: diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 5d55a3c..33c9516 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -88,7 +88,6 @@ typedef struct PIIX4PMState { /* for new pci hotplug (with PCI2PCI bridge support) */ AcpiPciHpState acpi_pci_hotplug; - bool use_acpi_pci_hotplug; uint8_t disable_s3; uint8_t disable_s4; @@ -263,13 +262,13 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int version_id) static bool vmstate_test_use_acpi_pci_hotplug(void *opaque, int version_id) { PIIX4PMState *s = opaque; - return s->use_acpi_pci_hotplug; + return !s->acpi_pci_hotplug.legacy_piix; } static bool vmstate_test_no_use_acpi_pci_hotplug(void *opaque, int version_id) { PIIX4PMState *s = opaque; - return !s->use_acpi_pci_hotplug; + return s->acpi_pci_hotplug.legacy_piix; } /* qemu-kvm 1.2 uses version 3 but advertised as 2 @@ -377,7 +376,7 @@ static void piix4_reset(void *opaque) pci_conf[0x5B] = 0x02; } pm_io_space_update(s); - if (s->use_acpi_pci_hotplug) { + if (!s->acpi_pci_hotplug.legacy_piix) { acpi_pcihp_reset(&s->acpi_pci_hotplug); } else { piix4_update_hotplug(s); @@ -426,7 +425,7 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque) 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) { + if (!s->acpi_pci_hotplug.legacy_piix) { pci_for_each_bus(d->bus, piix4_update_bus_hotplug, s); } } @@ -550,8 +549,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_BOOL("legacy-acpi-pci-hotplug", PIIX4PMState, + acpi_pci_hotplug.legacy_piix, false), DEFINE_PROP_END_OF_LIST(), }; @@ -694,7 +693,7 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, "acpi-gpe0", GPE_LEN); memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe); - if (s->use_acpi_pci_hotplug) { + if (!s->acpi_pci_hotplug.legacy_piix) { acpi_pcihp_init(&s->acpi_pci_hotplug, bus, parent); } else { memory_region_init_io(&s->io_pci, OBJECT(s), &piix4_pci_ops, s, diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h index aa297c2..6877892 100644 --- a/include/hw/acpi/pcihp.h +++ b/include/hw/acpi/pcihp.h @@ -46,6 +46,7 @@ typedef struct AcpiPciHpState { uint32_t hotplug_select; PCIBus *root; MemoryRegion io; + bool legacy_piix; } AcpiPciHpState; void acpi_pcihp_init(AcpiPciHpState *, PCIBus *root, diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 3e1e81b..68d75d8 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -268,8 +268,8 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); },\ {\ .driver = "PIIX4_PM",\ - .property = "acpi-pci-hotplug-with-bridge-support",\ - .value = "off",\ + .property = "legacy-acpi-pci-hotplug",\ + .value = "on",\ } #define PC_COMPAT_1_6 \
due to recent change introduced by: "pcihp: reduce number of device check events" 'up' field is cleared right after it's read. This is incompatible with legacy BIOS ACPI code where PCNF ACPI method reads this field 32 times. To make pci_read mmio callback compatible with legacy 'up' behavior, pcihp code will need to know in which mode it runs so move 'use_acpi_pci_hotplug' into AcpiPciHpState structure and alter register behavior accordingly. PS: And since field 'use_acpi_pci_hotplug' is touched anyway rename it to reflect its meaning more crearly to 'legacy_piix' and corresponding property to 'legacy-acpi-pci-hotplug'. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/acpi/pcihp.c | 4 +++- hw/acpi/piix4.c | 15 +++++++-------- include/hw/acpi/pcihp.h | 1 + include/hw/i386/pc.h | 4 ++-- 4 files changed, 13 insertions(+), 11 deletions(-)