On 2018/2/1 9:11, Jeremy Linton wrote:
> Hi,
>
>
> On 01/26/2018 02:00 AM, Ming Huang wrote:
>> Add Processor Properties Topology Table, PPTT include
>> Processor hierarchy node, Cache Type Structure and ID structure.
>>
>> PPTT is needed for lscpu command to show socket information correctly.
>> https://bugs.linaro.org/show_bug.cgi?id=3206
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <[email protected]>
>> Signed-off-by: Heyi Guo <[email protected]>
>> ---
>> Platform/Hisilicon/D05/D05.dsc | 1 +
>> Platform/Hisilicon/D05/D05.fdf | 1 +
>> Silicon/Hisilicon/Hi1616/Pptt/Pptt.c | 540 ++++++++++++++++++++
>> Silicon/Hisilicon/Hi1616/Pptt/Pptt.h | 88 ++++
>> Silicon/Hisilicon/Hi1616/Pptt/Pptt.inf | 48 ++
>> 5 files changed, 678 insertions(+)
>>
>> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
>> index 77a89fd..710339c 100644
>> --- a/Platform/Hisilicon/D05/D05.dsc
>> +++ b/Platform/Hisilicon/D05/D05.dsc
>> @@ -506,6 +506,7 @@
>> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
>> Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
>> + Silicon/Hisilicon/Hi1616/Pptt/Pptt.inf
>> Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> #
>> diff --git a/Platform/Hisilicon/D05/D05.fdf b/Platform/Hisilicon/D05/D05.fdf
>> index 78ab0c8..97de4d2 100644
>> --- a/Platform/Hisilicon/D05/D05.fdf
>> +++ b/Platform/Hisilicon/D05/D05.fdf
>> @@ -241,6 +241,7 @@ READ_LOCK_STATUS = TRUE
>> INF Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/AcpiPlatformDxe.inf
>> INF RuleOverride=ACPITABLE
>> Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
>> + INF Silicon/Hisilicon/Hi1616/Pptt/Pptt.inf
>> INF Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> #
>> diff --git a/Silicon/Hisilicon/Hi1616/Pptt/Pptt.c
>> b/Silicon/Hisilicon/Hi1616/Pptt/Pptt.c
>> new file mode 100644
>> index 0000000..71c456c
>> --- /dev/null
>> +++ b/Silicon/Hisilicon/Hi1616/Pptt/Pptt.c
>> @@ -0,0 +1,540 @@
>> +/** @file
>> +*
>> +* Copyright (c) 2018, Hisilicon Limited. All rights reserved.
>> +* Copyright (c) 2018, Linaro Limited. All rights reserved.
>> +*
>> +* This program and the accompanying materials
>> +* are licensed and made available under the terms and conditions of the
>> BSD License
>> +* which accompanies this distribution. The full text of the license may
>> be found at
>> +* http://opensource.org/licenses/bsd-license.php
>> +*
>> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
>> IMPLIED.
>> +*
>> +* Based on the files under Platform/ARM/JunoPkg/AcpiTables/
>> +*
>> +**/
>> +
>> +#include "Pptt.h"
>> +
>> +EFI_ACPI_TABLE_PROTOCOL *mAcpiTableProtocol = NULL;
>> +EFI_ACPI_SDT_PROTOCOL *mAcpiSdtProtocol = NULL;
>> +
>> +EFI_ACPI_DESCRIPTION_HEADER mPpttHeader =
>> + ARM_ACPI_HEADER (
>> + EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
>> + EFI_ACPI_DESCRIPTION_HEADER,
>> + EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION
>> + );
>> +
>> +EFI_ACPI_6_2_PPTT_STRUCTURE_ID mPpttSocketType2[PPTT_SOCKET_COMPONENT_NO] =
>> +{
>> + {2, sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_ID), {0, 0}, 0, 0, 0, 0, 0, 0}
>> +};
>
> I missed this the first time. I think at a minimum the VENDOR_ID needs to
> contain something other than 0 if your populating a type2 structure. Did I
> miss it getting overridden somewhere?
>
> Checking the ACPI id, registry there is an existing entry for Hisilicon
> Technologies Co.., LTD and its 'HISI'.
>
> I would suggest you use that, and come up with a plan for how the remaining
> fields are provided.
>
>
VENDOR_ID uses the value of 'HISI', there is no a appropriate value for
remaining fields now.
>> +
>> +EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE mPpttCacheType1[PPTT_CACHE_NO];
>> +
>> +
>> +STATIC
>> +VOID
>> +InitCacheInfo (
>> + VOID
>> + )
>> +{
>> + UINT8 Index;
>> + EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES Type1Attributes;
>> + CSSELR_DATA CsselrData;
>> + CCSIDR_DATA CcsidrData;
>> +
>> + for (Index = 0; Index < PPTT_CACHE_NO; Index++) {
>> + CsselrData.Data = 0;
>> + CcsidrData.Data = 0;
>> + SetMem (
>> + &Type1Attributes,
>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES),
>> + 0
>> + );
>> +
>> + if (Index == 0) { //L1I
>> + CsselrData.Bits.InD = 1;
>> + CsselrData.Bits.Level = 0;
>> + Type1Attributes.CacheType = 1;
>> + } else if (Index == 1) {
>> + Type1Attributes.CacheType = 0;
>> + CsselrData.Bits.Level = Index - 1;
>> + } else {
>> + Type1Attributes.CacheType = 2;
>> + CsselrData.Bits.Level = Index - 1;
>> + }
>> +
>> + CcsidrData.Data = ReadCCSIDR (CsselrData.Data);
>> +
>> + if (CcsidrData.Bits.Wa == 1) {
>> + Type1Attributes.AllocationType = PPTT_TYPE1_ALLOCATION_WRITE;
>> + if (CcsidrData.Bits.Ra == 1) {
>> + Type1Attributes.AllocationType = PPTT_TYPE1_ALLOCATION_READ_WRITE;
>> + }
>> + }
>> +
>> + if (CcsidrData.Bits.Wt == 1) {
>> + Type1Attributes.WritePolicy = 1;
>> + }
>> + DEBUG ((DEBUG_INFO,
>> + "[Acpi PPTT] Level = %x!CcsidrData = %x!\n",
>> + CsselrData.Bits.Level,
>> + CcsidrData.Data));
>> +
>> + mPpttCacheType1[Index].Type = EFI_ACPI_6_2_PPTT_TYPE_CACHE;
>> + mPpttCacheType1[Index].Length = sizeof
>> (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE);
>> + mPpttCacheType1[Index].Reserved[0] = 0;
>> + mPpttCacheType1[Index].Reserved[1] = 0;
>> + mPpttCacheType1[Index].Flags.SizePropertyValid = 1;
>> + mPpttCacheType1[Index].Flags.NumberOfSetsValid = 1;
>> + mPpttCacheType1[Index].Flags.AssociativityValid = 1;
>> + mPpttCacheType1[Index].Flags.AllocationTypeValid = 1;
>> + mPpttCacheType1[Index].Flags.CacheTypeValid = 1;
>> + mPpttCacheType1[Index].Flags.WritePolicyValid = 1;
>> + mPpttCacheType1[Index].Flags.LineSizeValid = 1;
>> + mPpttCacheType1[Index].Flags.Reserved = 0;
>> + mPpttCacheType1[Index].NextLevelOfCache = 0;
>> +
>> + if (Index != PPTT_CACHE_NO - 1) {
>> + mPpttCacheType1[Index].NumberOfSets = (UINT16)CcsidrData.Bits.NumSets
>> + 1;
>> + mPpttCacheType1[Index].Associativity =
>> (UINT16)CcsidrData.Bits.Associativity + 1;
>> + mPpttCacheType1[Index].LineSize = (UINT16)( 1 <<
>> (CcsidrData.Bits.LineSize + 4));
>> + mPpttCacheType1[Index].Size = mPpttCacheType1[Index].LineSize * \
>> + mPpttCacheType1[Index].Associativity * \
>> + mPpttCacheType1[Index].NumberOfSets;
>> + CopyMem (
>> + &mPpttCacheType1[Index].Attributes,
>> + &Type1Attributes,
>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES)
>> + );
>> + } else {
>> + // L3 cache
>> + mPpttCacheType1[Index].Size = 0x1000000; // 16m
>> + mPpttCacheType1[Index].NumberOfSets = 0x2000;
>> + mPpttCacheType1[Index].Associativity = 0x10; //
>> CacheAssociativity16Way
>> + SetMem (
>> + &mPpttCacheType1[Index].Attributes,
>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES),
>> + 0x0A
>> + );
>> + mPpttCacheType1[Index].LineSize = 0x80; // 128byte
>> + }
>> + }
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +AddCoreTable (
>> + IN EFI_ACPI_DESCRIPTION_HEADER *PpttTable,
>> + IN OUT UINT32 *PpttTableLengthRemain,
>> + IN UINT32 Flags,
>> + IN UINT32 Parent,
>> + IN UINT32 ResourceNo,
>> + IN UINT32 ProcessorId
>> + )
>> +{
>> + EFI_ACPI_6_2_PPTT_TYPE0 *PpttType0;
>> + EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE *PpttType1;
>> + UINT32 *PrivateResource;
>> + UINT8 Index;
>> +
>> + if (*PpttTableLengthRemain <
>> + (sizeof (EFI_ACPI_6_2_PPTT_TYPE0) + ResourceNo * 4)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + PpttType0 = (EFI_ACPI_6_2_PPTT_TYPE0 *)((UINT8 *)PpttTable +
>> + PpttTable->Length);
>> + PpttType0->Type = 0;
>> + CopyMem (&PpttType0->Flags, &Flags, sizeof(PpttType0->Flags));;
>> + PpttType0->Parent= Parent;
>> + PpttType0->AcpiProcessorId = ProcessorId;
>> + PpttType0->NumberOfPrivateResources = ResourceNo;
>> + PpttType0->Length = sizeof (EFI_ACPI_6_2_PPTT_TYPE0) +
>> + ResourceNo * 4;
>> +
>> + *PpttTableLengthRemain -= (UINTN)PpttType0->Length;
>> + PpttTable->Length += PpttType0->Length;
>> + PrivateResource = (UINT32 *)((UINT8 *)PpttType0 +
>> + sizeof (EFI_ACPI_6_2_PPTT_TYPE0));
>> +
>> + // Add cache type structure
>> + for (Index = 0; Index < ResourceNo; Index++, PrivateResource++) {
>> + if (*PpttTableLengthRemain < sizeof
>> (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + *PrivateResource = PpttTable->Length;
>> + PpttType1 = (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE *)((UINT8 *)PpttTable +
>> + PpttTable->Length);
>> + gBS->CopyMem (
>> + PpttType1,
>> + &mPpttCacheType1[Index],
>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE)
>> + );
>> + *PpttTableLengthRemain -= PpttType1->Length;
>> + PpttTable->Length += PpttType1->Length;
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +AddClusterTable (
>> + IN EFI_ACPI_DESCRIPTION_HEADER *PpttTable,
>> + IN OUT UINT32 *PpttTableLengthRemain,
>> + IN UINT32 Flags,
>> + IN UINT32 Parent,
>> + IN UINT32 ResourceNo
>> + )
>> +{
>> + EFI_ACPI_6_2_PPTT_TYPE0 *PpttType0;
>> + EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE *PpttType1;
>> + UINT32 *PrivateResource;
>> +
>> + if ((*PpttTableLengthRemain) <
>> + (sizeof (EFI_ACPI_6_2_PPTT_TYPE0) + ResourceNo * 4)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + PpttType0 = (EFI_ACPI_6_2_PPTT_TYPE0 *)((UINT8 *)PpttTable +
>> + PpttTable->Length);
>> + PpttType0->Type = 0;
>> + CopyMem (&PpttType0->Flags, &Flags, sizeof(PpttType0->Flags));;
>> + PpttType0->Parent= Parent;
>> + PpttType0->NumberOfPrivateResources = ResourceNo;
>> + PpttType0->Length = sizeof (EFI_ACPI_6_2_PPTT_TYPE0) +
>> + ResourceNo * 4;
>> +
>> + *PpttTableLengthRemain -= PpttType0->Length;
>> + PpttTable->Length += PpttType0->Length;
>> + PrivateResource = (UINT32 *)((UINT8 *)PpttType0 +
>> + sizeof (EFI_ACPI_6_2_PPTT_TYPE0));
>> +
>> + // Add cache type structure
>> + if (*PpttTableLengthRemain < sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + *PrivateResource = PpttTable->Length;
>> + PpttType1 = (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE *)((UINT8 *)PpttTable +
>> + PpttTable->Length);
>> + gBS->CopyMem (
>> + PpttType1,
>> + &mPpttCacheType1[2],
>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE)
>> + );
>> + *PpttTableLengthRemain -= PpttType1->Length;
>> + PpttTable->Length += PpttType1->Length;
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +AddScclTable (
>> + IN EFI_ACPI_DESCRIPTION_HEADER *PpttTable,
>> + IN OUT UINT32 *PpttTableLengthRemain,
>> + IN UINT32 Flags,
>> + IN UINT32 Parent,
>> + IN UINT32 ResourceNo
>> + )
>> +{
>> + EFI_ACPI_6_2_PPTT_TYPE0 *PpttType0;
>> + EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE *PpttType1;
>> + UINT32 *PrivateResource;
>> +
>> + if (*PpttTableLengthRemain <
>> + (sizeof (EFI_ACPI_6_2_PPTT_TYPE0) + ResourceNo * 4)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + PpttType0 = (EFI_ACPI_6_2_PPTT_TYPE0 *)((UINT8 *)PpttTable +
>> + PpttTable->Length);
>> + PpttType0->Type = 0;
>> + CopyMem (&PpttType0->Flags, &Flags, sizeof(PpttType0->Flags));;
>> + PpttType0->Parent= Parent;
>> + PpttType0->NumberOfPrivateResources = ResourceNo;
>> + PpttType0->Length = sizeof (EFI_ACPI_6_2_PPTT_TYPE0) +
>> + ResourceNo * 4;
>> +
>> + *PpttTableLengthRemain -= PpttType0->Length;
>> + PpttTable->Length += PpttType0->Length;
>> + PrivateResource = (UINT32 *)((UINT8 *)PpttType0 +
>> + sizeof (EFI_ACPI_6_2_PPTT_TYPE0));
>> +
>> + // Add cache type structure
>> + if (*PpttTableLengthRemain < sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + *PrivateResource = PpttTable->Length;
>> + PpttType1 = (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE *)((UINT8 *)PpttTable +
>> + PpttTable->Length);
>> + gBS->CopyMem (
>> + PpttType1,
>> + &mPpttCacheType1[3],
>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE)
>> + );
>> + *PpttTableLengthRemain -= PpttType1->Length;
>> + PpttTable->Length += PpttType1->Length;
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +AddSocketTable (
>> + IN EFI_ACPI_DESCRIPTION_HEADER *PpttTable,
>> + IN OUT UINT32 *PpttTableLengthRemain,
>> + IN UINT32 Flags,
>> + IN UINT32 Parent,
>> + IN UINT32 ResourceNo
>> + )
>> +{
>> + EFI_ACPI_6_2_PPTT_TYPE0 *PpttType0;
>> + EFI_ACPI_6_2_PPTT_STRUCTURE_ID *PpttType2;
>> + UINT32 *PrivateResource;
>> + UINT8 Index;
>> +
>> + if (*PpttTableLengthRemain < sizeof (EFI_ACPI_6_2_PPTT_TYPE0)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + PpttType0 = (EFI_ACPI_6_2_PPTT_TYPE0 *)((UINT8 *)PpttTable +
>> + PpttTable->Length);
>> + PpttType0->Type = 0;
>> + CopyMem (&PpttType0->Flags, &Flags, sizeof(PpttType0->Flags));;
>> + PpttType0->Parent= Parent;
>> + PpttType0->NumberOfPrivateResources = ResourceNo;
>> + PpttType0->Length = sizeof (EFI_ACPI_6_2_PPTT_TYPE0) +
>> + ResourceNo * 4;
>> + PpttTable->Length += PpttType0->Length;
>> +
>> + *PpttTableLengthRemain -= PpttType0->Length;
>> + if (*PpttTableLengthRemain < ResourceNo * 4) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + PrivateResource = (UINT32 *)((UINT8 *)PpttType0 +
>> + sizeof (EFI_ACPI_6_2_PPTT_TYPE0));
>> + DEBUG ((DEBUG_INFO,
>> + "[Acpi PPTT] sizeof(EFI_ACPI_6_2_PPTT_STRUCTURE_ID) = %x!\n",
>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_ID)));
>> +
>> + for (Index = 0; Index < ResourceNo; Index++, PrivateResource++) {
>> + if (*PpttTableLengthRemain < sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_ID)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> + *PrivateResource = PpttTable->Length;
>> + PpttType2 = (EFI_ACPI_6_2_PPTT_STRUCTURE_ID *)((UINT8 *)PpttTable +
>> + PpttTable->Length);
>> + gBS->CopyMem (
>> + PpttType2,
>> + &mPpttSocketType2[Index],
>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_ID)
>> + );
>> + *PpttTableLengthRemain -= PpttType2->Length;
>> + PpttTable->Length += PpttType2->Length;
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +VOID
>> +GetApic (
>> + IN EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE *ApicTable,
>> + IN OUT EFI_ACPI_DESCRIPTION_HEADER *PpttTable,
>> + IN UINT32 PpttTableLengthRemain,
>> + IN UINT32 Index1
>> +)
>> +{
>> + UINT32 IndexSocket, IndexSccl, IndexCluster, IndexCore;
>> + UINT32 SocketOffset, ScclOffset, ClusterOffset;
>> + UINT32 Parent = 0;
>> + UINT32 Flags = 0;
>> + UINT32 ResourceNo = 0;
>> +
>> + // Get APIC data
>> + for (IndexSocket = 0; IndexSocket < PPTT_SOCKET_NO; IndexSocket++) {
>> + SocketOffset = 0;
>> + for (IndexSccl = 0; IndexSccl < PPTT_SCCL_NO; IndexSccl++) {
>> + ScclOffset = 0;
>> + for (IndexCluster = 0; IndexCluster < PPTT_CLUSTER_NO;
>> IndexCluster++) {
>> + ClusterOffset = 0;
>> + for (IndexCore = 0; IndexCore < PPTT_CORE_NO; IndexCore++) {
>> + if (ApicTable->GicInterfaces[Index1].AcpiProcessorUid != Index1) {
>> + // This processor is unusable
>> + DEBUG ((DEBUG_ERROR, "[Acpi PPTT] Please check MADT table for
>> UID!\n"));
>> + return;
>> + }
>> + if ((ApicTable->GicInterfaces[Index1].Flags & BIT0) == 0) {
>> + // This processor is unusable
>> + Index1++;
>> + continue;
>> + }
>> +
>> + if (SocketOffset == 0) {
>> + // Add socket0 for type0 table
>> + ResourceNo = PPTT_SOCKET_COMPONENT_NO;
>> + SocketOffset = PpttTable->Length;
>> + Parent = 0;
>> + Flags = PPTT_TYPE0_SOCKET_FLAG;
>> + AddSocketTable (
>> + PpttTable,
>> + &PpttTableLengthRemain,
>> + Flags,
>> + Parent,
>> + ResourceNo
>> + );
>> + }
>> + if (ScclOffset == 0) {
>> + // Add socket0sccl0 for type0 table
>> + ResourceNo = 1;
>> + ScclOffset = PpttTable->Length;
>> + Parent = SocketOffset;
>> + Flags = PPTT_TYPE0_SCCL_FLAG;
>> + AddScclTable (
>> + PpttTable,
>> + &PpttTableLengthRemain,
>> + Flags,
>> + Parent,
>> + ResourceNo
>> + );
>> + }
>> + if (ClusterOffset == 0) {
>> + // Add socket0sccl0ClusterId for type0 table
>> + ResourceNo = 1;
>> + ClusterOffset = PpttTable->Length ;
>> + Parent = ScclOffset;
>> + Flags = PPTT_TYPE0_CLUSTER_FLAG;
>> + AddClusterTable (
>> + PpttTable,
>> + &PpttTableLengthRemain,
>> + Flags,
>> + Parent,
>> + ResourceNo
>> + );
>> + }
>> +
>> + // Add socket0sccl0ClusterIdCoreId for type0 table
>> + ResourceNo = 2;
>> + Parent = ClusterOffset;
>> + Flags = PPTT_TYPE0_CORE_FLAG;
>> + AddCoreTable (
>> + PpttTable,
>> + &PpttTableLengthRemain,
>> + Flags,
>> + Parent,
>> + ResourceNo,
>> + Index1
>> + );
>> +
>> + Index1++;
>> + }
>> + }
>> + }
>> + }
>> + return ;
>> +}
>> +
>> +STATIC
>> +VOID
>> +PpttSetAcpiTable (
>> + IN EFI_EVENT Event,
>> + IN VOID *Context
>> + )
>> +{
>> + UINTN AcpiTableHandle;
>> + EFI_STATUS Status;
>> + UINT8 Checksum;
>> + EFI_ACPI_SDT_HEADER *Table;
>> + EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE *ApicTable;
>> + EFI_ACPI_TABLE_VERSION TableVersion;
>> + EFI_ACPI_DESCRIPTION_HEADER *PpttTable;
>> + UINTN TableKey;
>> + UINT32 Index0, Index1;
>> + UINT32 PpttTableLengthRemain = 0;
>> +
>> + gBS->CloseEvent (Event);
>> +
>> + InitCacheInfo ();
>> +
>> + PpttTable = (EFI_ACPI_DESCRIPTION_HEADER *)AllocateZeroPool
>> (PPTT_TABLE_MAX_LEN);
>> + gBS->CopyMem (
>> + (VOID *)PpttTable,
>> + &mPpttHeader,
>> + sizeof (EFI_ACPI_DESCRIPTION_HEADER)
>> + );
>> + PpttTableLengthRemain = PPTT_TABLE_MAX_LEN - sizeof
>> (EFI_ACPI_DESCRIPTION_HEADER);
>> +
>> + for (Index0 = 0; Index0 < EFI_ACPI_MAX_NUM_TABLES; Index0++) {
>> + Status = mAcpiSdtProtocol->GetAcpiTable (
>> + Index0,
>> + &Table,
>> + &TableVersion,
>> + &TableKey
>> + );
>> + if (EFI_ERROR (Status)) {
>> + break;
>> + }
>> +
>> + // Find APIC table
>> + if (Table->Signature ==
>> EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE) {
>> + break;
>> + }
>> +
>> + }
>> +
>> + if (!EFI_ERROR (Status) && (Index0 != EFI_ACPI_MAX_NUM_TABLES)) {
>> + ApicTable = (EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE *)Table;
>> + Index1 = 0;
>> +
>> + GetApic (ApicTable, PpttTable, PpttTableLengthRemain, Index1);
>> +
>> + Checksum = CalculateCheckSum8 ((UINT8 *)(PpttTable), PpttTable->Length);
>> + PpttTable->Checksum = Checksum;
>> +
>> + AcpiTableHandle = 0;
>> + Status = mAcpiTableProtocol->InstallAcpiTable (
>> + mAcpiTableProtocol,
>> + PpttTable,
>> + PpttTable->Length,
>> + &AcpiTableHandle);
>> + }
>> +
>> + FreePool (PpttTable);
>> + return ;
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +PpttEntryPoint(
>> + IN EFI_HANDLE ImageHandle,
>> + IN EFI_SYSTEM_TABLE *SystemTable
>> + )
>> +{
>> + EFI_STATUS Status;
>> + EFI_EVENT ReadyToBootEvent;
>> +
>> + Status = gBS->LocateProtocol (
>> + &gEfiAcpiTableProtocolGuid,
>> + NULL,
>> + (VOID **)&mAcpiTableProtocol);
>> + ASSERT_EFI_ERROR (Status);
>> +
>> + Status = gBS->LocateProtocol (
>> + &gEfiAcpiSdtProtocolGuid,
>> + NULL,
>> + (VOID **)&mAcpiSdtProtocol);
>> + ASSERT_EFI_ERROR (Status);
>> +
>> + Status = EfiCreateEventReadyToBootEx (
>> + TPL_NOTIFY,
>> + PpttSetAcpiTable,
>> + NULL,
>> + &ReadyToBootEvent
>> + );
>> + ASSERT_EFI_ERROR (Status);
>> +
>> + DEBUG ((DEBUG_INFO, "Acpi Pptt init done.\n"));
>> +
>> + return Status;
>> +}
>> diff --git a/Silicon/Hisilicon/Hi1616/Pptt/Pptt.h
>> b/Silicon/Hisilicon/Hi1616/Pptt/Pptt.h
>> new file mode 100644
>> index 0000000..57f8386
>> --- /dev/null
>> +++ b/Silicon/Hisilicon/Hi1616/Pptt/Pptt.h
>> @@ -0,0 +1,88 @@
>> +/** @file
>> +*
>> +* Copyright (c) 2018, Hisilicon Limited. All rights reserved.
>> +* Copyright (c) 2018, Linaro Limited. All rights reserved.
>> +*
>> +* This program and the accompanying materials
>> +* are licensed and made available under the terms and conditions of the
>> BSD License
>> +* which accompanies this distribution. The full text of the license may
>> be found at
>> +* http://opensource.org/licenses/bsd-license.php
>> +*
>> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
>> IMPLIED.
>> +*
>> +* Based on the files under Platform/ARM/JunoPkg/AcpiTables/
>> +*
>> +**/
>> +
>> +#ifndef _PPTT_H_
>> +#define _PPTT_H_
>> +
>> +#include <IndustryStandard/Acpi.h>
>> +#include <Library/ArmLib/ArmLibPrivate.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Protocol/AcpiSystemDescriptionTable.h>
>> +#include <Protocol/AcpiTable.h>
>> +#include "../D05AcpiTables/Hi1616Platform.h"
>> +
>> +#define EFI_ACPI_MAX_NUM_TABLES 20
>> +
>> +#define PPTT_TABLE_MAX_LEN 0x6000
>> +#define PPTT_SOCKET_NO 0x2
>> +#define PPTT_SCCL_NO 0x2
>> +#define PPTT_CLUSTER_NO 0x4
>> +#define PPTT_CORE_NO 0x4
>> +#define PPTT_SOCKET_COMPONENT_NO 0x1
>> +#define PPTT_CACHE_NO 0x4
>> +
>> +#define PPTT_TYPE0_PHYSICAL_PKG BIT0
>> +#define PPTT_TYPE0_PROCESSORID_VALID BIT1
>
> I think these two flags belong in the acpi header file.. That said they are
> already defined by the bitfields in
> EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR_FLAGS so it might be a good idea to
> just remove them and convert the following definitions to the struct.
>
> That said, its a bit ugly either way, so your choice.
I plan to move these tow flags to Acpi62.h, and optimize the code for Flags.
>
>
>> +#define PPTT_TYPE0_SOCKET_FLAG PPTT_TYPE0_PHYSICAL_PKG
>> +#define PPTT_TYPE0_SCCL_FLAG 0
>> +#define PPTT_TYPE0_CLUSTER_FLAG 0
>> +#define PPTT_TYPE0_CORE_FLAG PPTT_TYPE0_PROCESSORID_VALID
>> +
>> +#define PPTT_TYPE1_ALLOCATION_WRITE 0x1
>> +#define PPTT_TYPE1_ALLOCATION_READ_WRITE 0x2
>
> Its more clear for these two, they should be in the acpi header file. While
> your at it the write policy and cache type should also probably be defined
> and used in your init routing.
>
>
I plan to move these two macro to Acpi62.h.
While your at it the write policy and cache type should also probably be
defined and used in your init routing
I don't really understand the mean above.
>> +
>> +typedef union {
>> + struct {
>> + UINT32 InD :1;
>> + UINT32 Level :3;
>> + UINT32 Reserved :28;
>> + } Bits;
>> + UINT32 Data;
>> +} CSSELR_DATA;
>> +
>> +typedef union {
>> + struct {
>> + UINT32 LineSize :3;
>> + UINT32 Associativity :10;
>> + UINT32 NumSets :15;
>> + UINT32 Wa :1;
>> + UINT32 Ra :1;
>> + UINT32 Wb :1;
>> + UINT32 Wt :1;
>> + } Bits;
>> + UINT32 Data;
>> +} CCSIDR_DATA;
>
> This feels like it belongs in a diffrent header file closer to the
> ReadCCISDR() function (ArmLibPrivate.h).
>
I compare those macro of the ArmLibPrivae.h and these two struct. Those macro
are not equivalent to
these two struct, like offset of Associativity is 3 in CCSIDR_DATA (as
DDI0487A_h_armv8_arm.pdf),
but 15 in ArmLibPrivae.h, and miss NumSets in ArmLibPrivae.h, and so on.
>> +
>> +//
>> +// Processor Hierarchy Node Structure
>> +//
>> +typedef struct {
>> + UINT8 Type;
>> + UINT8 Length;
>> + UINT8 Reserved[2];
>> + UINT32 Flags;
>> + UINT32 Parent;
>> + UINT32 AcpiProcessorId;
>> + UINT32 NumberOfPrivateResources;
>> +} EFI_ACPI_6_2_PPTT_TYPE0;
>
> This belongs in the common ACPI header too.
>
I have replace it with struct in Acpi62.h.
Thanks,
Ming
>> +
>> +#endif // _PPTT_H_
>> +
>> diff --git a/Silicon/Hisilicon/Hi1616/Pptt/Pptt.inf
>> b/Silicon/Hisilicon/Hi1616/Pptt/Pptt.inf
>> new file mode 100644
>> index 0000000..ff6f772
>> --- /dev/null
>> +++ b/Silicon/Hisilicon/Hi1616/Pptt/Pptt.inf
>> @@ -0,0 +1,48 @@
>> +/** @file
>> +*
>> +* Copyright (c) 2018, Hisilicon Limited. All rights reserved.
>> +* Copyright (c) 2018, Linaro Limited. All rights reserved.
>> +*
>> +* This program and the accompanying materials
>> +* are licensed and made available under the terms and conditions of the
>> BSD License
>> +* which accompanies this distribution. The full text of the license may
>> be found at
>> +* http://opensource.org/licenses/bsd-license.php
>> +*
>> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
>> IMPLIED.
>> +*
>> +* Based on the files under Platform/ARM/JunoPkg/AcpiTables/
>> +*
>> +**/
>> +
>> +[Defines]
>> + INF_VERSION = 0x0001001A
>> + BASE_NAME = AcpiPptt
>> + FILE_GUID = AAB14F90-DC2E-4f33-A594-C7894A5B412D
>> + MODULE_TYPE = DXE_DRIVER
>> + VERSION_STRING = 1.0
>> + ENTRY_POINT = PpttEntryPoint
>> +
>> +[Sources.common]
>> + Pptt.c
>> +
>> +[Packages]
>> + ArmPkg/ArmPkg.dec
>> + MdePkg/MdePkg.dec
>> + Silicon/Hisilicon/HisiPkg.dec
>> +
>> +[LibraryClasses]
>> + ArmLib
>> + BaseMemoryLib
>> + DebugLib
>> + HobLib
>> + UefiDriverEntryPoint
>> + UefiRuntimeServicesTableLib
>> +
>> +[Protocols]
>> + gEfiAcpiSdtProtocolGuid ## PROTOCOL ALWAYS_CONSUMED
>> + gEfiAcpiTableProtocolGuid ## PROTOCOL ALWAYS_CONSUMED
>> +
>> +[Depex]
>> + gEfiAcpiTableProtocolGuid AND gEfiAcpiSdtProtocolGuid
>> +
>>
>
> Ok, so other than the above comments i'm pretty happy with it. IMHO the
> largest problem is the Type2 id definition which definitly needs to be 'HISI'.
>
> Reveiwed-by: Jeremy Linton <[email protected]>
>
>
> Generally, I think this is a pretty good way to build the table. Enough so,
> that the next person to do an arm PPTT implementation should make an effort
> to move some of your helper functions (InitCacheInfo, Add* routines) into a
> generic library and tweak them enough that they can be resused across
> multiple platforms.
>
> Thanks,
>
> .
>
--
Best Regards,
Ming
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
