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 <jbrasen@...> --- 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/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c index c5b23d91d0..d5982c24ff 100644 --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.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/SsdtPcieLibArm.inf @@ -30,6 +30,10 @@ AcpiHelperLib AmlLib BaseLib + UefiBootServicesTableLib [Pcd] gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdPciUseSegmentAsUid + +[Protocols] + gEdkiiDynamicTableSsdtPcieOverrideProtocolGuid -- 2.25.1
|
|
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
toggle quoted messageShow quoted text
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 <jbrasen@...> --- 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/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c index c5b23d91d0..d5982c24ff 100644 --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.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/SsdtPcieLibArm.inf @@ -30,6 +30,10 @@ AcpiHelperLib AmlLib BaseLib + UefiBootServicesTableLib [Pcd] gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdPciUseSegmentAsUid + +[Protocols] + gEdkiiDynamicTableSsdtPcieOverrideProtocolGuid
|
|
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
toggle quoted messageShow quoted text
-----Original Message----- From: Pierre Gondois <pierre.gondois@...> Sent: Friday, July 1, 2022 6:40 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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 <jbrasen@...> --- 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
|
|
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
toggle quoted messageShow quoted text
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.gondois@...> Sent: Friday, July 1, 2022 6:40 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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 <jbrasen@...> --- 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
|
|
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
toggle quoted messageShow quoted text
-----Original Message----- From: Pierre Gondois <pierre.gondois@...> Sent: Monday, July 4, 2022 2:48 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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.gondois@...> Sent: Friday, July 1, 2022 6:40 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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 <jbrasen@...> --- 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
|
|
Hello Jeff, To be sure I understand correctly, the SsdtPcieGenerator currently allows to generate something like this:
Scope(_SB) { Device(PCI0) { Device (D00_) { // Device 0 Name(_ADR, 0x0000FFFF) } Method(_OSC,4) { [...] // Generic _OSC method } } }
What you want to do is: 1. Use another _OSC method 2. Add more information in the D00_ object, i.e.: - a _DSD object - a _RST method 3. As _RST is added in 2., also add a _RST method to PCI0
Is it correct ? Or is there some other information you want to add ?
Regards, Pierre
toggle quoted messageShow quoted text
On 7/8/22 17:36, Jeff Brasen wrote: 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.gondois@...> Sent: Monday, July 4, 2022 2:48 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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.gondois@...> Sent: Friday, July 1, 2022 6:40 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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 <jbrasen@...> --- 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
|
|
We also on some PCIe devices add a device entry under DOO_ w/ it's own _RST and _DSD. If you see the v3 version of the patch where I add a support library that we can override allows us to customize the device as we need.
-Jeff
toggle quoted messageShow quoted text
-----Original Message----- From: Pierre Gondois <pierre.gondois@...> Sent: Monday, July 11, 2022 9:02 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... Subject: Re: [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
External email: Use caution opening links or attachments
Hello Jeff, To be sure I understand correctly, the SsdtPcieGenerator currently allows to generate something like this:
Scope(_SB) { Device(PCI0) { Device (D00_) { // Device 0 Name(_ADR, 0x0000FFFF) } Method(_OSC,4) { [...] // Generic _OSC method } } }
What you want to do is: 1. Use another _OSC method 2. Add more information in the D00_ object, i.e.: - a _DSD object - a _RST method 3. As _RST is added in 2., also add a _RST method to PCI0
Is it correct ? Or is there some other information you want to add ?
Regards, Pierre
On 7/8/22 17:36, Jeff Brasen wrote:
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.gondois@...> Sent: Monday, July 4, 2022 2:48 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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.gondois@...> Sent: Friday, July 1, 2022 6:40 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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 <jbrasen@...> --- 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
|
|
On 7/11/22 17:32, Jeff Brasen wrote: We also on some PCIe devices add a device entry under DOO_ w/ it's own _RST and _DSD. If you see the v3 version of the patch where I add a support library that we can override allows us to customize the device as we need. Ok thanks! GeneratePciSlots() should allow you to add any platform specific device, and should ideally not modify the parent 'PCI0' object (even if it is possible in practice). So it should be ok. A one last question: [PATCH v3 2/3] DynamicTablesPkg: AcpiSsdtPcieLibArm: Support UID > 0xF Is is still needed ? Regards, Pierre -Jeff
-----Original Message----- From: Pierre Gondois <pierre.gondois@...> Sent: Monday, July 11, 2022 9:02 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... Subject: Re: [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
External email: Use caution opening links or attachments
Hello Jeff, To be sure I understand correctly, the SsdtPcieGenerator currently allows to generate something like this:
Scope(_SB) { Device(PCI0) { Device (D00_) { // Device 0 Name(_ADR, 0x0000FFFF) } Method(_OSC,4) { [...] // Generic _OSC method } } }
What you want to do is: 1. Use another _OSC method 2. Add more information in the D00_ object, i.e.: - a _DSD object - a _RST method 3. As _RST is added in 2., also add a _RST method to PCI0
Is it correct ? Or is there some other information you want to add ?
Regards, Pierre
On 7/8/22 17:36, Jeff Brasen wrote:
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.gondois@...> Sent: Monday, July 4, 2022 2:48 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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.gondois@...> Sent: Friday, July 1, 2022 6:40 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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 <jbrasen@...> --- 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
|
|
Yes we have systems where we will have more than 16 controllers/segments.
toggle quoted messageShow quoted text
-----Original Message----- From: Pierre Gondois <pierre.gondois@...> Sent: Monday, July 11, 2022 11:15 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... Subject: Re: [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
External email: Use caution opening links or attachments
On 7/11/22 17:32, Jeff Brasen wrote:
We also on some PCIe devices add a device entry under DOO_ w/ it's own _RST and _DSD. If you see the v3 version of the patch where I add a support library that we can override allows us to customize the device as we need.
Ok thanks! GeneratePciSlots() should allow you to add any platform specific device, and should ideally not modify the parent 'PCI0' object (even if it is possible in practice). So it should be ok.
A one last question:
[PATCH v3 2/3] DynamicTablesPkg: AcpiSsdtPcieLibArm: Support UID > 0xF Is is still needed ?
Regards, Pierre
-Jeff
-----Original Message----- From: Pierre Gondois <pierre.gondois@...> Sent: Monday, July 11, 2022 9:02 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... Subject: Re: [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
External email: Use caution opening links or attachments
Hello Jeff, To be sure I understand correctly, the SsdtPcieGenerator currently allows to generate something like this:
Scope(_SB) { Device(PCI0) { Device (D00_) { // Device 0 Name(_ADR, 0x0000FFFF) } Method(_OSC,4) { [...] // Generic _OSC method } } }
What you want to do is: 1. Use another _OSC method 2. Add more information in the D00_ object, i.e.: - a _DSD object - a _RST method 3. As _RST is added in 2., also add a _RST method to PCI0
Is it correct ? Or is there some other information you want to add ?
Regards, Pierre
On 7/8/22 17:36, Jeff Brasen wrote:
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.gondois@...> Sent: Monday, July 4, 2022 2:48 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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.gondois@...> Sent: Friday, July 1, 2022 6:40 AM To: Jeff Brasen <jbrasen@...>; devel@edk2.groups.io Cc: Sami.Mujawar@...; Alexei.Fedorov@... 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 <jbrasen@...> --- 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
|
|