tests/bios-tables-test-allowed-diff.h | 8 +++++++ hw/i386/acpi-build.c | 14 ++++++++++++ hw/pci-host/q35.c | 31 +++++++-------------------- 3 files changed, 30 insertions(+), 23 deletions(-)
This patch changes the handling of the mmconfig area. Thanks to the
pci(e) expander devices we already have the logic to exclude address
ranges from PCI0._CRS. We can simply add the mmconfig address range
to the list get it excluded as well.
With that in place we can go with a fixed pci hole which covers the
whole area from the end of (low) ram to the ioapic.
This will make the whole logic alot less fragile. No matter where the
firmware places the mmconfig xbar, things should work correctly. The
guest also gets a bit more PCI address space (seabios boot):
# cat /proc/iomem
[ ... ]
7ffdd000-7fffffff : reserved
80000000-afffffff : PCI Bus 0000:00 <<-- this is new
b0000000-bfffffff : PCI MMCONFIG 0000 [bus 00-ff]
b0000000-bfffffff : reserved
c0000000-febfffff : PCI Bus 0000:00
f8000000-fbffffff : 0000:00:01.0
[ ... ]
So this is a guest visible change.
Cc: László Érsek <lersek@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
tests/bios-tables-test-allowed-diff.h | 8 +++++++
hw/i386/acpi-build.c | 14 ++++++++++++
hw/pci-host/q35.c | 31 +++++++--------------------
3 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
index dfb8523c8bf4..3bbd22c62a3b 100644
--- a/tests/bios-tables-test-allowed-diff.h
+++ b/tests/bios-tables-test-allowed-diff.h
@@ -1 +1,9 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/DSDT",
+"tests/data/acpi/q35/DSDT.bridge",
+"tests/data/acpi/q35/DSDT.mmio64",
+"tests/data/acpi/q35/DSDT.ipmibt",
+"tests/data/acpi/q35/DSDT.cphp",
+"tests/data/acpi/q35/DSDT.memhp",
+"tests/data/acpi/q35/DSDT.numamem",
+"tests/data/acpi/q35/DSDT.dimmpxm",
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 85dc1640bc67..8e4f26977619 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -122,6 +122,8 @@ typedef struct FwCfgTPMConfig {
uint8_t tpmppi_version;
} QEMU_PACKED FwCfgTPMConfig;
+static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg);
+
static void init_common_fadt_data(Object *o, AcpiFadtData *data)
{
uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
@@ -1807,6 +1809,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
CrsRangeSet crs_range_set;
PCMachineState *pcms = PC_MACHINE(machine);
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
+ AcpiMcfgInfo mcfg;
uint32_t nr_mem = machine->ram_slots;
int root_bus_limit = 0xFF;
PCIBus *bus = NULL;
@@ -1921,6 +1924,17 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
}
}
+ /*
+ * At this point crs_range_set has all the ranges used by pci
+ * busses *other* than PCI0. These ranges will be excluded from
+ * the PCI0._CRS. Add mmconfig to the set so it will be excluded
+ * too.
+ */
+ if (acpi_get_mcfg(&mcfg)) {
+ crs_range_insert(crs_range_set.mem_ranges,
+ mcfg.base, mcfg.base + mcfg.size - 1);
+ }
+
scope = aml_scope("\\_SB.PCI0");
/* build PCI0._CRS */
crs = aml_resource_template();
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 960939f5ed3e..72093320befe 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -258,15 +258,6 @@ static void q35_host_initfn(Object *obj)
object_property_add_link(obj, MCH_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
(Object **) &s->mch.address_space_io,
qdev_prop_allow_set_link_before_realize, 0, NULL);
-
- /* Leave enough space for the biggest MCFG BAR */
- /* TODO: this matches current bios behaviour, but
- * it's not a power of two, which means an MTRR
- * can't cover it exactly.
- */
- range_set_bounds(&s->mch.pci_hole,
- MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + MCH_HOST_BRIDGE_PCIEXBAR_MAX,
- IO_APIC_DEFAULT_ADDRESS - 1);
}
static const TypeInfo q35_host_info = {
@@ -338,20 +329,6 @@ static void mch_update_pciexbar(MCHPCIState *mch)
}
addr = pciexbar & addr_mask;
pcie_host_mmcfg_update(pehb, enable, addr, length);
- /* Leave enough space for the MCFG BAR */
- /*
- * TODO: this matches current bios behaviour, but it's not a power of two,
- * which means an MTRR can't cover it exactly.
- */
- if (enable) {
- range_set_bounds(&mch->pci_hole,
- addr + length,
- IO_APIC_DEFAULT_ADDRESS - 1);
- } else {
- range_set_bounds(&mch->pci_hole,
- MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT,
- IO_APIC_DEFAULT_ADDRESS - 1);
- }
}
/* PAM */
@@ -484,6 +461,14 @@ static void mch_update(MCHPCIState *mch)
mch_update_pam(mch);
mch_update_smram(mch);
mch_update_ext_tseg_mbytes(mch);
+
+ /*
+ * pci hole goes from end-of-low-ram to io-apic.
+ * mmconfig will be excluded by the dsdt builder.
+ */
+ range_set_bounds(&mch->pci_hole,
+ mch->below_4g_mem_size,
+ IO_APIC_DEFAULT_ADDRESS - 1);
}
static int mch_post_load(void *opaque, int version_id)
--
2.18.1
On 06/07/19 09:34, Gerd Hoffmann wrote:
> This patch changes the handling of the mmconfig area. Thanks to the
> pci(e) expander devices we already have the logic to exclude address
> ranges from PCI0._CRS. We can simply add the mmconfig address range
> to the list get it excluded as well.
>
> With that in place we can go with a fixed pci hole which covers the
> whole area from the end of (low) ram to the ioapic.
>
> This will make the whole logic alot less fragile. No matter where the
> firmware places the mmconfig xbar, things should work correctly. The
> guest also gets a bit more PCI address space (seabios boot):
>
> # cat /proc/iomem
> [ ... ]
> 7ffdd000-7fffffff : reserved
> 80000000-afffffff : PCI Bus 0000:00 <<-- this is new
> b0000000-bfffffff : PCI MMCONFIG 0000 [bus 00-ff]
> b0000000-bfffffff : reserved
> c0000000-febfffff : PCI Bus 0000:00
> f8000000-fbffffff : 0000:00:01.0
> [ ... ]
>
> So this is a guest visible change.
>
> Cc: László Érsek <lersek@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/bios-tables-test-allowed-diff.h | 8 +++++++
> hw/i386/acpi-build.c | 14 ++++++++++++
> hw/pci-host/q35.c | 31 +++++++--------------------
> 3 files changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
> index dfb8523c8bf4..3bbd22c62a3b 100644
> --- a/tests/bios-tables-test-allowed-diff.h
> +++ b/tests/bios-tables-test-allowed-diff.h
> @@ -1 +1,9 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT",
> +"tests/data/acpi/q35/DSDT.bridge",
> +"tests/data/acpi/q35/DSDT.mmio64",
> +"tests/data/acpi/q35/DSDT.ipmibt",
> +"tests/data/acpi/q35/DSDT.cphp",
> +"tests/data/acpi/q35/DSDT.memhp",
> +"tests/data/acpi/q35/DSDT.numamem",
> +"tests/data/acpi/q35/DSDT.dimmpxm",
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 85dc1640bc67..8e4f26977619 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -122,6 +122,8 @@ typedef struct FwCfgTPMConfig {
> uint8_t tpmppi_version;
> } QEMU_PACKED FwCfgTPMConfig;
>
> +static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg);
> +
> static void init_common_fadt_data(Object *o, AcpiFadtData *data)
> {
> uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> @@ -1807,6 +1809,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> CrsRangeSet crs_range_set;
> PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> + AcpiMcfgInfo mcfg;
> uint32_t nr_mem = machine->ram_slots;
> int root_bus_limit = 0xFF;
> PCIBus *bus = NULL;
> @@ -1921,6 +1924,17 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> }
> }
>
> + /*
> + * At this point crs_range_set has all the ranges used by pci
> + * busses *other* than PCI0. These ranges will be excluded from
> + * the PCI0._CRS. Add mmconfig to the set so it will be excluded
> + * too.
> + */
> + if (acpi_get_mcfg(&mcfg)) {
> + crs_range_insert(crs_range_set.mem_ranges,
> + mcfg.base, mcfg.base + mcfg.size - 1);
> + }
> +
> scope = aml_scope("\\_SB.PCI0");
> /* build PCI0._CRS */
> crs = aml_resource_template();
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 960939f5ed3e..72093320befe 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -258,15 +258,6 @@ static void q35_host_initfn(Object *obj)
> object_property_add_link(obj, MCH_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
> (Object **) &s->mch.address_space_io,
> qdev_prop_allow_set_link_before_realize, 0, NULL);
> -
> - /* Leave enough space for the biggest MCFG BAR */
> - /* TODO: this matches current bios behaviour, but
> - * it's not a power of two, which means an MTRR
> - * can't cover it exactly.
> - */
> - range_set_bounds(&s->mch.pci_hole,
> - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + MCH_HOST_BRIDGE_PCIEXBAR_MAX,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> }
>
> static const TypeInfo q35_host_info = {
> @@ -338,20 +329,6 @@ static void mch_update_pciexbar(MCHPCIState *mch)
> }
> addr = pciexbar & addr_mask;
> pcie_host_mmcfg_update(pehb, enable, addr, length);
> - /* Leave enough space for the MCFG BAR */
> - /*
> - * TODO: this matches current bios behaviour, but it's not a power of two,
> - * which means an MTRR can't cover it exactly.
> - */
> - if (enable) {
> - range_set_bounds(&mch->pci_hole,
> - addr + length,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> - } else {
> - range_set_bounds(&mch->pci_hole,
> - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> - }
> }
>
> /* PAM */
> @@ -484,6 +461,14 @@ static void mch_update(MCHPCIState *mch)
> mch_update_pam(mch);
> mch_update_smram(mch);
> mch_update_ext_tseg_mbytes(mch);
> +
> + /*
> + * pci hole goes from end-of-low-ram to io-apic.
> + * mmconfig will be excluded by the dsdt builder.
> + */
> + range_set_bounds(&mch->pci_hole,
> + mch->below_4g_mem_size,
> + IO_APIC_DEFAULT_ADDRESS - 1);
> }
>
> static int mch_post_load(void *opaque, int version_id)
>
For other reviewers: the only change relative to v1 is the
"tests/bios-tables-test-allowed-diff.h" hunk, which is new in v2.
Acked-by: Laszlo Ersek <lersek@redhat.com>
Laszlo
On 6/7/19 10:34 AM, Gerd Hoffmann wrote:
> This patch changes the handling of the mmconfig area. Thanks to the
> pci(e) expander devices we already have the logic to exclude address
> ranges from PCI0._CRS. We can simply add the mmconfig address range
> to the list get it excluded as well.
>
> With that in place we can go with a fixed pci hole which covers the
> whole area from the end of (low) ram to the ioapic.
>
> This will make the whole logic alot less fragile. No matter where the
> firmware places the mmconfig xbar, things should work correctly. The
> guest also gets a bit more PCI address space (seabios boot):
>
> # cat /proc/iomem
> [ ... ]
> 7ffdd000-7fffffff : reserved
> 80000000-afffffff : PCI Bus 0000:00 <<-- this is new
> b0000000-bfffffff : PCI MMCONFIG 0000 [bus 00-ff]
> b0000000-bfffffff : reserved
> c0000000-febfffff : PCI Bus 0000:00
> f8000000-fbffffff : 0000:00:01.0
> [ ... ]
>
> So this is a guest visible change.
>
> Cc: László Érsek <lersek@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/bios-tables-test-allowed-diff.h | 8 +++++++
> hw/i386/acpi-build.c | 14 ++++++++++++
> hw/pci-host/q35.c | 31 +++++++--------------------
> 3 files changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
> index dfb8523c8bf4..3bbd22c62a3b 100644
> --- a/tests/bios-tables-test-allowed-diff.h
> +++ b/tests/bios-tables-test-allowed-diff.h
> @@ -1 +1,9 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT",
> +"tests/data/acpi/q35/DSDT.bridge",
> +"tests/data/acpi/q35/DSDT.mmio64",
> +"tests/data/acpi/q35/DSDT.ipmibt",
> +"tests/data/acpi/q35/DSDT.cphp",
> +"tests/data/acpi/q35/DSDT.memhp",
> +"tests/data/acpi/q35/DSDT.numamem",
> +"tests/data/acpi/q35/DSDT.dimmpxm",
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 85dc1640bc67..8e4f26977619 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -122,6 +122,8 @@ typedef struct FwCfgTPMConfig {
> uint8_t tpmppi_version;
> } QEMU_PACKED FwCfgTPMConfig;
>
> +static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg);
> +
> static void init_common_fadt_data(Object *o, AcpiFadtData *data)
> {
> uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> @@ -1807,6 +1809,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> CrsRangeSet crs_range_set;
> PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> + AcpiMcfgInfo mcfg;
> uint32_t nr_mem = machine->ram_slots;
> int root_bus_limit = 0xFF;
> PCIBus *bus = NULL;
> @@ -1921,6 +1924,17 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> }
> }
>
> + /*
> + * At this point crs_range_set has all the ranges used by pci
> + * busses *other* than PCI0. These ranges will be excluded from
> + * the PCI0._CRS. Add mmconfig to the set so it will be excluded
> + * too.
> + */
> + if (acpi_get_mcfg(&mcfg)) {
> + crs_range_insert(crs_range_set.mem_ranges,
> + mcfg.base, mcfg.base + mcfg.size - 1);
> + }
> +
> scope = aml_scope("\\_SB.PCI0");
> /* build PCI0._CRS */
> crs = aml_resource_template();
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 960939f5ed3e..72093320befe 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -258,15 +258,6 @@ static void q35_host_initfn(Object *obj)
> object_property_add_link(obj, MCH_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
> (Object **) &s->mch.address_space_io,
> qdev_prop_allow_set_link_before_realize, 0, NULL);
> -
> - /* Leave enough space for the biggest MCFG BAR */
> - /* TODO: this matches current bios behaviour, but
> - * it's not a power of two, which means an MTRR
> - * can't cover it exactly.
> - */
> - range_set_bounds(&s->mch.pci_hole,
> - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + MCH_HOST_BRIDGE_PCIEXBAR_MAX,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> }
>
> static const TypeInfo q35_host_info = {
> @@ -338,20 +329,6 @@ static void mch_update_pciexbar(MCHPCIState *mch)
> }
> addr = pciexbar & addr_mask;
> pcie_host_mmcfg_update(pehb, enable, addr, length);
> - /* Leave enough space for the MCFG BAR */
> - /*
> - * TODO: this matches current bios behaviour, but it's not a power of two,
> - * which means an MTRR can't cover it exactly.
> - */
> - if (enable) {
> - range_set_bounds(&mch->pci_hole,
> - addr + length,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> - } else {
> - range_set_bounds(&mch->pci_hole,
> - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> - }
> }
>
> /* PAM */
> @@ -484,6 +461,14 @@ static void mch_update(MCHPCIState *mch)
> mch_update_pam(mch);
> mch_update_smram(mch);
> mch_update_ext_tseg_mbytes(mch);
> +
> + /*
> + * pci hole goes from end-of-low-ram to io-apic.
> + * mmconfig will be excluded by the dsdt builder.
> + */
> + range_set_bounds(&mch->pci_hole,
> + mch->below_4g_mem_size,
> + IO_APIC_DEFAULT_ADDRESS - 1);
> }
>
> static int mch_post_load(void *opaque, int version_id)
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Thanks,
Marcel
On 07/06/19 09:34, Gerd Hoffmann wrote:
> This patch changes the handling of the mmconfig area. Thanks to the
> pci(e) expander devices we already have the logic to exclude address
> ranges from PCI0._CRS. We can simply add the mmconfig address range
> to the list get it excluded as well.
>
> With that in place we can go with a fixed pci hole which covers the
> whole area from the end of (low) ram to the ioapic.
>
> This will make the whole logic alot less fragile. No matter where the
> firmware places the mmconfig xbar, things should work correctly. The
> guest also gets a bit more PCI address space (seabios boot):
>
> # cat /proc/iomem
> [ ... ]
> 7ffdd000-7fffffff : reserved
> 80000000-afffffff : PCI Bus 0000:00 <<-- this is new
> b0000000-bfffffff : PCI MMCONFIG 0000 [bus 00-ff]
> b0000000-bfffffff : reserved
> c0000000-febfffff : PCI Bus 0000:00
> f8000000-fbffffff : 0000:00:01.0
> [ ... ]
>
> So this is a guest visible change.
>
> Cc: László Érsek <lersek@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/bios-tables-test-allowed-diff.h | 8 +++++++
> hw/i386/acpi-build.c | 14 ++++++++++++
> hw/pci-host/q35.c | 31 +++++++--------------------
> 3 files changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
> index dfb8523c8bf4..3bbd22c62a3b 100644
> --- a/tests/bios-tables-test-allowed-diff.h
> +++ b/tests/bios-tables-test-allowed-diff.h
> @@ -1 +1,9 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT",
> +"tests/data/acpi/q35/DSDT.bridge",
> +"tests/data/acpi/q35/DSDT.mmio64",
> +"tests/data/acpi/q35/DSDT.ipmibt",
> +"tests/data/acpi/q35/DSDT.cphp",
> +"tests/data/acpi/q35/DSDT.memhp",
> +"tests/data/acpi/q35/DSDT.numamem",
> +"tests/data/acpi/q35/DSDT.dimmpxm",
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 85dc1640bc67..8e4f26977619 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -122,6 +122,8 @@ typedef struct FwCfgTPMConfig {
> uint8_t tpmppi_version;
> } QEMU_PACKED FwCfgTPMConfig;
>
> +static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg);
> +
> static void init_common_fadt_data(Object *o, AcpiFadtData *data)
> {
> uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> @@ -1807,6 +1809,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> CrsRangeSet crs_range_set;
> PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> + AcpiMcfgInfo mcfg;
> uint32_t nr_mem = machine->ram_slots;
> int root_bus_limit = 0xFF;
> PCIBus *bus = NULL;
> @@ -1921,6 +1924,17 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> }
> }
>
> + /*
> + * At this point crs_range_set has all the ranges used by pci
> + * busses *other* than PCI0. These ranges will be excluded from
> + * the PCI0._CRS. Add mmconfig to the set so it will be excluded
> + * too.
> + */
> + if (acpi_get_mcfg(&mcfg)) {
> + crs_range_insert(crs_range_set.mem_ranges,
> + mcfg.base, mcfg.base + mcfg.size - 1);
> + }
> +
> scope = aml_scope("\\_SB.PCI0");
> /* build PCI0._CRS */
> crs = aml_resource_template();
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 960939f5ed3e..72093320befe 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -258,15 +258,6 @@ static void q35_host_initfn(Object *obj)
> object_property_add_link(obj, MCH_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
> (Object **) &s->mch.address_space_io,
> qdev_prop_allow_set_link_before_realize, 0, NULL);
> -
> - /* Leave enough space for the biggest MCFG BAR */
> - /* TODO: this matches current bios behaviour, but
> - * it's not a power of two, which means an MTRR
> - * can't cover it exactly.
> - */
> - range_set_bounds(&s->mch.pci_hole,
> - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + MCH_HOST_BRIDGE_PCIEXBAR_MAX,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> }
>
> static const TypeInfo q35_host_info = {
> @@ -338,20 +329,6 @@ static void mch_update_pciexbar(MCHPCIState *mch)
> }
> addr = pciexbar & addr_mask;
> pcie_host_mmcfg_update(pehb, enable, addr, length);
> - /* Leave enough space for the MCFG BAR */
> - /*
> - * TODO: this matches current bios behaviour, but it's not a power of two,
> - * which means an MTRR can't cover it exactly.
> - */
> - if (enable) {
> - range_set_bounds(&mch->pci_hole,
> - addr + length,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> - } else {
> - range_set_bounds(&mch->pci_hole,
> - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT,
> - IO_APIC_DEFAULT_ADDRESS - 1);
> - }
> }
>
> /* PAM */
> @@ -484,6 +461,14 @@ static void mch_update(MCHPCIState *mch)
> mch_update_pam(mch);
> mch_update_smram(mch);
> mch_update_ext_tseg_mbytes(mch);
> +
> + /*
> + * pci hole goes from end-of-low-ram to io-apic.
> + * mmconfig will be excluded by the dsdt builder.
> + */
> + range_set_bounds(&mch->pci_hole,
> + mch->below_4g_mem_size,
> + IO_APIC_DEFAULT_ADDRESS - 1);
> }
>
> static int mch_post_load(void *opaque, int version_id)
>
Queued, thanks.
Paolo
On Tue, Jun 11, 2019 at 01:37:35PM +0200, Paolo Bonzini wrote:
> On 07/06/19 09:34, Gerd Hoffmann wrote:
> > This patch changes the handling of the mmconfig area. Thanks to the
> > pci(e) expander devices we already have the logic to exclude address
> > ranges from PCI0._CRS. We can simply add the mmconfig address range
> > to the list get it excluded as well.
> >
> > With that in place we can go with a fixed pci hole which covers the
> > whole area from the end of (low) ram to the ioapic.
> >
> > This will make the whole logic alot less fragile. No matter where the
> > firmware places the mmconfig xbar, things should work correctly. The
> > guest also gets a bit more PCI address space (seabios boot):
> >
> > # cat /proc/iomem
> > [ ... ]
> > 7ffdd000-7fffffff : reserved
> > 80000000-afffffff : PCI Bus 0000:00 <<-- this is new
> > b0000000-bfffffff : PCI MMCONFIG 0000 [bus 00-ff]
> > b0000000-bfffffff : reserved
> > c0000000-febfffff : PCI Bus 0000:00
> > f8000000-fbffffff : 0000:00:01.0
> > [ ... ]
> >
> > So this is a guest visible change.
> >
> > Cc: László Érsek <lersek@redhat.com>
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > tests/bios-tables-test-allowed-diff.h | 8 +++++++
> > hw/i386/acpi-build.c | 14 ++++++++++++
> > hw/pci-host/q35.c | 31 +++++++--------------------
> > 3 files changed, 30 insertions(+), 23 deletions(-)
> >
> > diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
> > index dfb8523c8bf4..3bbd22c62a3b 100644
> > --- a/tests/bios-tables-test-allowed-diff.h
> > +++ b/tests/bios-tables-test-allowed-diff.h
> > @@ -1 +1,9 @@
> > /* List of comma-separated changed AML files to ignore */
> > +"tests/data/acpi/q35/DSDT",
> > +"tests/data/acpi/q35/DSDT.bridge",
> > +"tests/data/acpi/q35/DSDT.mmio64",
> > +"tests/data/acpi/q35/DSDT.ipmibt",
> > +"tests/data/acpi/q35/DSDT.cphp",
> > +"tests/data/acpi/q35/DSDT.memhp",
> > +"tests/data/acpi/q35/DSDT.numamem",
> > +"tests/data/acpi/q35/DSDT.dimmpxm",
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 85dc1640bc67..8e4f26977619 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -122,6 +122,8 @@ typedef struct FwCfgTPMConfig {
> > uint8_t tpmppi_version;
> > } QEMU_PACKED FwCfgTPMConfig;
> >
> > +static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg);
> > +
> > static void init_common_fadt_data(Object *o, AcpiFadtData *data)
> > {
> > uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> > @@ -1807,6 +1809,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > CrsRangeSet crs_range_set;
> > PCMachineState *pcms = PC_MACHINE(machine);
> > PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> > + AcpiMcfgInfo mcfg;
> > uint32_t nr_mem = machine->ram_slots;
> > int root_bus_limit = 0xFF;
> > PCIBus *bus = NULL;
> > @@ -1921,6 +1924,17 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > }
> > }
> >
> > + /*
> > + * At this point crs_range_set has all the ranges used by pci
> > + * busses *other* than PCI0. These ranges will be excluded from
> > + * the PCI0._CRS. Add mmconfig to the set so it will be excluded
> > + * too.
> > + */
> > + if (acpi_get_mcfg(&mcfg)) {
> > + crs_range_insert(crs_range_set.mem_ranges,
> > + mcfg.base, mcfg.base + mcfg.size - 1);
> > + }
> > +
> > scope = aml_scope("\\_SB.PCI0");
> > /* build PCI0._CRS */
> > crs = aml_resource_template();
> > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > index 960939f5ed3e..72093320befe 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -258,15 +258,6 @@ static void q35_host_initfn(Object *obj)
> > object_property_add_link(obj, MCH_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
> > (Object **) &s->mch.address_space_io,
> > qdev_prop_allow_set_link_before_realize, 0, NULL);
> > -
> > - /* Leave enough space for the biggest MCFG BAR */
> > - /* TODO: this matches current bios behaviour, but
> > - * it's not a power of two, which means an MTRR
> > - * can't cover it exactly.
> > - */
> > - range_set_bounds(&s->mch.pci_hole,
> > - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + MCH_HOST_BRIDGE_PCIEXBAR_MAX,
> > - IO_APIC_DEFAULT_ADDRESS - 1);
> > }
> >
> > static const TypeInfo q35_host_info = {
> > @@ -338,20 +329,6 @@ static void mch_update_pciexbar(MCHPCIState *mch)
> > }
> > addr = pciexbar & addr_mask;
> > pcie_host_mmcfg_update(pehb, enable, addr, length);
> > - /* Leave enough space for the MCFG BAR */
> > - /*
> > - * TODO: this matches current bios behaviour, but it's not a power of two,
> > - * which means an MTRR can't cover it exactly.
> > - */
> > - if (enable) {
> > - range_set_bounds(&mch->pci_hole,
> > - addr + length,
> > - IO_APIC_DEFAULT_ADDRESS - 1);
> > - } else {
> > - range_set_bounds(&mch->pci_hole,
> > - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT,
> > - IO_APIC_DEFAULT_ADDRESS - 1);
> > - }
> > }
> >
> > /* PAM */
> > @@ -484,6 +461,14 @@ static void mch_update(MCHPCIState *mch)
> > mch_update_pam(mch);
> > mch_update_smram(mch);
> > mch_update_ext_tseg_mbytes(mch);
> > +
> > + /*
> > + * pci hole goes from end-of-low-ram to io-apic.
> > + * mmconfig will be excluded by the dsdt builder.
> > + */
> > + range_set_bounds(&mch->pci_hole,
> > + mch->below_4g_mem_size,
> > + IO_APIC_DEFAULT_ADDRESS - 1);
> > }
> >
> > static int mch_post_load(void *opaque, int version_id)
> >
>
> Queued, thanks.
>
> Paolo
Paolo I would prefer to queue ACPI changes through my tree.
In particular there's a dance with expected tables I am
still working out.
--
MST
© 2016 - 2024 Red Hat, Inc.