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 <huangmin...@huawei.com> >> Signed-off-by: Heyi Guo <heyi....@linaro.org> >> --- >> 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 <jeremy.lin...@arm.com> > > > 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel