From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.69643.1656924487305474420 for ; Mon, 04 Jul 2022 01:48:07 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 188AD2B; Mon, 4 Jul 2022 01:48:07 -0700 (PDT) Received: from [10.57.41.108] (unknown [10.57.41.108]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F157F3F792; Mon, 4 Jul 2022 01:48:04 -0700 (PDT) Message-ID: Date: Mon, 4 Jul 2022 10:47:42 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol To: Jeff Brasen , "devel@edk2.groups.io" Cc: "Sami.Mujawar@arm.com" , "Alexei.Fedorov@arm.com" References: <45abcaa79586b025205269d17957537d613c9bc2.1656603839.git.jbrasen@nvidia.com> From: "PierreGondois" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Jeff, I think it would ideally be better to have the code adding/replacing the methods/objects you noted inside the edk2/edk2-platfoms repositories. The methods/objects that you want to replace seem to only concern: -the objects available in the 'Device (PCIx)' node -the _OSC method If a library with the following two methods (extracted from SsdtPcieLibArm.inf) was created, would it be sufficient for all the replacements you want to do ? Like this we would have a generic implementation and specific ones. EFI_STATUS EFIAPI GeneratePciDeviceInfo ( IN CONST CM_ARM_PCI_CONFIG_SPACE_INFO *PciInfo, IN UINT32 Uid, IN OUT AML_OBJECT_NODE_HANDLE PciNode ) EFI_STATUS EFIAPI AddOscMethod ( IN OUT AML_OBJECT_NODE_HANDLE PciNode ) Regards, Pierre On 7/1/22 17:52, Jeff Brasen wrote: > Currently we do the following in this call. > > Remove and replace the _OSC method on certain targets. I originally had this pass the template over but removed that when I added this more generic override support > Add a _RST method to the root port device sub node > Add a _DSD for device properties > Conditionally add an entry for the device attached to the PCIe bus if we need to add properties or _RST methods. This will likely eventually move to another driver (which is the purpose of patch 2/4 in this series) > > I figured trying to get the generator to handle that would be more difficult as these would be hard to generalize. > > > Thanks, > Jeff > >> -----Original Message----- >> From: Pierre Gondois >> Sent: Friday, July 1, 2022 6:40 AM >> To: Jeff Brasen ; devel@edk2.groups.io >> Cc: Sami.Mujawar@arm.com; Alexei.Fedorov@arm.com >> Subject: Re: [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add >> support for override protocol >> >> External email: Use caution opening links or attachments >> >> >> Hello Jeff, >> Is it possible to know what the UpdateTable() function would do ? >> Maybe it would be possible to integrate the alternative implementation >> without adding a new protocol. >> >> Regards, >> Pierre >> >> On 6/30/22 17:48, Jeff Brasen wrote: >>> Some platfoms may want to modify the ACPI table created. >>> Add support for protocol that can provide an alternative implementation. >>> >>> Signed-off-by: Jeff Brasen >>> --- >>> DynamicTablesPkg/DynamicTablesPkg.dec | 3 + >>> .../Protocol/SsdtPcieOverrideProtocol.h | 63 +++++++++++++++++++ >>> .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 31 ++++++++- >>> .../Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf | 4 ++ >>> 4 files changed, 98 insertions(+), 3 deletions(-) >>> create mode 100644 >>> DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h >>> >>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec >>> b/DynamicTablesPkg/DynamicTablesPkg.dec >>> index a890a048be..bb66bdaf14 100644 >>> --- a/DynamicTablesPkg/DynamicTablesPkg.dec >>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec >>> @@ -43,6 +43,9 @@ >>> # Dynamic Table Factory Protocol GUID >>> gEdkiiDynamicTableFactoryProtocolGuid = { 0x91d1e327, 0xfe5a, >>> 0x49b8, { 0xab, 0x65, 0xe, 0xce, 0x2d, 0xdb, 0x45, 0xec } } >>> >>> + # Protocol to override PCI SSDT table generation >>> + gEdkiiDynamicTableSsdtPcieOverrideProtocolGuid = { 0x962e8b44, >>> + 0x23b3, 0x41da, { 0x9f, 0x36, 0xca, 0xde, 0x68, 0x49, 0xfb, 0xf6 } } >>> + >>> [PcdsFixedAtBuild] >>> >>> # Maximum number of Custom ACPI Generators diff --git >>> a/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h >>> b/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h >>> new file mode 100644 >>> index 0000000000..29568a0159 >>> --- /dev/null >>> +++ b/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h >>> @@ -0,0 +1,63 @@ >>> +/** @file >>> + >>> + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved. >>> + >>> + SPDX-License-Identifier: BSD-2-Clause-Patent >>> + >>> +**/ >>> + >>> +#ifndef SSDT_PCIE_OVERRIDE_PROTOCOL_H_ #define >>> +SSDT_PCIE_OVERRIDE_PROTOCOL_H_ >>> + >>> +#include >>> +#include >>> + >>> +/** This macro defines the SSDT PCI Override Protocol GUID. >>> + >>> + GUID: {D85A4835-5A82-4894-AC02-706F43D5978E} >>> +*/ >>> +#define EDKII_SSDT_PCI_OVERRIDE_PROTOCOL_GUID \ >>> + { 0x962e8b44, 0x23b3, 0x41da, \ >>> + { 0x9f, 0x36, 0xca, 0xde, 0x68, 0x49, 0xfb, 0xf6 } \ >>> + }; >>> + >>> +/** >>> + Forward declarations: >>> +*/ >>> +typedef struct SsdtOverridePciProtocol >>> +EDKII_SSDT_PCI_OVERRIDE_PROTOCOL; >>> + >>> +/** The UpdateTable function allows the override protocol to update the >>> + * PCIe SSDT table prior to being created. >>> + >>> + @param [in] This Pointer to the SSDT PCI Override Protocol. >>> + @param [in] PciInfo The PCIe configuration info for this node. >>> + @param [in] Uid UID that was selected for this PCIe node. >>> + @param [in, out] PciNode Pointer to the PCI node of this ACPI table. >>> + >>> + @retval EFI_SUCCESS Success. >>> + @retval EFI_INVALID_PARAMETER A parameter is invalid. >>> + @retval EFI_DEVICE_ERROR Failed to update the table. >>> +**/ >>> +typedef >>> +EFI_STATUS >>> +(EFIAPI *EDKII_SSDT_PCI_OVERRIDE_PROTOCOL_UPDATE_TABLE)( >>> + IN CONST EDKII_SSDT_PCI_OVERRIDE_PROTOCOL *CONST This, >>> + IN CONST CM_ARM_PCI_CONFIG_SPACE_INFO *PciInfo, >>> + IN UINT32 Uid, >>> + IN OUT AML_ROOT_NODE_HANDLE *PciNode >>> + ); >>> + >>> +/** The EDKII_CONFIGURATION_MANAGER_PROTOCOL structure >> describes the >>> + Configuration Manager Protocol interface. >>> +*/ >>> +typedef struct SsdtOverridePciProtocol { >>> + /** The interface used to update the ACPI table for PCI. >>> + */ >>> + EDKII_SSDT_PCI_OVERRIDE_PROTOCOL_UPDATE_TABLE UpdateTable; >>> +} EDKII_SSDT_PCI_OVERRIDE_PROTOCOL; >>> + >>> +/** The SSDT PCI Override Protocol GUID. >>> +*/ >>> +extern EFI_GUID gEdkiiDynamicTableSsdtPcieOverrideProtocolGuid; >>> + >>> +#endif // SSDT_PCIE_OVERRIDE_PROTOCOL_H_ >>> diff --git >>> >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera >> t >>> or.c >>> >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera >> t >>> or.c >>> index c5b23d91d0..d5982c24ff 100644 >>> --- >>> >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera >> t >>> or.c >>> +++ >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGen >>> +++ erator.c >>> @@ -20,6 +20,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> >>> // Module specific include files. >>> @@ -30,6 +31,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include "SsdtPcieGenerator.h" >>> >>> @@ -798,9 +800,10 @@ GeneratePciDevice ( >>> { >>> EFI_STATUS Status; >>> >>> - CHAR8 AslName[AML_NAME_SEG_SIZE + 1]; >>> - AML_OBJECT_NODE_HANDLE ScopeNode; >>> - AML_OBJECT_NODE_HANDLE PciNode; >>> + CHAR8 AslName[AML_NAME_SEG_SIZE + 1]; >>> + AML_OBJECT_NODE_HANDLE ScopeNode; >>> + AML_OBJECT_NODE_HANDLE PciNode; >>> + EDKII_SSDT_PCI_OVERRIDE_PROTOCOL *OverrideProtocol; >>> >>> ASSERT (Generator != NULL); >>> ASSERT (CfgMgrProtocol != NULL); >>> @@ -860,6 +863,28 @@ GeneratePciDevice ( >>> >>> // Add the template _OSC method. >>> Status = AddOscMethod (PciNode); >>> + if (EFI_ERROR (Status)) { >>> + ASSERT (0); >>> + return Status; >>> + } >>> + >>> + Status = gBS->LocateProtocol ( >>> + &gEdkiiDynamicTableSsdtPcieOverrideProtocolGuid, >>> + NULL, >>> + (VOID **)&OverrideProtocol >>> + ); >>> + if (!EFI_ERROR (Status)) { >>> + Status = OverrideProtocol->UpdateTable ( >>> + OverrideProtocol, >>> + PciInfo, >>> + Uid, >>> + PciNode >>> + ); >>> + } else { >>> + // Not an error if override protocol is not found >>> + Status = EFI_SUCCESS; >>> + } >>> + >>> ASSERT_EFI_ERROR (Status); >>> return Status; >>> } >>> diff --git >>> >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm. >>> inf >>> >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm. >>> inf >>> index 431e32a777..8e916f15e9 100644 >>> --- >>> >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm. >>> inf >>> +++ >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLib >>> +++ Arm.inf >>> @@ -30,6 +30,10 @@ >>> AcpiHelperLib >>> AmlLib >>> BaseLib >>> + UefiBootServicesTableLib >>> >>> [Pcd] >>> gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdPciUseSegmentAsUid >>> + >>> +[Protocols] >>> + gEdkiiDynamicTableSsdtPcieOverrideProtocolGuid