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.gond...@arm.com>
Sent: Monday, July 11, 2022 9:02 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,
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.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 (#91221): https://edk2.groups.io/g/devel/message/91221
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]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to