Hi Liming, This is a feature support. This patch is a rebased version of a reviewed patch that was pending to be merged due to a lack of 'tested-by', see: https://edk2.groups.io/g/devel/message/42297?p=,,,20,0,0,0::Created,,Krzysztof+Koch+SRAT,20,2,0,32042587 https://edk2.groups.io/g/devel/message/42665?p=,,,20,0,0,0::Created,,We+can+review+for+your+code+change.+And+for+our+personal+point%2C+the+change+is+OK.,20,2,0,32042696
Kind regards, Krzysztof -----Original Message----- From: Gao, Liming <liming....@intel.com> Sent: Tuesday, November 19, 2019 14:24 To: devel@edk2.groups.io; Sami Mujawar <sami.muja...@arm.com>; Krzysztof Koch <krzysztof.k...@arm.com> Cc: Ni, Ray <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; Matteo Carlini <matteo.carl...@arm.com>; nd <n...@arm.com> Subject: RE: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: Update SRAT parser to ACPI 6.3 Krzysztof: Does this patch plan to catch to edk2 201911 stable tag? This change is like a feature support. But, it doesn't pass review before soft feature freeze. Thanks Liming > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami > Mujawar > Sent: Monday, November 18, 2019 5:33 PM > To: Krzysztof Koch <krzysztof.k...@arm.com>; devel@edk2.groups.io > Cc: Ni, Ray <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; > Matteo Carlini <matteo.carl...@arm.com>; nd <n...@arm.com> > Subject: Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: Update > SRAT parser to ACPI 6.3 > > Reviewed-by: Sami Mujawar <sami.muja...@arm.com> > > Regards, > > Sami Mujawar > -----Original Message----- > From: Krzysztof Koch <krzysztof.k...@arm.com> > Sent: 12 November 2019 02:40 PM > To: devel@edk2.groups.io > Cc: ray...@intel.com; zhichao....@intel.com; Matteo Carlini > <matteo.carl...@arm.com>; Sami Mujawar <sami.muja...@arm.com>; nd > <n...@arm.com> > Subject: [PATCH v2 1/1] ShellPkg: acpiview: Update SRAT parser to ACPI > 6.3 > > Add support for parsing revision 3 of System Resource Affinity Table (SRAT). > > Decode and dump the new Generic Initiator Affinity Structure. > > Validate the Device Handle Type field inside the Generic Initiator Affinity > Structure. > > Signed-off-by: Krzysztof Koch <krzysztof.k...@arm.com> > --- > > The changes can be seen at > https://github.com/KrzysztofKoch1/edk2/tree/582_acpiview_6_3_srat_v2 > > Notes: > v2: > - rebase on latest master [Krzysztof] > > Readme.md | 4 > +- > ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 33 > +++ > ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 16 > + > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c > | 313 ++++++++++++++++++-- > 4 files changed, 343 insertions(+), 23 deletions(-) > > diff --git a/Readme.md b/Readme.md > index > acbcca88d3c62d392e6f94b1ad7de2d8627e3a38..594f16b20756ab78987fef4feac2 > e439743d5b4a 100644 > --- a/Readme.md > +++ b/Readme.md > @@ -17,7 +17,7 @@ for the UEFI and PI specifications from www.uefi.org. > <tr> > <td>Windows</td> > <td>VS2019</td> > - <td>edk2-ci</td> > + <td>master</td> > <td> > <a > href="https://dev.azure.com/tianocore/edk2-ci/_build/latest?definitionId=32&branchName=master";> > <img > src="https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/Window > s%20VS2019%20CI?branchName=master"/></a> > @@ -34,7 +34,7 @@ for the UEFI and PI specifications from www.uefi.org. > <tr> > <td>Ubuntu</td> > <td>GCC</td> > - <td>edk2-ci</td> > + <td>master</td> > <td> > <a > href="https://dev.azure.com/tianocore/edk2-ci/_build/latest?definitionId=31&branchName=master";> > <img > src="https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/Ubuntu > %20GCC5%20CI?branchName=master"/></a> > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c > index > a569c3c55406ab58536834e56ce9701f7edeffee..2b2ecb93cef9ee28b752e7bf2d92 > 0b059dbf7d6b 100644 > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c > @@ -405,6 +405,39 @@ Dump8Chars ( > ); > } > > +/** > + This function traces 12 characters which can be optionally > + formated using the format string if specified. > + > + If no format string is specified the Format must be NULL. > + > + @param [in] Format Optional format string for tracing the data. > + @param [in] Ptr Pointer to the start of the buffer. > +**/ > +VOID > +EFIAPI > +Dump12Chars ( > + IN CONST CHAR16* Format OPTIONAL, > + IN UINT8* Ptr > + ) > +{ > + Print ( > + (Format != NULL) ? Format : L"%c%c%c%c%c%c%c%c%c%c%c%c", > + Ptr[0], > + Ptr[1], > + Ptr[2], > + Ptr[3], > + Ptr[4], > + Ptr[5], > + Ptr[6], > + Ptr[7], > + Ptr[8], > + Ptr[9], > + Ptr[10], > + Ptr[11] > + ); > +} > + > /** > This function indents and prints the ACPI table Field Name. > > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > index > 20ca358bddfa5953bfb1d1bebaebbf3079eaba01..0dc721bd2cc59538432d4a9cab6c > 21728cc77d33 100644 > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > @@ -184,6 +184,22 @@ Dump8Chars ( > IN UINT8* Ptr > ); > > +/** > + This function traces 12 characters which can be optionally > + formated using the format string if specified. > + > + If no format string is specified the Format must be NULL. > + > + @param [in] Format Optional format string for tracing the data. > + @param [in] Ptr Pointer to the start of the buffer. > +**/ > +VOID > +EFIAPI > +Dump12Chars ( > + IN CONST CHAR16* Format OPTIONAL, > + IN UINT8* Ptr > + ); > + > /** > This function indents and prints the ACPI table Field Name. > > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser > .c > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser > .c index > a8aa420487bb6bf29fc38221d0b221573c64b8b3..d60476eb748e022f45d231e24c32 > b2b53e29bbd7 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser > .c > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratPa > +++ rs > +++ er.c > @@ -5,7 +5,7 @@ > SPDX-License-Identifier: BSD-2-Clause-Patent > > @par Reference(s): > - - ACPI 6.2 Specification - Errata A, September 2017 > + - ACPI 6.3 Specification - January 2019 > **/ > > #include <IndustryStandard/Acpi.h> > @@ -17,6 +17,7 @@ > // Local Variables > STATIC CONST UINT8* SratRAType; > STATIC CONST UINT8* SratRALength; > +STATIC CONST UINT8* SratDeviceHandleType; > STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo; > > /** > @@ -32,13 +33,52 @@ EFIAPI > ValidateSratReserved ( > IN UINT8* Ptr, > IN VOID* Context > - ) > -{ > - if (*(UINT32*)Ptr != 1) { > - IncrementErrorCount (); > - Print (L"\nERROR: Reserved should be 1 for backward compatibility.\n"); > - } > -} > + ); > + > +/** > + This function validates the Device Handle Type field in the Generic > +Initiator > + Affinity Structure. > + > + @param [in] Ptr Pointer to the start of the field data. > + @param [in] Context Pointer to context specific information e.g. this > + could be a pointer to the ACPI table header. > +**/ > +STATIC > +VOID > +EFIAPI > +ValidateSratDeviceHandleType ( > + IN UINT8* Ptr, > + IN VOID* Context > + ); > + > +/** > + This function traces the Device Handle field inside Generic > +Initiator > + Affinity Structure. > + > + @param [in] Format Format string for tracing the data. > + @param [in] Ptr Pointer to the start of the buffer. > +**/ > +STATIC > +VOID > +EFIAPI > +DumpSratDeviceHandle ( > + IN CONST CHAR16* Format, > + IN UINT8* Ptr > + ); > + > +/** > + This function traces the PCI BDF Number field inside Device Handle > +- PCI > + > + @param [in] Format Format string for tracing the data. > + @param [in] Ptr Pointer to the start of the buffer. > +**/ > +STATIC > +VOID > +EFIAPI > +DumpSratPciBdfNumber ( > + IN CONST CHAR16* Format, > + IN UINT8* Ptr > + ); > > /** > This function traces the APIC Proximity Domain field. > @@ -52,14 +92,7 @@ EFIAPI > DumpSratApicProximity ( > IN CONST CHAR16* Format, > IN UINT8* Ptr > - ) > -{ > - UINT32 ProximityDomain; > - > - ProximityDomain = Ptr[0] | (Ptr[1] << 8) | (Ptr[2] << 16); > - > - Print (Format, ProximityDomain); > -} > + ); > > /** > An ACPI_PARSER array describing the SRAT Table. > @@ -103,6 +136,41 @@ STATIC CONST ACPI_PARSER SratGicITSAffinityParser[] = { > {L"ITS Id", 4, 8, L"0x%x", NULL, NULL, NULL, NULL}, }; > > +/** > + An ACPI_PARSER array describing the Generic Initiator Affinity > +Structure **/ STATIC CONST ACPI_PARSER > +SratGenericInitiatorAffinityParser[] = { > + {L"Type", 1, 0, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Length", 1, 1, L"0x%x", NULL, NULL, NULL, NULL}, > + > + {L"Reserved", 1, 2, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Device Handle Type", 1, 3, L"%d", NULL, (VOID**)&SratDeviceHandleType, > + ValidateSratDeviceHandleType, NULL}, > + {L"Proximity Domain", 4, 4, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Device Handle", 16, 8, L"%s", DumpSratDeviceHandle, NULL, NULL, > +NULL}, > + {L"Flags", 4, 24, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Reserved", 4, 28, L"0x%x", NULL, NULL, NULL, NULL} }; > + > +/** > + An ACPI_PARSER array describing the Device Handle - ACPI **/ STATIC > +CONST ACPI_PARSER SratDeviceHandleAcpiParser[] = { > + {L"ACPI_HID", 8, 0, L"0x%lx", NULL, NULL, NULL, NULL}, > + {L"ACPI_UID", 4, 8, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Reserved", 4, 12, L"0x%x", NULL, NULL, NULL, NULL} }; > + > +/** > + An ACPI_PARSER array describing the Device Handle - PCI **/ STATIC > +CONST ACPI_PARSER SratDeviceHandlePciParser[] = { > + {L"PCI Segment", 2, 0, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"PCI BDF Number", 2, 2, NULL, DumpSratPciBdfNumber, NULL, NULL, > +NULL}, > + {L"Reserved", 12, 4, L"%x %x %x %x - %x %x %x %x - %x %x %x %x", > Dump12Chars, > + NULL, NULL, NULL} > +}; > + > /** > An ACPI_PARSER array describing the Memory Affinity structure. > **/ > @@ -152,6 +220,190 @@ STATIC CONST ACPI_PARSER SratX2ApciAffinityParser[] = { > {L"Reserved", 4, 20, L"0x%x", NULL, NULL, NULL, NULL} }; > > +/** > + This function validates the Reserved field in the SRAT table header. > + > + @param [in] Ptr Pointer to the start of the field data. > + @param [in] Context Pointer to context specific information e.g. this > + could be a pointer to the ACPI table header. > +**/ > +STATIC > +VOID > +EFIAPI > +ValidateSratReserved ( > + IN UINT8* Ptr, > + IN VOID* Context > + ) > +{ > + if (*(UINT32*)Ptr != 1) { > + IncrementErrorCount (); > + Print (L"\nERROR: Reserved should be 1 for backward > +compatibility.\n"); > + } > +} > + > +/** > + This function validates the Device Handle Type field in the Generic > +Initiator > + Affinity Structure. > + > + @param [in] Ptr Pointer to the start of the field data. > + @param [in] Context Pointer to context specific information e.g. this > + could be a pointer to the ACPI table header. > +**/ > +STATIC > +VOID > +EFIAPI > +ValidateSratDeviceHandleType ( > + IN UINT8* Ptr, > + IN VOID* Context > + ) > +{ > + UINT8 DeviceHandleType; > + > + DeviceHandleType = *Ptr; > + > + if (DeviceHandleType > EFI_ACPI_6_3_PCI_DEVICE_HANDLE) { > + IncrementErrorCount (); > + Print ( > + L"\nERROR: Invalid Device Handle Type: %d. Must be between 0 and %d.", > + DeviceHandleType, > + EFI_ACPI_6_3_PCI_DEVICE_HANDLE > + ); > + } > +} > + > +/** > + This function traces the Device Handle field inside Generic > +Initiator > + Affinity Structure. > + > + @param [in] Format Format string for tracing the data. > + @param [in] Ptr Pointer to the start of the buffer. > +**/ > +STATIC > +VOID > +EFIAPI > +DumpSratDeviceHandle ( > + IN CONST CHAR16* Format, > + IN UINT8* Ptr > + ) > +{ > + if (SratDeviceHandleType == NULL) { > + IncrementErrorCount (); > + Print (L"\nERROR: Device Handle Type read incorrectly.\n"); > + return; > + } > + > + Print (L"\n"); > + > + if (*SratDeviceHandleType == EFI_ACPI_6_3_ACPI_DEVICE_HANDLE) { > + ParseAcpi ( > + TRUE, > + 2, > + NULL, > + Ptr, > + sizeof (EFI_ACPI_6_3_DEVICE_HANDLE_ACPI), > + PARSER_PARAMS (SratDeviceHandleAcpiParser) > + ); > + } else if (*SratDeviceHandleType == EFI_ACPI_6_3_PCI_DEVICE_HANDLE) { > + ParseAcpi ( > + TRUE, > + 2, > + NULL, > + Ptr, > + sizeof (EFI_ACPI_6_3_DEVICE_HANDLE_PCI), > + PARSER_PARAMS (SratDeviceHandlePciParser) > + ); > + } > +} > + > +/** > + This function traces the PCI BDF Number field inside Device Handle > +- PCI > + > + @param [in] Format Format string for tracing the data. > + @param [in] Ptr Pointer to the start of the buffer. > +**/ > +STATIC > +VOID > +EFIAPI > +DumpSratPciBdfNumber ( > + IN CONST CHAR16* Format, > + IN UINT8* Ptr > + ) > +{ > + CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH]; > + > + Print (L"\n"); > + > + /* > + The PCI BDF Number subfields are printed in the order specified in the > ACPI > + specification. The format of the 16-bit PCI BDF Number field is as > follows: > + > + +-----+------+------+ > + |DEV | FUNC | BUS | > + +-----+------+------+ > + |15:11| 10:8 | 7:0 | > + +-----+------+------+ > + */ > + > + // Print PCI Bus Number (Bits 7:0 of Byte 2) UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + L"PCI Bus Number" > + ); > + PrintFieldName (4, Buffer); > + Print ( > + L"0x%x\n", > + *Ptr > + ); > + > + Ptr++; > + > + // Print PCI Device Number (Bits 7:3 of Byte 3) UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + L"PCI Device Number" > + ); > + PrintFieldName (4, Buffer); > + Print ( > + L"0x%x\n", > + (*Ptr & (BIT7 | BIT6 | BIT5 | BIT4 | BIT3)) >> 3 > + ); > + > + // PCI Function Number (Bits 2:0 of Byte 3) > + UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + L"PCI Function Number" > + ); > + PrintFieldName (4, Buffer); > + Print ( > + L"0x%x\n", > + *Ptr & (BIT2 | BIT1 | BIT0) > + ); > +} > + > +/** > + This function traces the APIC Proximity Domain field. > + > + @param [in] Format Format string for tracing the data. > + @param [in] Ptr Pointer to the start of the buffer. > +**/ > +STATIC > +VOID > +EFIAPI > +DumpSratApicProximity ( > + IN CONST CHAR16* Format, > + IN UINT8* Ptr > + ) > +{ > + UINT32 ProximityDomain; > + > + ProximityDomain = Ptr[0] | (Ptr[1] << 8) | (Ptr[2] << 16); > + > + Print (Format, ProximityDomain); > +} > + > /** > This function parses the ACPI SRAT table. > When trace is enabled this function parses the SRAT table and @@ -183,6 > +435,7 @@ ParseAcpiSrat ( > UINT8* ResourcePtr; > UINT32 GicCAffinityIndex; > UINT32 GicITSAffinityIndex; > + UINT32 GenericInitiatorAffinityIndex; > UINT32 MemoryAffinityIndex; > UINT32 ApicSapicAffinityIndex; > UINT32 X2ApicAffinityIndex; > @@ -190,6 +443,7 @@ ParseAcpiSrat ( > > GicCAffinityIndex = 0; > GicITSAffinityIndex = 0; > + GenericInitiatorAffinityIndex = 0; > MemoryAffinityIndex = 0; > ApicSapicAffinityIndex = 0; > X2ApicAffinityIndex = 0; > @@ -232,7 +486,7 @@ ParseAcpiSrat ( > } > > switch (*SratRAType) { > - case EFI_ACPI_6_2_GICC_AFFINITY: > + case EFI_ACPI_6_3_GICC_AFFINITY: > AsciiSPrint ( > Buffer, > sizeof (Buffer), > @@ -249,7 +503,7 @@ ParseAcpiSrat ( > ); > break; > > - case EFI_ACPI_6_2_GIC_ITS_AFFINITY: > + case EFI_ACPI_6_3_GIC_ITS_AFFINITY: > AsciiSPrint ( > Buffer, > sizeof (Buffer), > @@ -266,7 +520,24 @@ ParseAcpiSrat ( > ); > break; > > - case EFI_ACPI_6_2_MEMORY_AFFINITY: > + case EFI_ACPI_6_3_GENERIC_INITIATOR_AFFINITY: > + AsciiSPrint ( > + Buffer, > + sizeof (Buffer), > + "Generic Initiator Affinity Structure [%d]", > + GenericInitiatorAffinityIndex++ > + ); > + ParseAcpi ( > + TRUE, > + 2, > + Buffer, > + ResourcePtr, > + *SratRALength, > + PARSER_PARAMS (SratGenericInitiatorAffinityParser) > + ); > + break; > + > + case EFI_ACPI_6_3_MEMORY_AFFINITY: > AsciiSPrint ( > Buffer, > sizeof (Buffer), > @@ -283,7 +554,7 @@ ParseAcpiSrat ( > ); > break; > > - case EFI_ACPI_6_2_PROCESSOR_LOCAL_APIC_SAPIC_AFFINITY: > + case EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_SAPIC_AFFINITY: > AsciiSPrint ( > Buffer, > sizeof (Buffer), > @@ -300,7 +571,7 @@ ParseAcpiSrat ( > ); > break; > > - case EFI_ACPI_6_2_PROCESSOR_LOCAL_X2APIC_AFFINITY: > + case EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_AFFINITY: > AsciiSPrint ( > Buffer, > sizeof (Buffer), > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#50906): https://edk2.groups.io/g/devel/message/50906 Mute This Topic: https://groups.io/mt/54286297/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
