Can this be merged? -----Original Message----- From: Pierre Gondois <[email protected]> Sent: Tuesday, October 10, 2023 1:31 AM To: Jeshua Smith <[email protected]>; [email protected] Cc: [email protected]; [email protected]; [email protected] Subject: Re: [PATCH v2] DynamicTablesPkg/AmlLib: Enumerate memory attributes
External email: Use caution opening links or attachments Hi Jeshua, Thanks for the v2, Reviewed-by: Pierre Gondois <[email protected]> Sami: There was also a tag from Leif: https://edk2.groups.io/g/devel/message/109285 Regards, Pierre On 10/5/23 18:38, Jeshua Smith wrote: > AmlCodeGenRdQWordMemory's and AmlCodeGenRdDWordMemory's Cacheable and > MemoryRangeType parameters treat specific values as having specific > meanings as defined by the spec. This change adds enums to map those > meanings to their corresponding values. > > Signed-off-by: Jeshua Smith <[email protected]> > --- > > Notes: > v2: based on comments from Pierre Gondois > - Added documentation reference > - Changed enum type and member names to closer align with documentation > - Changed enum member names to CamelCase > - Added *Max members to enums > - Updated the signatures of relevant functions to use the enum types > instead of UNIT8 > > .../Include/Library/AmlLib/AmlLib.h | 49 +++++++++++++++++-- > .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 12 ++--- > .../AmlLib/CodeGen/AmlResourceDataCodeGen.c | 8 +-- > 3 files changed, 55 insertions(+), 14 deletions(-) > > diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > index 510c79a399..71e8539b30 100644 > --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > @@ -59,6 +59,47 @@ typedef void *AML_DATA_NODE_HANDLE; > > #endif // AML_HANDLE > > +/** Memory attributes, _MEM (2 bits) > + > + Possible values are: > + 0-The memory is non-cacheable > + 1-The memory is cacheable (DEPRECATED) > + 2-The memory is cacheable and supports > + write combining (DEPRECATED) > + 3-The memory is cacheable and prefetchable > + > + @par Reference(s): > + - ACPI 6.5, s6.4.3.5.5 "Resource Type Specific Flags" > + > +**/ > +typedef enum { > + AmlMemoryNonCacheable = 0, > + AmlMemoryCacheable = 1, > + AmlMemoryCacheableWriteCombine = 2, > + AmlMemoryCacheablePrefetch = 3, > + AmlMemoryCacheablityMax = 4 > +} AML_MEMORY_ATTRIBUTES_MEM; > + > +/** Memory attributes, _MTP (2 bits) > + > + Possible values are: > + 0-AddressRangeMemory > + 1-AddressRangeReserved > + 2-AddressRangeACPI > + 3-AddressRangeNVS > + > + @par Reference(s): > + - ACPI 6.5, s6.4.3.5.5 "Resource Type Specific Flags" > + > +**/ > +typedef enum { > + AmlAddressRangeMemory = 0, > + AmlAddressRangeReserved = 1, > + AmlAddressRangeACPI = 2, > + AmlAddressRangeNVS = 3, > + AmlAddressRangeMax = 4 > +} AML_MEMORY_ATTRIBUTES_MTP; > + > /** Parse the definition block. > > The function parses the whole AML blob. It starts with the ACPI > DSDT/SSDT @@ -578,7 +619,7 @@ AmlCodeGenRdDWordMemory ( > IN BOOLEAN IsPosDecode, > IN BOOLEAN IsMinFixed, > IN BOOLEAN IsMaxFixed, > - IN UINT8 Cacheable, > + IN AML_MEMORY_ATTRIBUTES_MEM Cacheable, > IN BOOLEAN IsReadWrite, > IN UINT32 AddressGranularity, > IN UINT32 AddressMinimum, > @@ -587,7 +628,7 @@ AmlCodeGenRdDWordMemory ( > IN UINT32 RangeLength, > IN UINT8 ResourceSourceIndex, > IN CONST CHAR8 *ResourceSource, > - IN UINT8 MemoryRangeType, > + IN AML_MEMORY_ATTRIBUTES_MTP MemoryRangeType, > IN BOOLEAN IsTypeStatic, > IN AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL > OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL > @@ -809,7 +850,7 @@ AmlCodeGenRdQWordMemory ( > IN BOOLEAN IsPosDecode, > IN BOOLEAN IsMinFixed, > IN BOOLEAN IsMaxFixed, > - IN UINT8 Cacheable, > + IN AML_MEMORY_ATTRIBUTES_MEM Cacheable, > IN BOOLEAN IsReadWrite, > IN UINT64 AddressGranularity, > IN UINT64 AddressMinimum, > @@ -818,7 +859,7 @@ AmlCodeGenRdQWordMemory ( > IN UINT64 RangeLength, > IN UINT8 ResourceSourceIndex, > IN CONST CHAR8 *ResourceSource, > - IN UINT8 MemoryRangeType, > + IN AML_MEMORY_ATTRIBUTES_MTP MemoryRangeType, > IN BOOLEAN IsTypeStatic, > IN AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL > OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL > diff --git > a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerat > or.c > b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerat > or.c > index 9ddaddc198..72873709aa 100644 > --- > a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerat > or.c > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGen > +++ erator.c > @@ -566,7 +566,7 @@ GeneratePciCrs ( > IsPosDecode, > TRUE, > TRUE, > - TRUE, > + AmlMemoryCacheable, > TRUE, > 0, > AddrMapInfo->PciAddress, @@ -575,7 +575,7 @@ > GeneratePciCrs ( > AddrMapInfo->AddressSize, > 0, > NULL, > - 0, > + AmlAddressRangeMemory, > TRUE, > CrsNode, > NULL > @@ -588,7 +588,7 @@ GeneratePciCrs ( > IsPosDecode, > TRUE, > TRUE, > - TRUE, > + AmlMemoryCacheable, > TRUE, > 0, > AddrMapInfo->PciAddress, @@ -597,7 +597,7 @@ > GeneratePciCrs ( > AddrMapInfo->AddressSize, > 0, > NULL, > - 0, > + AmlAddressRangeMemory, > TRUE, > CrsNode, > NULL > @@ -718,7 +718,7 @@ ReserveEcamSpace ( > TRUE, > TRUE, > TRUE, > - FALSE, // non-cacheable > + AmlMemoryNonCacheable, > TRUE, > 0, > AddressMinimum, > @@ -727,7 +727,7 @@ ReserveEcamSpace ( > AddressMaximum - AddressMinimum + 1, > 0, > NULL, > - 0, > + AmlAddressRangeMemory, > TRUE, > CrsNode, > NULL > diff --git > a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGe > n.c > b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGe > n.c > index 9c6700b9e0..0bc1c1d119 100644 > --- > a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGe > n.c > +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCo > +++ deGen.c > @@ -570,7 +570,7 @@ AmlCodeGenRdDWordMemory ( > IN BOOLEAN IsPosDecode, > IN BOOLEAN IsMinFixed, > IN BOOLEAN IsMaxFixed, > - IN UINT8 Cacheable, > + IN AML_MEMORY_ATTRIBUTES_MEM Cacheable, > IN BOOLEAN IsReadWrite, > IN UINT32 AddressGranularity, > IN UINT32 AddressMinimum, > @@ -579,7 +579,7 @@ AmlCodeGenRdDWordMemory ( > IN UINT32 RangeLength, > IN UINT8 ResourceSourceIndex, > IN CONST CHAR8 *ResourceSource, > - IN UINT8 MemoryRangeType, > + IN AML_MEMORY_ATTRIBUTES_MTP MemoryRangeType, > IN BOOLEAN IsTypeStatic, > IN AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL > OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL > @@ -1161,7 +1161,7 @@ AmlCodeGenRdQWordMemory ( > IN BOOLEAN IsPosDecode, > IN BOOLEAN IsMinFixed, > IN BOOLEAN IsMaxFixed, > - IN UINT8 Cacheable, > + IN AML_MEMORY_ATTRIBUTES_MEM Cacheable, > IN BOOLEAN IsReadWrite, > IN UINT64 AddressGranularity, > IN UINT64 AddressMinimum, > @@ -1170,7 +1170,7 @@ AmlCodeGenRdQWordMemory ( > IN UINT64 RangeLength, > IN UINT8 ResourceSourceIndex, > IN CONST CHAR8 *ResourceSource, > - IN UINT8 MemoryRangeType, > + IN AML_MEMORY_ATTRIBUTES_MTP MemoryRangeType, > IN BOOLEAN IsTypeStatic, > IN AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL > OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109929): https://edk2.groups.io/g/devel/message/109929 Mute This Topic: https://groups.io/mt/101780411/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
