I think this would work. Instead of GeneratePciDeviceInfo the function we would want to break out would be GeneratePciSlots. I'll work on changing my patch series to this. Unless you have a better name will call this library SsdtPciSupportLib and place in under Library/Common
Going to also pass PciInfo into AddOscMethod in this new approach in case client needs to have different _OSC per controller (And to GeneratePciSlots as well). Thanks, Jeff > -----Original Message----- > From: Pierre Gondois <pierre.gond...@arm.com> > Sent: Monday, July 4, 2022 2:48 AM > To: Jeff Brasen <jbra...@nvidia.com>; devel@edk2.groups.io > Cc: sami.muja...@arm.com; alexei.fedo...@arm.com > Subject: Re: [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add > support for override protocol > > External email: Use caution opening links or attachments > > > 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 <pierre.gond...@arm.com> > >> Sent: Friday, July 1, 2022 6:40 AM > >> To: Jeff Brasen <jbra...@nvidia.com>; devel@edk2.groups.io > >> Cc: sami.muja...@arm.com; alexei.fedo...@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 <jbra...@nvidia.com> > >>> --- > >>> 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 <ArmNameSpaceObjects.h> > >>> +#include <Library/AmlLib/AmlLib.h> > >>> + > >>> +/** 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 <Library/BaseMemoryLib.h> > >>> #include <Library/DebugLib.h> > >>> #include <Library/MemoryAllocationLib.h> > >>> +#include <Library/UefiBootServicesTableLib.h> > >>> #include <Protocol/AcpiTable.h> > >>> > >>> // Module specific include files. > >>> @@ -30,6 +31,7 @@ > >>> #include <Library/TableHelperLib.h> > >>> #include <Library/AmlLib/AmlLib.h> > >>> #include <Protocol/ConfigurationManagerProtocol.h> > >>> +#include <Protocol/SsdtPcieOverrideProtocol.h> > >>> > >>> #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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91185): https://edk2.groups.io/g/devel/message/91185 Mute This Topic: https://groups.io/mt/92089321/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-