From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.12829.1611940277794810366 for ; Fri, 29 Jan 2021 09:11:18 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: vijayenthiran.subramaniam@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 38372152B for ; Fri, 29 Jan 2021 09:11:16 -0800 (PST) Received: from mail-wr1-f48.google.com (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 14CB63FA45 for ; Fri, 29 Jan 2021 09:11:16 -0800 (PST) Received: by mail-wr1-f48.google.com with SMTP id a1so9608094wrq.6 for ; Fri, 29 Jan 2021 09:11:16 -0800 (PST) X-Gm-Message-State: AOAM533yq0waJnGAsOLuy8GErgiOeRB0JJ8SfhH20kfFHp/LPUJ3+CbJ y2RKTgSMY8FRAtKk8oIGkm+glR/1CP3gJvipmrI= X-Google-Smtp-Source: ABdhPJxDEcS7DisPkgQR/emLLMKriKii/K2HTFkOWuj+ylGpj+e2ycGXraxXSI32WLhGIilTD+H2U65ylrX4orKA/XI= X-Received: by 2002:a5d:6884:: with SMTP id h4mr5894132wru.106.1611940274784; Fri, 29 Jan 2021 09:11:14 -0800 (PST) MIME-Version: 1.0 References: <1611841351-5039-1-git-send-email-vijayenthiran.subramaniam@arm.com> <1611841351-5039-3-git-send-email-vijayenthiran.subramaniam@arm.com> <165E70A6BEEFF3E5.24597@groups.io> <20210129094940.00004799@Huawei.com> In-Reply-To: <20210129094940.00004799@Huawei.com> From: "Vijayenthiran Subramaniam" Date: Fri, 29 Jan 2021 17:11:03 +0000 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [edk2-platforms] [PATCH v1 2/3] Platform/ARM/SgiPkg: Add HMAT ACPI table for RdN1EdgeX2 To: devel@edk2.groups.io, jonathan.cameron@huawei.com Cc: "Jonathan Cameron via groups.io" , Vijayenthiran Subramaniam , leif@nuviainc.com, ardb+tianocore@kernel.org, Sami Mujawar , Thomas Abraham Content-Type: text/plain; charset="UTF-8" Hi Jonathan, On Fri, Jan 29, 2021 at 9:50 AM Jonathan Cameron via groups.io wrote: > > On Thu, 28 Jan 2021 15:58:48 +0000 > Jonathan Cameron via groups.io wrote: > > > On Thu, 28 Jan 2021 19:12:30 +0530 > > Vijayenthiran Subramaniam wrote: > > > > > Add HMAT table support for RD-N1-Edge Dual-chip platform. > > > > > > Signed-off-by: Vijayenthiran Subramaniam > > > --- > > > Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf | 1 + > > > Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Hmat.aslc | 108 ++++++++++++++++++++ > > > 2 files changed, 109 insertions(+) > > > > > > diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf > > > index d44f02ab0c16..36d41281439d 100644 > > > --- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf > > > +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf > > > @@ -22,6 +22,7 @@ [Sources] > > > Iort.aslc > > > Mcfg.aslc > > > RdN1Edge/Dsdt.asl > > > + RdN1EdgeX2/Hmat.aslc > > > RdN1EdgeX2/Madt.aslc > > > RdN1EdgeX2/Srat.aslc > > > Spcr.aslc > > > diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Hmat.aslc b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Hmat.aslc > > > new file mode 100644 > > > index 000000000000..29d089aed053 > > > --- /dev/null > > > +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Hmat.aslc > > > @@ -0,0 +1,108 @@ > > > +/** @file > > > +* Heterogeneous Memory Attribute Table (HMAT) > > > +* > > > +* Copyright (c) 2020, ARM Limited. All rights reserved. > > > +* > > > +* SPDX-License-Identifier: BSD-2-Clause-Patent > > > +* > > > +**/ > > > + > > > +#include "SgiAcpiHeader.h" > > > +#include "SgiPlatform.h" > > > +#include > > > +#include > > > +#include > > > + > > > +#define CHIP_CNT FixedPcdGet32 (PcdChipCount) > > > +#define INITATOR_PROXIMITY_DOMAIN_CNT 2 > > > +#define TARGET_PROXIMITY_DOMIAIN_CNT 2 > > > > DOMAIN Ack, will fix in v2. > > > > > + > > > +// > > > +// HMAT Table > > > +// > > > +#pragma pack (1) > > > + > > > +typedef struct { > > > + UINT32 InitatorProximityDomain[INITATOR_PROXIMITY_DOMAIN_CNT]; > > > + UINT32 TargetProximityDomiain[TARGET_PROXIMITY_DOMIAIN_CNT]; > > > + UINT16 MatrixEntry[INITATOR_PROXIMITY_DOMAIN_CNT * TARGET_PROXIMITY_DOMIAIN_CNT]; > > > +} InitiatorTargetProximityMatrix; > > > + > > > +typedef struct { > > > + EFI_ACPI_6_3_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE_HEADER Header; > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES Proximity[CHIP_CNT]; > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_SYSTEM_LOCALITY_LATENCY_AND_BANDWIDTH_INFO LatencyInfo; > > > + InitiatorTargetProximityMatrix Matrix; > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO MemSideCache0; > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO MemSideCache1; > > > +} EFI_ACPI_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE; > > > + > > > +#pragma pack () > > > + > > > +#define HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTRIBUTES_INIT( \ > > > + TotalCacheLevels, CacheLevel, CacheAssociativity, WritePolicy, CacheLineSize \ > > > + ) \ > > > +{ \ > > > + TotalCacheLevels, CacheLevel, CacheAssociativity, WritePolicy, CacheLineSize \ > > > +} > > > + > > > +EFI_ACPI_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE Hmat = { > > > + // Header > > > + { > > > + ARM_ACPI_HEADER ( > > > + EFI_ACPI_6_3_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE_SIGNATURE, > > > + EFI_ACPI_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE, > > > + EFI_ACPI_6_3_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE_REVISION > > > + ), > > > + { > > > + EFI_ACPI_RESERVED_BYTE, > > > + EFI_ACPI_RESERVED_BYTE, > > > + EFI_ACPI_RESERVED_BYTE, > > > + EFI_ACPI_RESERVED_BYTE > > > + }, > > > + }, > > > + > > > + // Memory Proximity Domain > > > + { > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES_INIT ( > > > + 1, 0x0, 0x0), > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES_INIT ( > > > + 1, 0x1, 0x1), > > > + }, > > > + > > > + // Latency Info > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_SYSTEM_LOCALITY_LATENCY_AND_BANDWIDTH_INFO_INIT ( > > > + 0, 0, INITATOR_PROXIMITY_DOMAIN_CNT, TARGET_PROXIMITY_DOMIAIN_CNT, 100), > > > + { > > > + {0, 1}, {0, 1}, > > > + { > > > + 10, 20, > > > + 20, 10, > > > + } > > > + }, > > Hi, > > Now either you have a very odd machine in which latencies are round numbers that happen > to look like the values in SLIT or ... ? > > HMAT has very carefully defined real world units to overcome the fact that there > was no standard scaling etc for SLIT. It is not a good idea to put tables out > there which don't do this right as someone may copy this for a real system > and we end up with HMAT being roughly as useless for performance prediction as > SLIT is. That is not something I would want to see. > > Jonathan This patch is for a platform that is modelled and not for an actual board. So the latencies are hypothetical but ensure that the typical latency between two chips is represented. These tables are not expected to be copied for another platform without understanding what these values mean. Such a copy, without understanding what is being copied, would be an error in judgement on the part of the person doing it. The commit message of this patch (in v2) can be updated to exercise caution on reusing these values. [1]: https://developer.arm.com/products/system-design/fixed-virtual-platforms Best Regards, Vijayenthiran > > > > + > > > + // Memory Side Cache > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_INIT ( > > > + 0x0, SIZE_8MB, > > > + HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTRIBUTES_INIT ( > > > + 1, > > > + 1, > > > + 2, > > > + 2, > > > + 64 // 64 bytes cache line length > > > + ), > > > + 0), > > > + > > > + EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_INIT ( > > > + 0x1, SIZE_8MB, > > > + HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTRIBUTES_INIT ( > > > + 1, > > > + 1, > > > + 2, > > > + 2, > > > + 64 // 64 bytes cache line length > > > + ), > > > + 0), > > > +}; > > > + > > > +VOID* CONST ReferenceAcpiTable = &Hmat; > > > > > > > > > > > >