[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMC
From: |
Igor Mammedov |
Subject: |
Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region |
Date: |
Fri, 12 Feb 2021 16:40:38 +0100 |
On Wed, 10 Feb 2021 22:46:43 -0800
Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> Declare PNP0C01 device to reserve MMCONFIG region to conform to the
> spec better and play nice with guest BIOSes/OSes.
>
> According to PCI Firmware Specification[0], MMCONFIG region must be
> reserved by declaring a motherboard resource. It's optional to reserve
> the region in memory map by Int 15 E820h or EFIGetMemoryMap.
> Guest Linux checks if the MMCFG region is reserved by bios memory map
> or ACPI resource. If it's not reserved, Linux falls back to legacy PCI
> configuration access.
>
> TDVF [1] [2] doesn't reserve MMCONFIG the region in memory map.
> On the other hand OVMF reserves it in memory map without declaring a
> motherboard resource. With memory map reservation, linux guest uses
> MMCONFIG region. However it doesn't comply to PCI Firmware
> specification.
>
> [0] PCI Firmware specification Revision 3.2
> 4.1.2 MCFG Table Description table 4-2 NOTE 2
> If the operating system does not natively comprehend reserving the
> MMCFG region, The MMCFG region must e reserved by firmware. ...
> For most systems, the mortheroard resource would appear at the root
> of the ACPI namespace (under \_SB)...
> The resource can optionally be returned in Int15 E820h or
> EFIGetMemoryMap as reserved memory but must always be reported
> through ACPI as a motherboard resource
>
> [1] TDX: Intel Trust Domain Extension
>
> https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html
> [2] TDX Virtual Firmware
> https://github.com/tianocore/edk2-staging/tree/TDVF
>
> The change to DSDT is as follows.
> @@ -68,32 +68,51 @@
>
> If ((CDW3 != Local0))
> {
> CDW1 |= 0x10
> }
>
> CDW3 = Local0
> }
> Else
> {
> CDW1 |= 0x04
> }
>
> Return (Arg3)
> }
> }
> +
> + Device (DRAC)
> + {
> + Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID
> + Name (RBUF, ResourceTemplate ()
> + {
> + QWordMemory (ResourceProducer, PosDecode, MinFixed,
> MaxFixed, NonCacheable, ReadWrite,
> + 0x0000000000000000, // Granularity
> + 0x00000000B0000000, // Range Minimum
> + 0x00000000BFFFFFFF, // Range Maximum
> + 0x0000000000000000, // Translation Offset
> + 0x0000000010000000, // Length
> + ,, , AddressRangeMemory, TypeStatic)
> + })
> + Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
> + {
> + Return (RBUF) /* \_SB_.DRAC.RBUF */
> + }
> + }
> }
>
> Scope (_SB)
> {
> Device (HPET)
> {
> Name (_HID, EisaId ("PNP0103") /* HPET System Timer */) //
> _HID: Hardware ID
> Name (_UID, Zero) // _UID: Unique ID
> OperationRegion (HPTM, SystemMemory, 0xFED00000, 0x0400)
> Field (HPTM, DWordAcc, Lock, Preserve)
> {
> VEND, 32,
> PRD, 32
> }
>
> Method (_STA, 0, NotSerialized) // _STA: Status
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
> hw/i386/acpi-build.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 00cc119362..e369908b1a 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1072,6 +1072,46 @@ static void build_q35_pci0_int(Aml *table)
> aml_append(table, sb_scope);
> }
>
> +static Aml *build_q35_dram_controller(void)
> +{
> + AcpiMcfgInfo mcfg;
build_dsdt() already calls acpi_get_mcfg(),
I suggest to cache it there and pass to build_q35_dram_controller
as an argument.
> + Aml *dev;
> + Aml *rbuf;
> + Aml *resource_template;
> + Aml *rbuf_name;
> + Aml *crs;
> +
> + if (!acpi_get_mcfg(&mcfg)) {
> + return NULL;
> + }
> +
> + /* DRAM controller */
> + dev = aml_device("DRAC");
> + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> +
> + resource_template = aml_resource_template();
> + aml_append(resource_template,
> + aml_qword_memory(AML_POS_DECODE,
> + AML_MIN_FIXED,
> + AML_MAX_FIXED,
> + AML_NON_CACHEABLE,
> + AML_READ_WRITE,
> + 0x0000000000000000,
> + mcfg.base,
> + mcfg.base + mcfg.size - 1,
s/mcfg.base + mcfg.size - 1/mcfg.base/
> + 0x0000000000000000,
> + mcfg.size));
> + rbuf = aml_name_decl("RBUF", resource_template);
> + aml_append(dev, rbuf);
> +
> + crs = aml_method("_CRS", 0, AML_SERIALIZED);
> + rbuf_name = aml_name("RBUF");
> + aml_append(crs, aml_return(rbuf_name));
> + aml_append(dev, crs);
> +
> + return dev;
> +}
> +
> static void build_q35_isa_bridge(Aml *table)
> {
> Aml *dev;
> @@ -1212,7 +1252,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> Range *pci_hole, Range *pci_hole64, MachineState *machine)
> {
> CrsRangeEntry *entry;
> - Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
> + Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs, *drac;
maybe limit drac to
if (misc->is_piix4) {
... } else {
Aml *drac
scope
> CrsRangeSet crs_range_set;
> PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> @@ -1256,6 +1296,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> aml_append(dev, build_q35_osc_method());
> aml_append(sb_scope, dev);
> + drac = build_q35_dram_controller();
> + if (drac) {
> + aml_append(sb_scope, drac);
> + }
mmcfg isn't optional for q35, is it?
I'd simplify to:
aml_append(sb_scope, build_q35_dram_controller(mmcfg_info));
or put all of it under condition if it's optional
if(foo) aml_append(sb_scope, build_q35_dram_controller(mmcfg_info))
> if (pm->smi_on_cpuhp) {
> /* reserve SMI block resources, IO ports 0xB2, 0xB3 */
- [PATCH v3 03/10] i386: add properoty, x-smm-compat-5, to keep compatibility of SMM, (continued)
- [PATCH v3 03/10] i386: add properoty, x-smm-compat-5, to keep compatibility of SMM, Isaku Yamahata, 2021/02/11
- [PATCH v3 04/10] acpi/core: always set SCI_EN when SMM isn't supported, Isaku Yamahata, 2021/02/11
- [PATCH v3 05/10] acpi: set fadt.smi_cmd to zero when SMM is not supported, Isaku Yamahata, 2021/02/11
- [PATCH v3 06/10] acpi: add test case for smm unsupported -machine smm=off, Isaku Yamahata, 2021/02/11
- [PATCH v3 09/10] acpi: add test case for -no-hpet, Isaku Yamahata, 2021/02/11
- [PATCH v3 08/10] i386: acpi: Don't build HPET ACPI entry if HPET is disabled, Isaku Yamahata, 2021/02/11
- [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Isaku Yamahata, 2021/02/11
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region,
Igor Mammedov <=
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Isaku Yamahata, 2021/02/12
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Igor Mammedov, 2021/02/15
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Isaku Yamahata, 2021/02/16
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Michael S. Tsirkin, 2021/02/16
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Isaku Yamahata, 2021/02/16
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Igor Mammedov, 2021/02/16
[PATCH v3 10/10] qtest/acpi/bios-tables-test: update acpi tables, Isaku Yamahata, 2021/02/11