Re: [edk2-devel] [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
Yes we have systems where we will have more than 16 controllers/segments.
> -Original Message-
> From: Pierre Gondois
> Sent: Monday, July 11, 2022 11:15 AM
> To: Jeff Brasen ; 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
>
>
> 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
> >> Sent: Monday, July 11, 2022 9:02 AM
> >> To: Jeff Brasen ; 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, 0x)
> >> }
> >> 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
> Sent: Monday, July 4, 2022 2:48 AM
> To: Jeff Brasen ; 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,
> INUINT32Uid,
> IN OUT AML_OBJECT_NODE_HANDLEPciNode
> )
>
> 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.
> >
> >
>
Re: [edk2-devel] [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
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
Sent: Monday, July 11, 2022 9:02 AM
To: Jeff Brasen ; 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, 0x)
}
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
Sent: Monday, July 4, 2022 2:48 AM
To: Jeff Brasen ; 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,
INUINT32Uid,
IN OUT AML_OBJECT_NODE_HANDLEPciNode
)
EFI_STATUS
EFIAPI
AddOscMethod (
IN OUT AML_OBJECT_NODE_HANDLE PciNode
)
Regards,
Pierre
On 7/1/22 17:52, Jeff Brasen wrote:
Currently we do the following in this call.
Remove and replace the _OSC method on certain targets. I originally
had this pass the template over but removed that when I added this
more generic override support Add a _RST method to the root port
device sub node Add a _DSD for device properties Conditionally add
an entry for the device attached to the PCIe bus if we need to add
properties or _RST methods. This will likely eventually move to
another driver (which is the purpose of patch 2/4 in this series)
I figured trying to get the generator to handle that would be more
difficult
as these would be hard to generalize.
Thanks,
Jeff
-Original Message-
From: Pierre Gondois
Sent: Friday, July 1, 2022 6:40 AM
To: Jeff Brasen ; devel@edk2.groups.io
Cc: sami.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
---
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
Re: [edk2-devel] [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
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
> -Original Message-
> From: Pierre Gondois
> Sent: Monday, July 11, 2022 9:02 AM
> To: Jeff Brasen ; 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, 0x)
> }
> 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
> >> Sent: Monday, July 4, 2022 2:48 AM
> >> To: Jeff Brasen ; 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,
> >> INUINT32Uid,
> >> IN OUT AML_OBJECT_NODE_HANDLEPciNode
> >> )
> >>
> >> EFI_STATUS
> >> EFIAPI
> >> AddOscMethod (
> >> IN OUT AML_OBJECT_NODE_HANDLE PciNode
> >> )
> >>
> >> Regards,
> >> Pierre
> >>
> >> On 7/1/22 17:52, Jeff Brasen wrote:
> >>> Currently we do the following in this call.
> >>>
> >>> Remove and replace the _OSC method on certain targets. I originally
> >>> had this pass the template over but removed that when I added this
> >>> more generic override support Add a _RST method to the root port
> >>> device sub node Add a _DSD for device properties Conditionally add
> >>> an entry for the device attached to the PCIe bus if we need to add
> >>> properties or _RST methods. This will likely eventually move to
> >>> another driver (which is the purpose of patch 2/4 in this series)
> >>>
> >>> I figured trying to get the generator to handle that would be more
> >>> difficult
> >> as these would be hard to generalize.
> >>>
> >>>
> >>> Thanks,
> >>> Jeff
> >>>
> -Original Message-
> From: Pierre Gondois
> Sent: Friday, July 1, 2022 6:40 AM
> To: Jeff Brasen ; devel@edk2.groups.io
> Cc: sami.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
> > ---
> > DynamicTablesPkg/DynamicTablesPkg.dec | 3 +
> > .../Protocol/SsdtPcieOverrideProtocol.h | 63
> >> +++
> >
Re: [edk2-devel] [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
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, 0x)
}
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
Sent: Monday, July 4, 2022 2:48 AM
To: Jeff Brasen ; 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,
INUINT32Uid,
IN OUT AML_OBJECT_NODE_HANDLEPciNode
)
EFI_STATUS
EFIAPI
AddOscMethod (
IN OUT AML_OBJECT_NODE_HANDLE PciNode
)
Regards,
Pierre
On 7/1/22 17:52, Jeff Brasen wrote:
Currently we do the following in this call.
Remove and replace the _OSC method on certain targets. I originally
had this pass the template over but removed that when I added this
more generic override support Add a _RST method to the root port
device sub node Add a _DSD for device properties Conditionally add an
entry for the device attached to the PCIe bus if we need to add
properties or _RST methods. This will likely eventually move to
another driver (which is the purpose of patch 2/4 in this series)
I figured trying to get the generator to handle that would be more difficult
as these would be hard to generalize.
Thanks,
Jeff
-Original Message-
From: Pierre Gondois
Sent: Friday, July 1, 2022 6:40 AM
To: Jeff Brasen ; devel@edk2.groups.io
Cc: sami.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
---
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 00..29568a0159
--- /dev/null
+++ b/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h
@@ -0,0 +1,63 @@
Re: [edk2-devel] [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
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
> Sent: Monday, July 4, 2022 2:48 AM
> To: Jeff Brasen ; 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,
>INUINT32Uid,
>IN OUT AML_OBJECT_NODE_HANDLEPciNode
>)
>
> EFI_STATUS
> EFIAPI
> AddOscMethod (
>IN OUT AML_OBJECT_NODE_HANDLE PciNode
>)
>
> Regards,
> Pierre
>
> On 7/1/22 17:52, Jeff Brasen wrote:
> > Currently we do the following in this call.
> >
> > Remove and replace the _OSC method on certain targets. I originally
> > had this pass the template over but removed that when I added this
> > more generic override support Add a _RST method to the root port
> > device sub node Add a _DSD for device properties Conditionally add an
> > entry for the device attached to the PCIe bus if we need to add
> > properties or _RST methods. This will likely eventually move to
> > another driver (which is the purpose of patch 2/4 in this series)
> >
> > I figured trying to get the generator to handle that would be more difficult
> as these would be hard to generalize.
> >
> >
> > Thanks,
> > Jeff
> >
> >> -Original Message-
> >> From: Pierre Gondois
> >> Sent: Friday, July 1, 2022 6:40 AM
> >> To: Jeff Brasen ; devel@edk2.groups.io
> >> Cc: sami.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
> >>> ---
> >>>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 00..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
> >>> +
> >>>
Re: [edk2-devel] [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
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,
INUINT32Uid,
IN OUT AML_OBJECT_NODE_HANDLEPciNode
)
EFI_STATUS
EFIAPI
AddOscMethod (
IN OUT AML_OBJECT_NODE_HANDLE PciNode
)
Regards,
Pierre
On 7/1/22 17:52, Jeff Brasen wrote:
Currently we do the following in this call.
Remove and replace the _OSC method on certain targets. I originally had this
pass the template over but removed that when I added this more generic override
support
Add a _RST method to the root port device sub node
Add a _DSD for device properties
Conditionally add an entry for the device attached to the PCIe bus if we need
to add properties or _RST methods. This will likely eventually move to another
driver (which is the purpose of patch 2/4 in this series)
I figured trying to get the generator to handle that would be more difficult as
these would be hard to generalize.
Thanks,
Jeff
-Original Message-
From: Pierre Gondois
Sent: Friday, July 1, 2022 6:40 AM
To: Jeff Brasen ; devel@edk2.groups.io
Cc: sami.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
---
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 00..29568a0159
--- /dev/null
+++ b/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h
@@ -0,0 +1,63 @@
+/** @file
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SSDT_PCIE_OVERRIDE_PROTOCOL_H_ #define
+SSDT_PCIE_OVERRIDE_PROTOCOL_H_
+
+#include
+#include
+
+/** This macro defines the SSDT PCI Override Protocol GUID.
+
+ GUID: {D85A4835-5A82-4894-AC02-706F43D5978E}
+*/
+#define EDKII_SSDT_PCI_OVERRIDE_PROTOCOL_GUID \
+ { 0x962e8b44, 0x23b3, 0x41da, \
+{ 0x9f, 0x36, 0xca, 0xde, 0x68, 0x49, 0xfb, 0xf6 } \
+ };
+
+/**
+ Forward declarations:
+*/
+typedef struct SsdtOverridePciProtocol
+EDKII_SSDT_PCI_OVERRIDE_PROTOCOL;
+
+/** The UpdateTable function allows the override protocol to update the
+ * PCIe SSDT table prior to being created.
+
+ @param [in] ThisPointer 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
Re: [edk2-devel] [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
Currently we do the following in this call.
Remove and replace the _OSC method on certain targets. I originally had this
pass the template over but removed that when I added this more generic override
support
Add a _RST method to the root port device sub node
Add a _DSD for device properties
Conditionally add an entry for the device attached to the PCIe bus if we need
to add properties or _RST methods. This will likely eventually move to another
driver (which is the purpose of patch 2/4 in this series)
I figured trying to get the generator to handle that would be more difficult as
these would be hard to generalize.
Thanks,
Jeff
> -Original Message-
> From: Pierre Gondois
> Sent: Friday, July 1, 2022 6:40 AM
> To: Jeff Brasen ; devel@edk2.groups.io
> Cc: sami.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
> > ---
> > 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 00..29568a0159
> > --- /dev/null
> > +++ b/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h
> > @@ -0,0 +1,63 @@
> > +/** @file
> > +
> > + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef SSDT_PCIE_OVERRIDE_PROTOCOL_H_ #define
> > +SSDT_PCIE_OVERRIDE_PROTOCOL_H_
> > +
> > +#include
> > +#include
> > +
> > +/** This macro defines the SSDT PCI Override Protocol GUID.
> > +
> > + GUID: {D85A4835-5A82-4894-AC02-706F43D5978E}
> > +*/
> > +#define EDKII_SSDT_PCI_OVERRIDE_PROTOCOL_GUID \
> > + { 0x962e8b44, 0x23b3, 0x41da, \
> > +{ 0x9f, 0x36, 0xca, 0xde, 0x68, 0x49, 0xfb, 0xf6 } \
> > + };
> > +
> > +/**
> > + Forward declarations:
> > +*/
> > +typedef struct SsdtOverridePciProtocol
> > +EDKII_SSDT_PCI_OVERRIDE_PROTOCOL;
> > +
> > +/** The UpdateTable function allows the override protocol to update the
> > + * PCIe SSDT table prior to being created.
> > +
> > + @param [in] ThisPointer 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,
> > + INUINT32Uid,
> > + 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_TABLEUpdateTable;
> > +}
Re: [edk2-devel] [PATCH 4/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Add support for override protocol
Hello Jeff,
Is it possible to know what the UpdateTable() function would do ?
Maybe it would be possible to integrate the alternative
implementation without adding a new protocol.
Regards,
Pierre
On 6/30/22 17:48, Jeff Brasen wrote:
Some platfoms may want to modify the ACPI table created.
Add support for protocol that can provide an alternative implementation.
Signed-off-by: Jeff Brasen
---
DynamicTablesPkg/DynamicTablesPkg.dec | 3 +
.../Protocol/SsdtPcieOverrideProtocol.h | 63 +++
.../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c| 31 -
.../Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf | 4 ++
4 files changed, 98 insertions(+), 3 deletions(-)
create mode 100644
DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec
b/DynamicTablesPkg/DynamicTablesPkg.dec
index a890a048be..bb66bdaf14 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dec
+++ b/DynamicTablesPkg/DynamicTablesPkg.dec
@@ -43,6 +43,9 @@
# Dynamic Table Factory Protocol GUID
gEdkiiDynamicTableFactoryProtocolGuid = { 0x91d1e327, 0xfe5a, 0x49b8, {
0xab, 0x65, 0xe, 0xce, 0x2d, 0xdb, 0x45, 0xec } }
+ # Protocol to override PCI SSDT table generation
+ gEdkiiDynamicTableSsdtPcieOverrideProtocolGuid = { 0x962e8b44, 0x23b3,
0x41da, { 0x9f, 0x36, 0xca, 0xde, 0x68, 0x49, 0xfb, 0xf6 } }
+
[PcdsFixedAtBuild]
# Maximum number of Custom ACPI Generators
diff --git a/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h
b/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h
new file mode 100644
index 00..29568a0159
--- /dev/null
+++ b/DynamicTablesPkg/Include/Protocol/SsdtPcieOverrideProtocol.h
@@ -0,0 +1,63 @@
+/** @file
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SSDT_PCIE_OVERRIDE_PROTOCOL_H_
+#define SSDT_PCIE_OVERRIDE_PROTOCOL_H_
+
+#include
+#include
+
+/** This macro defines the SSDT PCI Override Protocol GUID.
+
+ GUID: {D85A4835-5A82-4894-AC02-706F43D5978E}
+*/
+#define EDKII_SSDT_PCI_OVERRIDE_PROTOCOL_GUID \
+ { 0x962e8b44, 0x23b3, 0x41da, \
+{ 0x9f, 0x36, 0xca, 0xde, 0x68, 0x49, 0xfb, 0xf6 } \
+ };
+
+/**
+ Forward declarations:
+*/
+typedef struct SsdtOverridePciProtocol EDKII_SSDT_PCI_OVERRIDE_PROTOCOL;
+
+/** The UpdateTable function allows the override protocol to update the
+ * PCIe SSDT table prior to being created.
+
+ @param [in] ThisPointer 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,
+ INUINT32Uid,
+ 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_TABLEUpdateTable;
+} 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
#include
#include
+#include
#include
// Module specific include files.
@@ -30,6 +31,7 @@
#include
#include
#include
+#include
#include "SsdtPcieGenerator.h"
@@ -798,9 +800,10 @@ GeneratePciDevice (
{
EFI_STATUS Status;
- CHAR8 AslName[AML_NAME_SEG_SIZE + 1];
- AML_OBJECT_NODE_HANDLE ScopeNode;
- AML_OBJECT_NODE_HANDLE PciNode;
+ CHAR8 AslName[AML_NAME_SEG_SIZE + 1];
+ AML_OBJECT_NODE_HANDLEScopeNode;
+ AML_OBJECT_NODE_HANDLEPciNode;
+ EDKII_SSDT_PCI_OVERRIDE_PROTOCOL *OverrideProtocol;
ASSERT (Generator != NULL);
ASSERT (CfgMgrProtocol != NULL);
@@ -860,6 +863,28 @@ GeneratePciDevice (
// Add the template _OSC method.
Status = AddOscMethod
