Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74712?usp=email )
Change subject: soc/amd/picasso/chip: use common data fabric domain resource code ......................................................................
soc/amd/picasso/chip: use common data fabric domain resource code
Use amd_pci_domain_read_resources function that gets the configured MMIO regions for the PCI root domain from the data fabric's MMIO decode registers instead of using pci_domain_read_resources. This results in the same IO port range being used by the allocator, but makes sure that the allocator will only allocate non-fixed MMIO resources in the address ranges that get decoded to the PCI root complex. In order for the PCI0 _CRS ACPI resource template to match the decoded PCI root domain MMIO windows, use amd_pci_domain_fill_ssdt to generate the _CRS ACPI code instead of having a mostly hard-coded _CRS method in the DSDT. This makes sure that the OS will know about the MMIO regions it is allowed to used.
Before this patch, only the region from TOM1 to right below CONFIG_ECAM_MMCONF_BASE_ADDRESS was advertised as usable PCI MMIO in the PCI0 _CRS method. Also the resource allocator didn't get any constraint on which address ranges it can use to put the non-fixed MMIO resources. This approach worked until now, since all address range from 0 up to right below TOM1 was filled with either usable or reserved memory and the allocator was allocating beginning right from TOM1, since it was using the bottom-up allocation approach and everything below TOM1 was already in use. The MMIO region from TOM1 to right below CONFIG_ECAM_MMCONF_BASE_ADDRESS also matched the MMIO decode window configured in the data fabric's MMIO decode registers, so everything seemed to work fine. However, when either selecting RESOURCE_ALLOCATION_TOP_DOWN or enabling above 4GB MMIO, things broke badly. This was partially due to the allocator putting non-fixed MMIO resources in regions that weren't decoded to the PCI root, since AMD family 17h and 19h silicon doesn't subtractively decode PCI MMIO and the wrong ranges the allocator used also weren't advertised in ACPI.
TEST=Even when selecting RESOURCE_ALLOCATION_TOP_DOWN that usually ends up with a non-working system when the MMIO ranges aren't reported correctly to the resource allocator due to the reasons descried above, Ubuntu 22.04 LTS still boots on Mandolin both with SeaBIOS and EDK2 payload and Windows 10 boots with EDK payload. There's however an EDK2 bug that results the MMCONFIG region not being advertised in the e820 table, which causes Linux to not use the MMCONFIG and fall back to the legacy PCI config access method. This only happens with EDK2 payload and everything works fine when using SeaBIOS as payload. That e820 issue is unaffected by this patch.
At the end of the data_fabric_set_mmio_np call, this is the data fabric MMIO register configuration:
=== Data Fabric MMIO configuration registers === idx base limit control R W NP F-ID 0 fc000000 febfffff 93 x x 9 1 10000000000 ffffffffffff 93 x x 9 2 d0000000 f7ffffff 93 x x 9 3 fed00000 fedfffff 1093 x x x 9 4 0 ffff 90 9 5 0 ffff 90 9 6 0 ffff 90 9 7 0 ffff 90 9
The limit of the data fabric MMIO decode register 1 is configured as 0xffffffffffff although this is way beyond the addressable memory space. add_data_fabric_mmio_regions fixes this up, so the range that gets passed to the allocator in that case is 0x7fcffffffff which takes both the reserved most significant address bits used for the memory encryption and the 12GB reserved data fabric MMIO at the top of the usable address space into account.
This results in the following domain ranges passed to the resource allocator:
DOMAIN: 0000 io: base: 0 size: 0 align: 0 gran: 0 limit: ffff done DOMAIN: 0000 mem: base: fc000000 size: 0 align: 0 gran: 0 limit: febfffff DOMAIN: 0000 mem: base: 10000000000 size: 0 align: 0 gran: 0 limit: 7fcffffffff DOMAIN: 0000 mem: base: d0000000 size: 0 align: 0 gran: 0 limit: f7ffffff
The IO resource producer region is split into two parts to not cover the PCI config IO region resource consumer. This results in these resources being added to the PCI0 _CRS resource template:
amd_pci_domain_fill_ssdt ACPI scope: '_SB.PCI0' PCI0 _CRS: adding busses [0-3f] PCI0 _CRS: adding IO range [0-cf7] PCI0 _CRS: adding IO range [d00-ffff] PCI0 _CRS: adding MMIO range [fc000000-febfffff] PCI0 _CRS: adding MMIO range [10000000000-7fcffffffff] PCI0 _CRS: adding MMIO range [d0000000-f7ffffff] PCI0 _CRS: adding VGA resource
Kernel version 5.15.0-43 from Ubuntu 2022.4 LTS prints this in dmesg:
PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-3f] pci_bus 0000:00: root bus resource [io 0x0000-0x0cf7 window] pci_bus 0000:00: root bus resource [io 0x0d00-0xffff window] pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000bffff window] pci_bus 0000:00: root bus resource [mem 0xd0000000-0xf7ffffff window] pci_bus 0000:00: root bus resource [mem 0xfc000000-0xfebfffff window] pci_bus 0000:00: root bus resource [mem 0x10000000000-0x7fcffffffff window]
Another noteworthy thing I wasn't aware of at first when testing ACPI changes on Windows 10 is that a normal Windows shutdown and boot cycle won't result in it processing the changed ACPI tables; you have to tell it to reboot to do a proper full boot where it will process the updated ACPI tables (and fail if it dislikes something about the ACPI tables and bytecode).
Signed-off-by: Felix Held felix-coreboot@felixheld.de Change-Id: Ia24930ec2a9962dd15e874e9defea441cffae9f2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/74712 Reviewed-by: Raul Rangel rrangel@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi/northbridge.asl M src/soc/amd/picasso/acpi/sb_pci0_fch.asl M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/root_complex.c 5 files changed, 6 insertions(+), 80 deletions(-)
Approvals: Arthur Heymans: Looks good to me, approved build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 11410ae..9738d19 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -38,6 +38,7 @@ select SOC_AMD_COMMON_BLOCK_BANKED_GPIOS select SOC_AMD_COMMON_BLOCK_CPUFREQ_FAM17H_19H select SOC_AMD_COMMON_BLOCK_DATA_FABRIC + select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN select SOC_AMD_COMMON_BLOCK_GRAPHICS select SOC_AMD_COMMON_BLOCK_HAS_ESPI select SOC_AMD_COMMON_BLOCK_HDA diff --git a/src/soc/amd/picasso/acpi/northbridge.asl b/src/soc/amd/picasso/acpi/northbridge.asl index 99d04b5..688f138 100644 --- a/src/soc/amd/picasso/acpi/northbridge.asl +++ b/src/soc/amd/picasso/acpi/northbridge.asl @@ -1,8 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
/* Note: Only need HID on Primary Bus */ -External (TOM1) -External (TOM2) Name(_HID, EISAID("PNP0A08")) /* PCI Express Root Bridge */ Name(_CID, EISAID("PNP0A03")) /* PCI Root Bridge */
diff --git a/src/soc/amd/picasso/acpi/sb_pci0_fch.asl b/src/soc/amd/picasso/acpi/sb_pci0_fch.asl index 1f2c0d9..898914c 100644 --- a/src/soc/amd/picasso/acpi/sb_pci0_fch.asl +++ b/src/soc/amd/picasso/acpi/sb_pci0_fch.asl @@ -1,7 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-#include <arch/ioapic.h> - /* System Bus */ /* _SB.PCI0 */
@@ -23,74 +21,3 @@ /* 0:14.3 - LPC */ #include <soc/amd/common/acpi/lpc.asl> #include <soc/amd/common/acpi/platform.asl> - -Name(CRES, ResourceTemplate() { - /* Set the Bus number and Secondary Bus number for the PCI0 device - * The Secondary bus range for PCI0 lets the system - * know what bus values are allowed on the downstream - * side of this PCI bus if there is a PCI-PCI bridge. - * PCI buses can have 256 secondary buses which - * range from [0-0xFF] but they do not need to be - * sequential. - */ - WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, - 0x0000, /* address granularity */ - 0x0000, /* range minimum */ - 0x00ff, /* range maximum */ - 0x0000, /* translation */ - 0x0100, /* length */ - ,, PSB0) /* ResourceSourceIndex, ResourceSource, DescriptorName */ - - IO(Decode16, 0x0cf8, 0x0cf8, 1, 8) - - WORDIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange, - 0x0000, /* address granularity */ - 0x0000, /* range minimum */ - 0x0cf7, /* range maximum */ - 0x0000, /* translation */ - 0x0cf8 /* length */ - ) - - WORDIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange, - 0x0000, /* address granularity */ - 0x0d00, /* range minimum */ - 0xffff, /* range maximum */ - 0x0000, /* translation */ - 0xf300 /* length */ - ) - - /* VGA memory (0xa0000-0xbffff) */ - DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, - Cacheable, ReadWrite, - 0x00000000, 0x000a0000, 0x000bffff, 0x00000000, - 0x00020000) - DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, - Cacheable, ReadOnly, - 0x00000000, 0x000c0000, 0x000dffff, 0x00000000, - 0x00020000) - - /* memory space for PCI BARs below 4GB */ - DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, - NonCacheable, ReadWrite, - 0x00000000, 0x00000000, 0x00000000, 0x00000000, - 0x00000000,,, PM01) -}) /* End Name(_SB.PCI0.CRES) */ - -Method(_CRS, 0) { - /* Find PCI resource area in CRES */ - CreateDwordField (CRES, ^PM01._MIN, P1MN) - CreateDwordField (CRES, ^PM01._MAX, P1MX) - CreateDwordField (CRES, ^PM01._LEN, P1LN) - - /* Declare memory between TOM1 and MMCONF as available for PCI MMIO. */ - P1MN = TOM1 - P1MX = CONFIG_ECAM_MMCONF_BASE_ADDRESS - 1 - P1LN = P1MX - P1MN + 1 - - CreateWordField(CRES, ^PSB0._MAX, BMAX) - CreateWordField(CRES, ^PSB0._LEN, BLEN) - BMAX = CONFIG_ECAM_MMCONF_BUS_NUMBER - 1 - BLEN = CONFIG_ECAM_MMCONF_BUS_NUMBER - - Return(CRES) /* note to change the Name buffer */ -} /* end of Method(_SB.PCI0._CRS) */ diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c index 782c1c3..067c4a07 100644 --- a/src/soc/amd/picasso/chip.c +++ b/src/soc/amd/picasso/chip.c @@ -27,10 +27,11 @@ };
struct device_operations picasso_pci_domain_ops = { - .read_resources = pci_domain_read_resources, - .set_resources = pci_domain_set_resources, - .scan_bus = pci_domain_scan_bus, - .acpi_name = soc_acpi_name, + .read_resources = amd_pci_domain_read_resources, + .set_resources = pci_domain_set_resources, + .scan_bus = amd_pci_domain_scan_bus, + .acpi_name = soc_acpi_name, + .acpi_fill_ssdt = amd_pci_domain_fill_ssdt, };
static void soc_init(void *chip_info) diff --git a/src/soc/amd/picasso/root_complex.c b/src/soc/amd/picasso/root_complex.c index 65021a3..7a83197 100644 --- a/src/soc/amd/picasso/root_complex.c +++ b/src/soc/amd/picasso/root_complex.c @@ -196,7 +196,6 @@
static void root_complex_fill_ssdt(const struct device *device) { - acpi_fill_root_complex_tom(device); if (CONFIG(SOC_AMD_COMMON_BLOCK_ACPI_DPTC)) acipgen_dptci(); }