From: "Leif Lindholm" <quic_llindhol@quicinc.com>
To: <devel@edk2.groups.io>, <xiongyining1480@phytium.com.cn>
Cc: <ardb+tianocore@kernel.org>, <graeme@xora.org.uk>,
<marcin.juszkiewicz@linaro.org>, <chenbaozi@phytium.com.cn>
Subject: Re: [edk2-devel] [PATCH v3 1/1] SbsaQemu: AcpiDxe: Create SRAT table at runtime
Date: Wed, 27 Mar 2024 14:55:24 +0000 [thread overview]
Message-ID: <ZgQzXOxpoz5UudEg@qc-i7.hemma.eciton.net> (raw)
In-Reply-To: <20240327135934.3802327-2-xiongyining1480@phytium.com.cn>
On Wed, Mar 27, 2024 at 13:59:34 +0000, Xiong Yining wrote:
> Add support to create SRAT(System resource affinity table) for
> sbsa platform at runtime.
>
> Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
No one can sign off patches on behalf of someone else. Please only
include your own signed-off-by.
> Reviewed-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> Change-Id: I4a65084e6ade66d87ea935adfa4be15a7030d3eb
We don't use gerrit. Please drop Change-Id:s before upstreaming.
> ---
> .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h | 27 ++++++
> .../Include/Library/HardwareInfoLib.h | 10 ++
> .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 92 +++++++++++++++++++
> .../SbsaQemuHardwareInfoLib.c | 36 ++++++++
> 4 files changed, 165 insertions(+)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> index 7595df4c8a2d..83a085cd86f4 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> @@ -63,4 +63,31 @@ typedef struct {
>
> #define GTDT_WDTIMER_FLAGS (GTDT_WDTIMER_ACTIVE_HIGH | GTDT_WDTIMER_LEVEL_TRIGGERED)
>
> +#define SBSAQEMU_ACPI_MEMORY_AFFINITY_STRUCTURE_INIT( \
> + ProximityDomain, Base, Length, Flags) \
> + { \
> + 1, /* Type */ \
> + sizeof (EFI_ACPI_6_4_MEMORY_AFFINITY_STRUCTURE), /* Length */ \
> + ProximityDomain, /* Proximity Domain */ \
> + 0, /* Reserved */ \
> + (Base) & 0xffffffff, /* Base Address Low */ \
> + ((Base) >> 32) & 0xffffffff , /* Base Address High */ \
> + (Length) & 0xffffffff, /* Length Low */ \
> + ((Length) >> 32) & 0xffffffff, /* Length High */ \
> + 0, /* Reserved */ \
> + Flags, /* Flags */ \
> + 0 /* Reserved */ \
> + }
> +
> +#define SBSAQEMU_ACPI_GICC_AFFINITY_STRUCTURE_INIT( \
> + ProximityDomain, ACPIProcessorUID, Flags, ClockDomain) \
> + { \
> + 3, /* Type */ \
> + sizeof (EFI_ACPI_6_4_GICC_AFFINITY_STRUCTURE), /* Length */ \
> + ProximityDomain, /* Proximity Domain */ \
> + ACPIProcessorUID, /* ACPI Processor UID */ \
> + Flags, /* Flags */ \
> + ClockDomain /* Clock Domain */ \
> + }
> +
> #endif
> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
> index 5db0eacc9d2d..46fdad45353c 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
> @@ -73,4 +73,14 @@ GetMemInfo (
> OUT MemoryInfo *MemInfo
> );
>
> +/**
> + Get the number of numa node from device tree passed by Qemu.
> +
> + @retval the number of numa node.
> +**/
> +UINT64
> +GetNumaNodeCount (
> + VOID
> + );
> +
> #endif /* HARDWARE_INFO_LIB */
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index 4ebe2a445344..30239e7dca0d 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -682,6 +682,91 @@ AddGtdtTable (
> return Status;
> }
>
> +/*
> + * A function that adds the SRAT ACPI table.
> + */
> +EFI_STATUS
> +AddSratTable (
> + IN EFI_ACPI_TABLE_PROTOCOL *AcpiTable
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 *New;
> + EFI_PHYSICAL_ADDRESS PageAddress;
> + UINTN TableHandle;
> + UINT32 TableSize;
> + UINT32 Index;
> + UINT32 NodeId;
> + UINT32 NumMemNodes;
> + MemoryInfo MemInfo;
> + UINT32 NumCores = GetCpuCount ();
> +
> + // Initialize SRAT ACPI Header
> + EFI_ACPI_6_4_SYSTEM_RESOURCE_AFFINITY_TABLE_HEADER Header = {
> + SBSAQEMU_ACPI_HEADER (EFI_ACPI_6_4_SYSTEM_RESOURCE_AFFINITY_TABLE_SIGNATURE,
> + EFI_ACPI_6_4_SYSTEM_RESOURCE_AFFINITY_TABLE_HEADER,
> + EFI_ACPI_6_4_SYSTEM_RESOURCE_AFFINITY_TABLE_REVISION),
> + 1, 0 };
> +
> + NumMemNodes = GetMemNodeCount();
> +
> + // Calculate the new table size based on the number of cores
> + TableSize = sizeof (EFI_ACPI_6_4_SYSTEM_RESOURCE_AFFINITY_TABLE_HEADER) +
> + (sizeof (EFI_ACPI_6_4_MEMORY_AFFINITY_STRUCTURE) * NumMemNodes ) +
> + (sizeof (EFI_ACPI_6_4_GICC_AFFINITY_STRUCTURE) * NumCores);
> +
> + Status = gBS->AllocatePages (
> + AllocateAnyPages,
> + EfiACPIReclaimMemory,
> + EFI_SIZE_TO_PAGES (TableSize),
> + &PageAddress
> + );
> +
> + if (EFI_ERROR(Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to allocate pages for SRAT table\n"));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + New = (UINT8 *)(UINTN) PageAddress;
> + ZeroMem (New, TableSize);
> +
> + // Add the ACPI Description table header
> + CopyMem (New, &Header, sizeof (EFI_ACPI_6_4_SYSTEM_RESOURCE_AFFINITY_TABLE_HEADER));
> + ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
> + New += sizeof (EFI_ACPI_6_4_SYSTEM_RESOURCE_AFFINITY_TABLE_HEADER);
> +
> + // Add memory structures
> + for (Index = 0; Index < NumMemNodes ; Index++) {
> + GetMemInfo (Index, &MemInfo);
> + EFI_ACPI_6_4_MEMORY_AFFINITY_STRUCTURE memory = SBSAQEMU_ACPI_MEMORY_AFFINITY_STRUCTURE_INIT (MemInfo.NodeId, MemInfo.AddressBase, MemInfo.AddressSize, 1);
> + CopyMem (New, &memory, sizeof (EFI_ACPI_6_4_MEMORY_AFFINITY_STRUCTURE));
> + New += sizeof (EFI_ACPI_6_4_MEMORY_AFFINITY_STRUCTURE);
> + }
> +
> + // Add processor structures for the cores
> + for (Index = 0; Index < NumCores; Index++) {
> + NodeId = GetCpuNumaNode(Index);
> + EFI_ACPI_6_4_GICC_AFFINITY_STRUCTURE gicc = SBSAQEMU_ACPI_GICC_AFFINITY_STRUCTURE_INIT(NodeId, Index, 1, 0);
> + CopyMem (New, &gicc, sizeof (EFI_ACPI_6_4_GICC_AFFINITY_STRUCTURE));
> + New += sizeof (EFI_ACPI_6_4_GICC_AFFINITY_STRUCTURE);
> + }
> +
> + // Perform Checksum
> + AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize);
> +
> + Status = AcpiTable->InstallAcpiTable (
> + AcpiTable,
> + (EFI_ACPI_COMMON_HEADER *)PageAddress,
> + TableSize,
> + &TableHandle
> + );
> + if (EFI_ERROR(Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to install SRAT table\n"));
> + }
> +
> + return Status;
> +}
> +
> /*
> * A function to disable XHCI node on Platform Version lower than 0.3
> */
> @@ -793,6 +878,13 @@ InitializeSbsaQemuAcpiDxe (
> DEBUG ((DEBUG_ERROR, "Failed to add PPTT table\n"));
> }
>
> + if (GetNumaNodeCount() > 1){
> + Status = AddSratTable (AcpiTable);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to add SRAT table\n"));
> + }
> + }
> +
> Status = AddGtdtTable (AcpiTable);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n"));
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> index b178cf6c6c43..ebd49b3a0762 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> @@ -143,3 +143,39 @@ GetMemInfo (
> MemInfo->AddressBase + MemInfo->AddressSize -1 ));
>
> }
> +
> +UINT64
> +GetNumaNodeCount (
> + VOID
> +)
> +{
> + UINT64 Arg;
> + UINT32 Index;
> + UINT32 NumNumaCount;
> + UINT32 NumMemCount;
I don't understand these variable names.
Please rename them to represent, without abbreviations, what they
hold. And "Num", which should be written as "Number" or "NumberOf" is
effectively a synonym of "Count", so pick one.
"Numa" is a well understood abbreviation, no need to expand that :)
/
Leif
> + UINT32 NumCores = GetCpuCount();
> + MemoryInfo MemInfo;
> +
> + NumNumaCount = 0;
> + NumMemCount = GetMemNodeCount();
> +
> + if (NumCores > 0){
> + for (Index = 0; Index < NumCores; Index ++){
> + Arg = GetCpuNumaNode(Index);
> + if (NumNumaCount == 0 || NumNumaCount < (Arg + 1)){
> + NumNumaCount = Arg + 1;
> + }
> + }
> + }
> +
> + if (NumMemCount > 0){
> + for (Index = 0; Index < NumMemCount; Index ++){
> + GetMemInfo(Index, &MemInfo);
> + if (NumNumaCount == 0 || NumNumaCount < (MemInfo.NodeId + 1)){
> + NumNumaCount = MemInfo.NodeId + 1;
> + }
> + }
> + }
> +
> + return NumNumaCount;
> +}
> --
> 2.34.1
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117175): https://edk2.groups.io/g/devel/message/117175
Mute This Topic: https://groups.io/mt/105177558/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
prev parent reply other threads:[~2024-03-27 14:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-27 13:59 [edk2-devel] [PATCH v3 0/1] Add support for generating SRAT tables Xiong Yining
2024-03-27 13:59 ` [edk2-devel] [PATCH v3 1/1] SbsaQemu: AcpiDxe: Create SRAT table at runtime Xiong Yining
2024-03-27 14:55 ` Leif Lindholm [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZgQzXOxpoz5UudEg@qc-i7.hemma.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox