| Message ID | 20200818215227.181654-4-jusual@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Use ACPI PCI hot-plug for q35 | expand |
On Tue, 18 Aug 2020 23:52:26 +0200 Julia Suvorova <jusual@redhat.com> wrote: > Other methods may be used if the system is capable of this and the _OSC bit > is set. Disable them explicitly to force ACPI PCI hot-plug use. The older > versions will still use PCIe native. > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > --- > hw/i386/acpi-build.h | 11 +++++++++++ > hw/i386/acpi-build.c | 21 +++++++++++++++------ > 2 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > index 74df5fc612..6f94312c39 100644 > --- a/hw/i386/acpi-build.h > +++ b/hw/i386/acpi-build.h > @@ -5,6 +5,17 @@ > > extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > +/* PCI Firmware Specification 3.2, Table 4-5 */ > +typedef enum { > + ACPI_OSC_NATIVE_HP_EN = 0, > + ACPI_OSC_SHPC_EN = 1, > + ACPI_OSC_PME_EN = 2, > + ACPI_OSC_AER_EN = 3, > + ACPI_OSC_PCIE_CAP_EN = 4, > + ACPI_OSC_LTR_EN = 5, > + ACPI_OSC_ALLONES_INVALID = 6, > +} AcpiOSCField; > + > void acpi_setup(void); > > #endif > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index f3cd52bd06..c5f4802b8c 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1411,7 +1411,7 @@ static void build_i386_pci_hotplug(Aml *table, uint64_t pcihp_addr) > aml_append(table, scope); > } > > -static Aml *build_q35_osc_method(void) > +static Aml *build_q35_osc_method(AcpiPmInfo *pm) > { > Aml *if_ctx; > Aml *if_ctx2; > @@ -1419,6 +1419,7 @@ static Aml *build_q35_osc_method(void) > Aml *method; > Aml *a_cwd1 = aml_name("CDW1"); > Aml *a_ctrl = aml_local(0); > + unsigned osc_ctrl; > > method = aml_method("_OSC", 4, AML_NOTSERIALIZED); > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1")); > @@ -1430,11 +1431,19 @@ static Aml *build_q35_osc_method(void) > > aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl)); > > + /* Always allow native PME, AER (depend on PCIE Capability Control) */ > + osc_ctrl = BIT(ACPI_OSC_PME_EN) | BIT(ACPI_OSC_AER_EN) | > + BIT(ACPI_OSC_PCIE_CAP_EN); > + > /* > - * Always allow native PME, AER (no dependencies) > - * Allow SHPC (PCI bridges can have SHPC controller) > + * Guests seem to generally prefer native hot-plug control. > + * Enable it only when we do not use ACPI hot-plug. > */ > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl)); > + if (!pm->pcihp_bridge_en) { > + osc_ctrl |= BIT(ACPI_OSC_NATIVE_HP_EN) | BIT(ACPI_OSC_SHPC_EN); > + } ACPI hotplug works only for coldplugged bridges, and native one is used on hotplugged ones. Wouldn't that break SHPC/Native hotplug on hotplugged PCI/PCI-E bridge? > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(osc_ctrl), a_ctrl)); > > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1)))); > /* Unknown revision */ > @@ -1514,7 +1523,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > aml_append(dev, aml_name_decl("_ADR", aml_int(0))); > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > - aml_append(dev, build_q35_osc_method()); > + aml_append(dev, build_q35_osc_method(pm)); > aml_append(sb_scope, dev); > aml_append(dsdt, sb_scope); > > @@ -1590,7 +1599,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > if (pci_bus_is_express(bus)) { > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > - aml_append(dev, build_q35_osc_method()); > + aml_append(dev, build_q35_osc_method(pm)); > } else { > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > }
On Fri, Aug 21, 2020 at 2:13 PM Igor Mammedov <imammedo@redhat.com> wrote: > > On Tue, 18 Aug 2020 23:52:26 +0200 > Julia Suvorova <jusual@redhat.com> wrote: > > > Other methods may be used if the system is capable of this and the _OSC bit > > is set. Disable them explicitly to force ACPI PCI hot-plug use. The older > > versions will still use PCIe native. > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > --- > > hw/i386/acpi-build.h | 11 +++++++++++ > > hw/i386/acpi-build.c | 21 +++++++++++++++------ > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > > index 74df5fc612..6f94312c39 100644 > > --- a/hw/i386/acpi-build.h > > +++ b/hw/i386/acpi-build.h > > @@ -5,6 +5,17 @@ > > > > extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > > > +/* PCI Firmware Specification 3.2, Table 4-5 */ > > +typedef enum { > > + ACPI_OSC_NATIVE_HP_EN = 0, > > + ACPI_OSC_SHPC_EN = 1, > > + ACPI_OSC_PME_EN = 2, > > + ACPI_OSC_AER_EN = 3, > > + ACPI_OSC_PCIE_CAP_EN = 4, > > + ACPI_OSC_LTR_EN = 5, > > + ACPI_OSC_ALLONES_INVALID = 6, > > +} AcpiOSCField; > > + > > void acpi_setup(void); > > > > #endif > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index f3cd52bd06..c5f4802b8c 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -1411,7 +1411,7 @@ static void build_i386_pci_hotplug(Aml *table, uint64_t pcihp_addr) > > aml_append(table, scope); > > } > > > > -static Aml *build_q35_osc_method(void) > > +static Aml *build_q35_osc_method(AcpiPmInfo *pm) > > { > > Aml *if_ctx; > > Aml *if_ctx2; > > @@ -1419,6 +1419,7 @@ static Aml *build_q35_osc_method(void) > > Aml *method; > > Aml *a_cwd1 = aml_name("CDW1"); > > Aml *a_ctrl = aml_local(0); > > + unsigned osc_ctrl; > > > > method = aml_method("_OSC", 4, AML_NOTSERIALIZED); > > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1")); > > @@ -1430,11 +1431,19 @@ static Aml *build_q35_osc_method(void) > > > > aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl)); > > > > + /* Always allow native PME, AER (depend on PCIE Capability Control) */ > > + osc_ctrl = BIT(ACPI_OSC_PME_EN) | BIT(ACPI_OSC_AER_EN) | > > + BIT(ACPI_OSC_PCIE_CAP_EN); > > + > > /* > > - * Always allow native PME, AER (no dependencies) > > - * Allow SHPC (PCI bridges can have SHPC controller) > > + * Guests seem to generally prefer native hot-plug control. > > + * Enable it only when we do not use ACPI hot-plug. > > */ > > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl)); > > + if (!pm->pcihp_bridge_en) { > > + osc_ctrl |= BIT(ACPI_OSC_NATIVE_HP_EN) | BIT(ACPI_OSC_SHPC_EN); > > + } > > ACPI hotplug works only for coldplugged bridges, and native one is used > on hotplugged ones. > Wouldn't that break SHPC/Native hotplug on hotplugged PCI/PCI-E bridge? Yes, it would. I'll mention it in the commit message. > > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(osc_ctrl), a_ctrl)); > > > > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1)))); > > /* Unknown revision */ > > @@ -1514,7 +1523,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > aml_append(dev, aml_name_decl("_ADR", aml_int(0))); > > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > > - aml_append(dev, build_q35_osc_method()); > > + aml_append(dev, build_q35_osc_method(pm)); > > aml_append(sb_scope, dev); > > aml_append(dsdt, sb_scope); > > > > @@ -1590,7 +1599,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > if (pci_bus_is_express(bus)) { > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > - aml_append(dev, build_q35_osc_method()); > > + aml_append(dev, build_q35_osc_method(pm)); > > } else { > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > } >
On Wed, Sep 16, 2020 at 8:03 PM Julia Suvorova <jusual@redhat.com> wrote: > > On Fri, Aug 21, 2020 at 2:13 PM Igor Mammedov <imammedo@redhat.com> wrote: > > > > On Tue, 18 Aug 2020 23:52:26 +0200 > > Julia Suvorova <jusual@redhat.com> wrote: > > > > > Other methods may be used if the system is capable of this and the _OSC bit > > > is set. Disable them explicitly to force ACPI PCI hot-plug use. The older > > > versions will still use PCIe native. > > > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > > --- > > > hw/i386/acpi-build.h | 11 +++++++++++ > > > hw/i386/acpi-build.c | 21 +++++++++++++++------ > > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > > > index 74df5fc612..6f94312c39 100644 > > > --- a/hw/i386/acpi-build.h > > > +++ b/hw/i386/acpi-build.h > > > @@ -5,6 +5,17 @@ > > > > > > extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > > > > > +/* PCI Firmware Specification 3.2, Table 4-5 */ > > > +typedef enum { > > > + ACPI_OSC_NATIVE_HP_EN = 0, > > > + ACPI_OSC_SHPC_EN = 1, > > > + ACPI_OSC_PME_EN = 2, > > > + ACPI_OSC_AER_EN = 3, > > > + ACPI_OSC_PCIE_CAP_EN = 4, > > > + ACPI_OSC_LTR_EN = 5, > > > + ACPI_OSC_ALLONES_INVALID = 6, > > > +} AcpiOSCField; > > > + > > > void acpi_setup(void); > > > > > > #endif > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > index f3cd52bd06..c5f4802b8c 100644 > > > --- a/hw/i386/acpi-build.c > > > +++ b/hw/i386/acpi-build.c > > > @@ -1411,7 +1411,7 @@ static void build_i386_pci_hotplug(Aml *table, uint64_t pcihp_addr) > > > aml_append(table, scope); > > > } > > > > > > -static Aml *build_q35_osc_method(void) > > > +static Aml *build_q35_osc_method(AcpiPmInfo *pm) > > > { > > > Aml *if_ctx; > > > Aml *if_ctx2; > > > @@ -1419,6 +1419,7 @@ static Aml *build_q35_osc_method(void) > > > Aml *method; > > > Aml *a_cwd1 = aml_name("CDW1"); > > > Aml *a_ctrl = aml_local(0); > > > + unsigned osc_ctrl; > > > > > > method = aml_method("_OSC", 4, AML_NOTSERIALIZED); > > > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1")); > > > @@ -1430,11 +1431,19 @@ static Aml *build_q35_osc_method(void) > > > > > > aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl)); > > > > > > + /* Always allow native PME, AER (depend on PCIE Capability Control) */ > > > + osc_ctrl = BIT(ACPI_OSC_PME_EN) | BIT(ACPI_OSC_AER_EN) | > > > + BIT(ACPI_OSC_PCIE_CAP_EN); > > > + > > > /* > > > - * Always allow native PME, AER (no dependencies) > > > - * Allow SHPC (PCI bridges can have SHPC controller) > > > + * Guests seem to generally prefer native hot-plug control. > > > + * Enable it only when we do not use ACPI hot-plug. > > > */ > > > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl)); > > > + if (!pm->pcihp_bridge_en) { > > > + osc_ctrl |= BIT(ACPI_OSC_NATIVE_HP_EN) | BIT(ACPI_OSC_SHPC_EN); > > > + } > > > > ACPI hotplug works only for coldplugged bridges, and native one is used > > on hotplugged ones. > > Wouldn't that break SHPC/Native hotplug on hotplugged PCI/PCI-E bridge? Wait, what configuration are you talking about exactly? > > > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(osc_ctrl), a_ctrl)); > > > > > > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1)))); > > > /* Unknown revision */ > > > @@ -1514,7 +1523,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > > aml_append(dev, aml_name_decl("_ADR", aml_int(0))); > > > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > > > - aml_append(dev, build_q35_osc_method()); > > > + aml_append(dev, build_q35_osc_method(pm)); > > > aml_append(sb_scope, dev); > > > aml_append(dsdt, sb_scope); > > > > > > @@ -1590,7 +1599,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > if (pci_bus_is_express(bus)) { > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > > - aml_append(dev, build_q35_osc_method()); > > > + aml_append(dev, build_q35_osc_method(pm)); > > > } else { > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > > } > >
On Wed, 16 Sep 2020 21:14:36 +0200 Julia Suvorova <jusual@redhat.com> wrote: > On Wed, Sep 16, 2020 at 8:03 PM Julia Suvorova <jusual@redhat.com> wrote: > > > > On Fri, Aug 21, 2020 at 2:13 PM Igor Mammedov <imammedo@redhat.com> wrote: > > > > > > On Tue, 18 Aug 2020 23:52:26 +0200 > > > Julia Suvorova <jusual@redhat.com> wrote: > > > > > > > Other methods may be used if the system is capable of this and the _OSC bit > > > > is set. Disable them explicitly to force ACPI PCI hot-plug use. The older > > > > versions will still use PCIe native. > > > > > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > > > --- > > > > hw/i386/acpi-build.h | 11 +++++++++++ > > > > hw/i386/acpi-build.c | 21 +++++++++++++++------ > > > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > > > > index 74df5fc612..6f94312c39 100644 > > > > --- a/hw/i386/acpi-build.h > > > > +++ b/hw/i386/acpi-build.h > > > > @@ -5,6 +5,17 @@ > > > > > > > > extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > > > > > > > +/* PCI Firmware Specification 3.2, Table 4-5 */ > > > > +typedef enum { > > > > + ACPI_OSC_NATIVE_HP_EN = 0, > > > > + ACPI_OSC_SHPC_EN = 1, > > > > + ACPI_OSC_PME_EN = 2, > > > > + ACPI_OSC_AER_EN = 3, > > > > + ACPI_OSC_PCIE_CAP_EN = 4, > > > > + ACPI_OSC_LTR_EN = 5, > > > > + ACPI_OSC_ALLONES_INVALID = 6, > > > > +} AcpiOSCField; > > > > + > > > > void acpi_setup(void); > > > > > > > > #endif > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > index f3cd52bd06..c5f4802b8c 100644 > > > > --- a/hw/i386/acpi-build.c > > > > +++ b/hw/i386/acpi-build.c > > > > @@ -1411,7 +1411,7 @@ static void build_i386_pci_hotplug(Aml *table, uint64_t pcihp_addr) > > > > aml_append(table, scope); > > > > } > > > > > > > > -static Aml *build_q35_osc_method(void) > > > > +static Aml *build_q35_osc_method(AcpiPmInfo *pm) > > > > { > > > > Aml *if_ctx; > > > > Aml *if_ctx2; > > > > @@ -1419,6 +1419,7 @@ static Aml *build_q35_osc_method(void) > > > > Aml *method; > > > > Aml *a_cwd1 = aml_name("CDW1"); > > > > Aml *a_ctrl = aml_local(0); > > > > + unsigned osc_ctrl; > > > > > > > > method = aml_method("_OSC", 4, AML_NOTSERIALIZED); > > > > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1")); > > > > @@ -1430,11 +1431,19 @@ static Aml *build_q35_osc_method(void) > > > > > > > > aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl)); > > > > > > > > + /* Always allow native PME, AER (depend on PCIE Capability Control) */ > > > > + osc_ctrl = BIT(ACPI_OSC_PME_EN) | BIT(ACPI_OSC_AER_EN) | > > > > + BIT(ACPI_OSC_PCIE_CAP_EN); > > > > + > > > > /* > > > > - * Always allow native PME, AER (no dependencies) > > > > - * Allow SHPC (PCI bridges can have SHPC controller) > > > > + * Guests seem to generally prefer native hot-plug control. > > > > + * Enable it only when we do not use ACPI hot-plug. > > > > */ > > > > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl)); > > > > + if (!pm->pcihp_bridge_en) { > > > > + osc_ctrl |= BIT(ACPI_OSC_NATIVE_HP_EN) | BIT(ACPI_OSC_SHPC_EN); > > > > + } > > > > > > ACPI hotplug works only for coldplugged bridges, and native one is used > > > on hotplugged ones. > > > Wouldn't that break SHPC/Native hotplug on hotplugged PCI/PCI-E bridge? > > Wait, what configuration are you talking about exactly? Currently on piix4, we have ACPI and native hotplug working simultaneously the former works on cold-plugged bridges and if you hotplug another bridge, that one will use native method. With above hunk it probably will break. > > > > > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(osc_ctrl), a_ctrl)); > > > > > > > > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1)))); > > > > /* Unknown revision */ > > > > @@ -1514,7 +1523,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > > > aml_append(dev, aml_name_decl("_ADR", aml_int(0))); > > > > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > > > > - aml_append(dev, build_q35_osc_method()); > > > > + aml_append(dev, build_q35_osc_method(pm)); > > > > aml_append(sb_scope, dev); > > > > aml_append(dsdt, sb_scope); > > > > > > > > @@ -1590,7 +1599,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > if (pci_bus_is_express(bus)) { > > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > > > - aml_append(dev, build_q35_osc_method()); > > > > + aml_append(dev, build_q35_osc_method(pm)); > > > > } else { > > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > > > } > > > >
On Thu, Sep 17, 2020 at 8:01 AM Igor Mammedov <imammedo@redhat.com> wrote: > > On Wed, 16 Sep 2020 21:14:36 +0200 > Julia Suvorova <jusual@redhat.com> wrote: > > > On Wed, Sep 16, 2020 at 8:03 PM Julia Suvorova <jusual@redhat.com> wrote: > > > > > > On Fri, Aug 21, 2020 at 2:13 PM Igor Mammedov <imammedo@redhat.com> wrote: > > > > > > > > On Tue, 18 Aug 2020 23:52:26 +0200 > > > > Julia Suvorova <jusual@redhat.com> wrote: > > > > > > > > > Other methods may be used if the system is capable of this and the _OSC bit > > > > > is set. Disable them explicitly to force ACPI PCI hot-plug use. The older > > > > > versions will still use PCIe native. > > > > > > > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > > > > --- > > > > > hw/i386/acpi-build.h | 11 +++++++++++ > > > > > hw/i386/acpi-build.c | 21 +++++++++++++++------ > > > > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > > > > > index 74df5fc612..6f94312c39 100644 > > > > > --- a/hw/i386/acpi-build.h > > > > > +++ b/hw/i386/acpi-build.h > > > > > @@ -5,6 +5,17 @@ > > > > > > > > > > extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > > > > > > > > > +/* PCI Firmware Specification 3.2, Table 4-5 */ > > > > > +typedef enum { > > > > > + ACPI_OSC_NATIVE_HP_EN = 0, > > > > > + ACPI_OSC_SHPC_EN = 1, > > > > > + ACPI_OSC_PME_EN = 2, > > > > > + ACPI_OSC_AER_EN = 3, > > > > > + ACPI_OSC_PCIE_CAP_EN = 4, > > > > > + ACPI_OSC_LTR_EN = 5, > > > > > + ACPI_OSC_ALLONES_INVALID = 6, > > > > > +} AcpiOSCField; > > > > > + > > > > > void acpi_setup(void); > > > > > > > > > > #endif > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > > index f3cd52bd06..c5f4802b8c 100644 > > > > > --- a/hw/i386/acpi-build.c > > > > > +++ b/hw/i386/acpi-build.c > > > > > @@ -1411,7 +1411,7 @@ static void build_i386_pci_hotplug(Aml *table, uint64_t pcihp_addr) > > > > > aml_append(table, scope); > > > > > } > > > > > > > > > > -static Aml *build_q35_osc_method(void) > > > > > +static Aml *build_q35_osc_method(AcpiPmInfo *pm) > > > > > { > > > > > Aml *if_ctx; > > > > > Aml *if_ctx2; > > > > > @@ -1419,6 +1419,7 @@ static Aml *build_q35_osc_method(void) > > > > > Aml *method; > > > > > Aml *a_cwd1 = aml_name("CDW1"); > > > > > Aml *a_ctrl = aml_local(0); > > > > > + unsigned osc_ctrl; > > > > > > > > > > method = aml_method("_OSC", 4, AML_NOTSERIALIZED); > > > > > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1")); > > > > > @@ -1430,11 +1431,19 @@ static Aml *build_q35_osc_method(void) > > > > > > > > > > aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl)); > > > > > > > > > > + /* Always allow native PME, AER (depend on PCIE Capability Control) */ > > > > > + osc_ctrl = BIT(ACPI_OSC_PME_EN) | BIT(ACPI_OSC_AER_EN) | > > > > > + BIT(ACPI_OSC_PCIE_CAP_EN); > > > > > + > > > > > /* > > > > > - * Always allow native PME, AER (no dependencies) > > > > > - * Allow SHPC (PCI bridges can have SHPC controller) > > > > > + * Guests seem to generally prefer native hot-plug control. > > > > > + * Enable it only when we do not use ACPI hot-plug. > > > > > */ > > > > > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl)); > > > > > + if (!pm->pcihp_bridge_en) { > > > > > + osc_ctrl |= BIT(ACPI_OSC_NATIVE_HP_EN) | BIT(ACPI_OSC_SHPC_EN); > > > > > + } > > > > > > > > ACPI hotplug works only for coldplugged bridges, and native one is used > > > > on hotplugged ones. > > > > Wouldn't that break SHPC/Native hotplug on hotplugged PCI/PCI-E bridge? > > > > Wait, what configuration are you talking about exactly? > Currently on piix4, we have ACPI and native hotplug working simultaneously > the former works on cold-plugged bridges and if you hotplug another bridge, > that one will use native method. > With above hunk it probably will break. Ok, I will add a check for piix4. > > > > > > > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(osc_ctrl), a_ctrl)); > > > > > > > > > > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1)))); > > > > > /* Unknown revision */ > > > > > @@ -1514,7 +1523,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > > > > aml_append(dev, aml_name_decl("_ADR", aml_int(0))); > > > > > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > > > > > - aml_append(dev, build_q35_osc_method()); > > > > > + aml_append(dev, build_q35_osc_method(pm)); > > > > > aml_append(sb_scope, dev); > > > > > aml_append(dsdt, sb_scope); > > > > > > > > > > @@ -1590,7 +1599,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > > if (pci_bus_is_express(bus)) { > > > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > > > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > > > > - aml_append(dev, build_q35_osc_method()); > > > > > + aml_append(dev, build_q35_osc_method(pm)); > > > > > } else { > > > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > > > > } > > > > > > >
On Thu, 17 Sep 2020 12:40:16 +0200 Julia Suvorova <jusual@redhat.com> wrote: > On Thu, Sep 17, 2020 at 8:01 AM Igor Mammedov <imammedo@redhat.com> wrote: > > > > On Wed, 16 Sep 2020 21:14:36 +0200 > > Julia Suvorova <jusual@redhat.com> wrote: > > > > > On Wed, Sep 16, 2020 at 8:03 PM Julia Suvorova <jusual@redhat.com> wrote: > > > > > > > > On Fri, Aug 21, 2020 at 2:13 PM Igor Mammedov <imammedo@redhat.com> wrote: > > > > > > > > > > On Tue, 18 Aug 2020 23:52:26 +0200 > > > > > Julia Suvorova <jusual@redhat.com> wrote: > > > > > > > > > > > Other methods may be used if the system is capable of this and the _OSC bit > > > > > > is set. Disable them explicitly to force ACPI PCI hot-plug use. The older > > > > > > versions will still use PCIe native. > > > > > > > > > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > > > > > --- > > > > > > hw/i386/acpi-build.h | 11 +++++++++++ > > > > > > hw/i386/acpi-build.c | 21 +++++++++++++++------ > > > > > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > > > > > > > > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > > > > > > index 74df5fc612..6f94312c39 100644 > > > > > > --- a/hw/i386/acpi-build.h > > > > > > +++ b/hw/i386/acpi-build.h > > > > > > @@ -5,6 +5,17 @@ > > > > > > > > > > > > extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > > > > > > > > > > > +/* PCI Firmware Specification 3.2, Table 4-5 */ > > > > > > +typedef enum { > > > > > > + ACPI_OSC_NATIVE_HP_EN = 0, > > > > > > + ACPI_OSC_SHPC_EN = 1, > > > > > > + ACPI_OSC_PME_EN = 2, > > > > > > + ACPI_OSC_AER_EN = 3, > > > > > > + ACPI_OSC_PCIE_CAP_EN = 4, > > > > > > + ACPI_OSC_LTR_EN = 5, > > > > > > + ACPI_OSC_ALLONES_INVALID = 6, > > > > > > +} AcpiOSCField; > > > > > > + > > > > > > void acpi_setup(void); > > > > > > > > > > > > #endif > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > > > index f3cd52bd06..c5f4802b8c 100644 > > > > > > --- a/hw/i386/acpi-build.c > > > > > > +++ b/hw/i386/acpi-build.c > > > > > > @@ -1411,7 +1411,7 @@ static void build_i386_pci_hotplug(Aml *table, uint64_t pcihp_addr) > > > > > > aml_append(table, scope); > > > > > > } > > > > > > > > > > > > -static Aml *build_q35_osc_method(void) > > > > > > +static Aml *build_q35_osc_method(AcpiPmInfo *pm) > > > > > > { > > > > > > Aml *if_ctx; > > > > > > Aml *if_ctx2; > > > > > > @@ -1419,6 +1419,7 @@ static Aml *build_q35_osc_method(void) > > > > > > Aml *method; > > > > > > Aml *a_cwd1 = aml_name("CDW1"); > > > > > > Aml *a_ctrl = aml_local(0); > > > > > > + unsigned osc_ctrl; > > > > > > > > > > > > method = aml_method("_OSC", 4, AML_NOTSERIALIZED); > > > > > > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1")); > > > > > > @@ -1430,11 +1431,19 @@ static Aml *build_q35_osc_method(void) > > > > > > > > > > > > aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl)); > > > > > > > > > > > > + /* Always allow native PME, AER (depend on PCIE Capability Control) */ > > > > > > + osc_ctrl = BIT(ACPI_OSC_PME_EN) | BIT(ACPI_OSC_AER_EN) | > > > > > > + BIT(ACPI_OSC_PCIE_CAP_EN); > > > > > > + > > > > > > /* > > > > > > - * Always allow native PME, AER (no dependencies) > > > > > > - * Allow SHPC (PCI bridges can have SHPC controller) > > > > > > + * Guests seem to generally prefer native hot-plug control. > > > > > > + * Enable it only when we do not use ACPI hot-plug. > > > > > > */ > > > > > > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl)); > > > > > > + if (!pm->pcihp_bridge_en) { > > > > > > + osc_ctrl |= BIT(ACPI_OSC_NATIVE_HP_EN) | BIT(ACPI_OSC_SHPC_EN); > > > > > > + } > > > > > > > > > > ACPI hotplug works only for coldplugged bridges, and native one is used > > > > > on hotplugged ones. > > > > > Wouldn't that break SHPC/Native hotplug on hotplugged PCI/PCI-E bridge? > > > > > > Wait, what configuration are you talking about exactly? > > Currently on piix4, we have ACPI and native hotplug working simultaneously > > the former works on cold-plugged bridges and if you hotplug another bridge, > > that one will use native method. > > With above hunk it probably will break. > > Ok, I will add a check for piix4. can we have the same behavior for q35 as well? i.e. could-plugged => ACPI hotplug hotplugged ports, bridges, whatnot => native hotplug > > > > > > > > > > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(osc_ctrl), a_ctrl)); > > > > > > > > > > > > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1)))); > > > > > > /* Unknown revision */ > > > > > > @@ -1514,7 +1523,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > > > > > aml_append(dev, aml_name_decl("_ADR", aml_int(0))); > > > > > > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > > > > > > - aml_append(dev, build_q35_osc_method()); > > > > > > + aml_append(dev, build_q35_osc_method(pm)); > > > > > > aml_append(sb_scope, dev); > > > > > > aml_append(dsdt, sb_scope); > > > > > > > > > > > > @@ -1590,7 +1599,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > > > if (pci_bus_is_express(bus)) { > > > > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > > > > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); > > > > > > - aml_append(dev, build_q35_osc_method()); > > > > > > + aml_append(dev, build_q35_osc_method(pm)); > > > > > > } else { > > > > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > > > > > } > > > > > > > > > > >
diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h index 74df5fc612..6f94312c39 100644 --- a/hw/i386/acpi-build.h +++ b/hw/i386/acpi-build.h @@ -5,6 +5,17 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; +/* PCI Firmware Specification 3.2, Table 4-5 */ +typedef enum { + ACPI_OSC_NATIVE_HP_EN = 0, + ACPI_OSC_SHPC_EN = 1, + ACPI_OSC_PME_EN = 2, + ACPI_OSC_AER_EN = 3, + ACPI_OSC_PCIE_CAP_EN = 4, + ACPI_OSC_LTR_EN = 5, + ACPI_OSC_ALLONES_INVALID = 6, +} AcpiOSCField; + void acpi_setup(void); #endif diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f3cd52bd06..c5f4802b8c 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1411,7 +1411,7 @@ static void build_i386_pci_hotplug(Aml *table, uint64_t pcihp_addr) aml_append(table, scope); } -static Aml *build_q35_osc_method(void) +static Aml *build_q35_osc_method(AcpiPmInfo *pm) { Aml *if_ctx; Aml *if_ctx2; @@ -1419,6 +1419,7 @@ static Aml *build_q35_osc_method(void) Aml *method; Aml *a_cwd1 = aml_name("CDW1"); Aml *a_ctrl = aml_local(0); + unsigned osc_ctrl; method = aml_method("_OSC", 4, AML_NOTSERIALIZED); aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1")); @@ -1430,11 +1431,19 @@ static Aml *build_q35_osc_method(void) aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl)); + /* Always allow native PME, AER (depend on PCIE Capability Control) */ + osc_ctrl = BIT(ACPI_OSC_PME_EN) | BIT(ACPI_OSC_AER_EN) | + BIT(ACPI_OSC_PCIE_CAP_EN); + /* - * Always allow native PME, AER (no dependencies) - * Allow SHPC (PCI bridges can have SHPC controller) + * Guests seem to generally prefer native hot-plug control. + * Enable it only when we do not use ACPI hot-plug. */ - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl)); + if (!pm->pcihp_bridge_en) { + osc_ctrl |= BIT(ACPI_OSC_NATIVE_HP_EN) | BIT(ACPI_OSC_SHPC_EN); + } + + aml_append(if_ctx, aml_and(a_ctrl, aml_int(osc_ctrl), a_ctrl)); if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1)))); /* Unknown revision */ @@ -1514,7 +1523,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(1))); - aml_append(dev, build_q35_osc_method()); + aml_append(dev, build_q35_osc_method(pm)); aml_append(sb_scope, dev); aml_append(dsdt, sb_scope); @@ -1590,7 +1599,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, if (pci_bus_is_express(bus)) { aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); - aml_append(dev, build_q35_osc_method()); + aml_append(dev, build_q35_osc_method(pm)); } else { aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); }
Other methods may be used if the system is capable of this and the _OSC bit is set. Disable them explicitly to force ACPI PCI hot-plug use. The older versions will still use PCIe native. Signed-off-by: Julia Suvorova <jusual@redhat.com> --- hw/i386/acpi-build.h | 11 +++++++++++ hw/i386/acpi-build.c | 21 +++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-)