Hi Vijay, Please find my response inline marked [SAMI] Regards, Sami Mujawar ________________________________ From: Vijayenthiran Subramanian Sent: Wednesday, 17 February 2021, 9:15 am To: devel@edk2.groups.io; Sami Mujawar Cc: Vijayenthiran Subramaniam; Jonathan.Cameron@Huawei.com; leif@nuviainc.com; ardb+tianocore@kernel.org; Thomas Abraham Subject: Re: [edk2-devel] [edk2-platforms] [PATCH v2 1/3] Platform/ARM/SgiPkg: Add helper macros for HMAT table Hi Sami, On Fri, Feb 12, 2021 at 5:28 PM Sami Mujawar wrote: > > Hi Vijay, > > Please find my response inline marked [SAMI]. > > Regards, > > Sami Mujawar > > -----Original Message----- > From: Vijayenthiran Subramaniam > Sent: 03 February 2021 09:07 AM > To: devel@edk2.groups.io; Jonathan.Cameron@Huawei.com; leif@nuviainc.com; ardb+tianocore@kernel.org; Sami Mujawar > Cc: Thomas Abraham > Subject: [edk2-platforms] [PATCH v2 1/3] Platform/ARM/SgiPkg: Add helper macros for HMAT table > > Add helper macros to create Memory Proximity Domain, System Locality > Latency and Bandwidth Information, Memory Side Cache Information > structures in the HMAT table. > > Signed-off-by: Vijayenthiran Subramaniam > --- > Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h | 110 ++++++++++++++++++++ > 1 file changed, 110 insertions(+) > > diff --git a/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h b/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h > index 8d715de173c9..181d9d771d1e 100644 > --- a/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h > +++ b/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h > @@ -119,4 +119,114 @@ > ACPIProcessorUID, Flags, ClockDomain \ > } > > +// > +// HMAT related structures > +// > +// Memory Proximity Domain Attributes Structure > +// Refer Section 5.2.27.3 in ACPI Specification, Version 6.3 > +#define EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES_INIT( \ > + Flags, ProximityDomainForAttachedIntiator, ProximityDomainForMemory) \ > + { \ > + 0, \ > [SAMI] EFI_ACPI_6_3_HMAT_TYPE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES ? > [/SAMI] EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES is already a typedef and EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES_INIT follows the same naming as used in the Acpi63.h file: https://github.com/tianocore/edk2/blob/4f4d862c1c7232a18347616d94c343c929657fdb/MdePkg/Include/IndustryStandard/Acpi63.h#L2057 [SAMI] Please use the defined values from https://github.com/tianocore/edk2/blob/4f4d862c1c7232a18347616d94c343c929657fdb/MdePkg/Include/IndustryStandard/Acpi63.h#L2024 instead of magic numbers. Same for other places in this patch. [SAMI] > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + }, \ > + sizeof (EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES), \ > + { \ > + Flags, \ > + 0 \ > + }, \ > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + }, \ > + ProximityDomainForAttachedIntiator, \ > + ProximityDomainForMemory, \ > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + } \ > + } > + > +// System Locality Latency and Bandwidth Information Structure > +// Refer Section 5.2.27.4 in ACPI Specification, Version 6.3 > +#define EFI_ACPI_6_3_HMAT_STRUCTURE_SYSTEM_LOCALITY_LATENCY_AND_BANDWIDTH_INFO_INIT( \ > + Flags, DataType, NumInitiatorProximityDomains, \ > + NumTargetProximityDomains, EntryBaseUnit) \ > + { \ > + 1, \ > [SAMI] EFI_ACPI_6_3_HMAT_TYPE_SYSTEM_LOCALITY_LATENCY_AND_BANDWIDTH_INFO ? > [/SAMI] Same as above. > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + }, \ > + sizeof (EFI_ACPI_6_3_HMAT_STRUCTURE_SYSTEM_LOCALITY_LATENCY_AND_BANDWIDTH_INFO) + \ > + (4 * NumInitiatorProximityDomains) + (4 * NumTargetProximityDomains) + \ > + (2 * NumInitiatorProximityDomains * NumTargetProximityDomains), \ > + { \ > + Flags, 0 \ > [SAMI] 0 on new line for consistency ? > [/SAMI] Ack, will fix in the next version. > + }, \ > + DataType, \ > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + }, \ > + NumInitiatorProximityDomains, \ > + NumTargetProximityDomains, \ > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + }, \ > + EntryBaseUnit \ > + } > + > +// Memory Side Cache Information Structure > +// Refer Section 5.2.27.5 in ACPI Specification, Version 6.3 > +#define EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_INIT( \ > + MemoryProximityDomain, MemorySideCacheSize, CacheAttributes, \ > + NumberOfSmbiosHandles) \ > + { \ > + 2, \ > [SAMI] EFI_ACPI_6_3_HMAT_TYPE_MEMORY_SIDE_CACHE_INFO ? > [/SAMI] Same as above. > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + }, \ > + sizeof (EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO) + \ > + (NumberOfSmbiosHandles * 2), \ > + MemoryProximityDomain, \ > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + }, \ > + MemorySideCacheSize, \ > + CacheAttributes, \ > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE \ > + }, \ > + NumberOfSmbiosHandles \ > + } > + > #endif /* __SGI_ACPI_HEADER__ */ > -- > 2.17.1 > > BR, Vijayenthiran