From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by mx.groups.io with SMTP id smtpd.web10.109481.1597924932042749121 for ; Thu, 20 Aug 2020 05:02:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=mGHvPJ70; spf=pass (domain: nuviainc.com, ip: 209.85.128.67, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f67.google.com with SMTP id d190so1340385wmd.4 for ; Thu, 20 Aug 2020 05:02:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=CWgOA/azIp84VWSm92xMNkoXOtD5Hq/6obtHHPoGkTg=; b=mGHvPJ7054Feh13A1yzNrpfUZriSzZFxa7HfqRJEWYEMTfqfdYsoRqCb41bi8WXHjQ VmJj/hyZUKVKR4rF62AKWiFaSsXexklG/8vqQtCsfLnMQ4SWZ0wNAFlglTbtcqKWwOWb WztVuJDrkDSSkiqvvZyIAMBmwZ5yzrHcOJrqT5LzTYYs3qt+YjF/Ae3atPgF0N6M2Scz lAJcmaneIHL4pVX/+X9TAz5loxXTJa50q8ZzaYFLjjGzFTDgIl2gwSS3YNvxnGlcjJQi Vb6t0b+pkUsC5ih2ZTC/EUJA0X6X9OlqyenSXAUQtLlrGKCOfK/2BlpNvGhN3MFgvg1J bcLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=CWgOA/azIp84VWSm92xMNkoXOtD5Hq/6obtHHPoGkTg=; b=VkAuZ2MCW1X6yg2UAWWD9dkBDlyxM1ZLZPcAPWFXegkVQHsYQAjYTFZtN8/Kj6jxkP rWziDt2l7/bCHQGI4aySG2wyCWC0BStHJa4k8IIAIyqbfZ/qxKjo1sCklPs6oWOY02hJ 0UPvNC+G1guMsNR4mYxSboU2PjzRszg/OGNVhMQH6FLnWLscijL5iP/2zYf7tFqdpXhj wcVgTnJNVYo6Sir+zIqaPObibUE1bqPi7euFxxNC9cIaI7QtAnp7qL70mOapLP0aH+SR 1oHakL9R9Tzj1weLzIP5aGk5k+WiYno+aDfVUYtvmut17q8yfgsq3gFLPnUcr/dp7w2P Z1Tw== X-Gm-Message-State: AOAM532jQiYjNt7+ZLrN3xh5ATeTBf2MM1JuhMkO25uDk6UtvHVGwni+ gNYPjpH6oZBdT72SyjJHflO0/A== X-Google-Smtp-Source: ABdhPJxD1BuU7HutmHD7BZYNwijTIovZWtU7k69Vs0Z4nUDqXiuv2Sup36M7eODG9Sf504nzzTTiMg== X-Received: by 2002:a7b:c954:: with SMTP id i20mr3392926wml.189.1597924929455; Thu, 20 Aug 2020 05:02:09 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id g7sm3858065wrv.82.2020.08.20.05.02.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Aug 2020 05:02:09 -0700 (PDT) Date: Thu, 20 Aug 2020 13:02:07 +0100 From: "Leif Lindholm" To: Tanmay Jagdale Cc: graeme@nuviainc.com, shashi.mallela@linaro.org, devel@edk2.groups.io, paul.isaacs@linaro.org, tanmay@marvell.com Subject: Re: [PATCH edk2-platforms 4/7] SbsaQemu: AcpiDxe: Create MADT table at runtime Message-ID: <20200820120207.GE1191@vanye> References: <20200819143005.13999-1-tanmay.jagdale@linaro.org> <20200819143005.13999-5-tanmay.jagdale@linaro.org> MIME-Version: 1.0 In-Reply-To: <20200819143005.13999-5-tanmay.jagdale@linaro.org> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Aug 19, 2020 at 20:00:02 +0530, Tanmay Jagdale wrote: > - Add support to create MADT table at runtime. > - Included a macro for GIC Redistributor structure initialisation. > > Signed-off-by: Tanmay Jagdale > --- > .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 159 ++++++++++++++++++ > .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 20 ++- > .../Include/IndustryStandard/SbsaQemuAcpi.h | 15 ++ > 3 files changed, 193 insertions(+), 1 deletion(-) > > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > index 09e5ba432a59..569cda8b6474 100644 > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > @@ -6,11 +6,19 @@ > * SPDX-License-Identifier: BSD-2-Clause-Patent > * > **/ > +#include > +#include > +#include Looking at Acpi.h, that contains the line #include Looking at Acpi63.h, that contains the line #include Looking at Acpi62.h, that contains the line #include Looking at Acpi61.h, that contains the line #include ... Looking at Acpi20.h, that contains the line #include I.e. - there may be a point to including a specific versioned ACPI header, to make it clear what minimum revision is required, but there is no need to include multiple ACPI headers. For SbsaQemu, I would prefer not tying us to a specific revision - we will be tracking architectural evolution. So only including Acpi.h is probably appropriate. > +#include > +#include > +#include > #include > +#include > #include > #include > #include > #include > +#include > #include > #include > > @@ -62,6 +70,138 @@ CountCpusFromFdt ( > ASSERT_RETURN_ERROR (PcdStatus); > } > > +/* > + * A Function to Compute the ACPI Table Checksum > + */ > +VOID > +AcpiPlatformChecksum ( > + IN UINT8 *Buffer, > + IN UINTN Size > + ) > +{ > + UINTN ChecksumOffset; > + > + ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum); > + > + // Set checksum to 0 first This comment is completely pointless ... *unless* the checksum byte is also included in the checksum calculation. And if that is the case, *that* is what should be pointed out. Not the arithmetic operation that can be clearly seen happening on the next line. > + Buffer[ChecksumOffset] = 0; > + > + // Update checksum value > + Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size); (Or, if the checksum byte can be relied on to be the last one in Buffer, we can calculate the sum over Size - 1.) Note to self: this is very clearly something that should go into a generic helper library. / Leif > +} > + > +/* > + * A function that add the MADT ACPI table. > + IN EFI_ACPI_COMMON_HEADER *CurrentTable > + */ > +EFI_STATUS > +AddMadtTable ( > + IN EFI_ACPI_TABLE_PROTOCOL *AcpiTable > + ) > +{ > + EFI_STATUS Status; > + UINTN TableHandle; > + UINT32 TableSize; > + EFI_PHYSICAL_ADDRESS PageAddress; > + UINT8 *New; > + UINT32 NumCores; > + > + // Initialize MADT ACPI Header > + EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header = { > + SBSAQEMU_ACPI_HEADER (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, > + EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER, > + EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION), > + 0, 0 }; > + > + // Initialize GICC Structure > + EFI_ACPI_6_0_GIC_STRUCTURE Gicc = EFI_ACPI_6_0_GICC_STRUCTURE_INIT ( > + 0, /* GicID */ > + 0, /* AcpiCpuUid */ > + 0, /* Mpidr */ > + EFI_ACPI_6_0_GIC_ENABLED, /* Flags */ > + SBSAQEMU_MADT_GIC_PMU_IRQ, /* PMU Irq */ > + FixedPcdGet32 (PcdGicDistributorBase), /* PhysicalBaseAddress */ > + SBSAQEMU_MADT_GIC_VBASE, /* GicVBase */ > + SBSAQEMU_MADT_GIC_HBASE, /* GicHBase */ > + 25, /* GsivId */ > + 0, /* GicRBase */ > + 0 /* Efficiency */ > + ); > + > + // Initialize GIC Distributor Structure > + EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE Gicd = > + EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT ( > + 0, > + FixedPcdGet32 (PcdGicDistributorBase), > + 0, > + 3 /* GicVersion */ > + ); > + > + // Initialize GIC Redistributor Structure > + EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT(); > + > + // Get CoreCount which was determined eariler after parsing device tree > + NumCores = PcdGet32 (PcdCoreCount); > + > + // Calculate the new table size based on the number of cores > + TableSize = sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) + > + (sizeof (EFI_ACPI_6_0_GIC_STRUCTURE) * NumCores) + > + sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE) + > + sizeof (EFI_ACPI_6_0_GICR_STRUCTURE); > + > + Status = gBS->AllocatePages ( > + AllocateAnyPages, > + EfiACPIReclaimMemory, > + EFI_SIZE_TO_PAGES (TableSize), > + &PageAddress > + ); > + if (EFI_ERROR(Status)) { > + DEBUG ((EFI_D_ERROR, "Failed to allocate pages for MADT 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_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER)); > + ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize; > + New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER); > + > + // Add new GICC structures for the Cores > + for (NumCores = 0; NumCores < PcdGet32 (PcdCoreCount); NumCores++) { > + EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr; > + > + CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE)); > + GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New; > + GiccPtr->AcpiProcessorUid = NumCores; > + GiccPtr->MPIDR = NumCores; > + New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE); > + } > + > + // GIC Distributor Structure > + CopyMem (New, &Gicd, sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE)); > + New += sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE); > + > + // GIC ReDistributor Structure > + CopyMem (New, &Gicr, sizeof (EFI_ACPI_6_0_GICR_STRUCTURE)); > + New += sizeof (EFI_ACPI_6_0_GICR_STRUCTURE); > + > + AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize); > + > + Status = AcpiTable->InstallAcpiTable ( > + AcpiTable, > + (EFI_ACPI_COMMON_HEADER *)PageAddress, > + TableSize, > + &TableHandle > + ); > + if (EFI_ERROR(Status)) { > + DEBUG ((EFI_D_ERROR, "Failed to install MADT table\n")); > + } > + > + return Status; > +} > + > EFI_STATUS > EFIAPI > InitializeSbsaQemuAcpiDxe ( > @@ -69,8 +209,27 @@ InitializeSbsaQemuAcpiDxe ( > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > + EFI_STATUS Status; > + EFI_ACPI_TABLE_PROTOCOL *AcpiTable; > + > // Parse the device tree and get the number of CPUs > CountCpusFromFdt (); > > + // Check if ACPI Table Protocol has been installed > + Status = gBS->LocateProtocol ( > + &gEfiAcpiTableProtocolGuid, > + NULL, > + (VOID **)&AcpiTable > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "Failed to locate ACPI Table Protocol\n")); > + return Status; > + } > + > + Status = AddMadtTable (AcpiTable); > + if (EFI_ERROR(Status)) { > + DEBUG((EFI_D_ERROR, "Failed to add MADT table\n")); > + } > + > return EFI_SUCCESS; > } > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > index efc4d295bfb7..16bc1b0c8cb1 100644 > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > @@ -41,9 +41,27 @@ [LibraryClasses] > UefiRuntimeServicesTableLib > > [Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile > gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount > gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount > gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress > > [Depex] > - TRUE > + gEfiAcpiTableProtocolGuid ## CONSUMES > + > +[Guids] > + gEdkiiPlatformHasAcpiGuid > + > +[Protocols] > + gEfiAcpiTableProtocolGuid ## CONSUMES > + > +[FixedPcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision > + gArmTokenSpaceGuid.PcdGicDistributorBase > + gArmTokenSpaceGuid.PcdGicRedistributorsBase > + > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision > diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h > index eac195b0585c..7a9a0061675f 100644 > --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h > +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h > @@ -22,6 +22,21 @@ > FixedPcdGet32 (PcdAcpiDefaultCreatorRevision)/* UINT32 CreatorRevision */ \ > } > > +// Defines for MADT > +#define SBSAQEMU_MADT_GIC_VBASE 0x2c020000 > +#define SBSAQEMU_MADT_GIC_HBASE 0x2c010000 > +#define SBSAQEMU_MADT_GIC_PMU_IRQ 23 > +#define SBSAQEMU_MADT_GICR_SIZE 0x4000000 > + > +// Macro for MADT GIC Redistributor Structure > +#define SBSAQEMU_MADT_GICR_INIT() { \ > + EFI_ACPI_6_0_GICR, /* Type */ \ > + sizeof (EFI_ACPI_6_0_GICR_STRUCTURE), /* Length */ \ > + EFI_ACPI_RESERVED_WORD, /* Reserved */ \ > + FixedPcdGet32 (PcdGicRedistributorsBase), /* DiscoveryRangeBaseAddress */ \ > + SBSAQEMU_MADT_GICR_SIZE /* DiscoveryRangeLength */ \ > + } > + > #define SBSAQEMU_UART0_BASE 0x60000000 > > #define SBSAQEMU_PCI_SEG0_CONFIG_BASE 0xf0000000 > -- > 2.28.0 >