From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web10.14916.1598456126595791731 for ; Wed, 26 Aug 2020 08:35:27 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=o4qhv5wl; spf=pass (domain: nuviainc.com, ip: 209.85.221.66, mailfrom: graeme@nuviainc.com) Received: by mail-wr1-f66.google.com with SMTP id r15so2223593wrp.13 for ; Wed, 26 Aug 2020 08:35:26 -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; bh=HUXujl6y2pFWSTlV7DWh2P2eehJ84ouYKWNF2LwjTr4=; b=o4qhv5wl/gAAlUDkZ2F9SO7Mfk1NJGKOLjhVVbTQydDcnhmVlS5krldHrkMi8xpJFz OqH95ZR3F/ufZ1Irw908FGAFLxicZFZd5H2vywi3S+gxG49/6ojfiN1DCm0G2+GyqBM6 4hTXK2yhcDfwcSk0sPR4RcSidZgzj4d5kKRboqXxcKbZeeyn3Nhp0j/ozUq8foHp/msd 40zdmESWveq7j8f0nPkj9qaGwy8u6XzJ3BVN/D+nbkcymQxUOkQoWlkn4lxGvpFk6w0R jNsuXx4RiQl5NtsjdGY9dMK3RKdSw5h2qGUBU9ztXezneGCR804FVr0kFcwP0CUy2Vm+ AhBQ== 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; bh=HUXujl6y2pFWSTlV7DWh2P2eehJ84ouYKWNF2LwjTr4=; b=WIkiqR/QBVwa0KeVYkQrBy7f+2MF4PU7fXx+FW2i9uZZZpB1VCIkhWftEQejEuM01F djLYEnAd62D3tmRCphvGxE8LwsAr5jVoxfTBr1rz+A4R0D2i9A92TDFMLhZsvIm5Rqcd Efrx2qaeVOH6qhwQS2cUAWgdbccqmTiSf9KnXLPnfQsI8OlnoWjlcnYeRfCbVcaN2+HD 4Vofptw2jrxTDC3CyBbwce6KOZCjboMYOXUUUc1FQwfye4bfkCtnLVgc7Yi6hVW64Znw dkJj8ZZJof5kNST2VdUK/LDI7GqqD8O4GIZc6MwwCLbBy1HgPYAxPUyNWtNTiD32rzjf YB6w== X-Gm-Message-State: AOAM532Re0xpr0XYu2ihsgHGk0Jt778AnAjXlrqGvb0gonJl0PuWxFBa siIIDySIrrK+YEW4xm0tM5fUTg== X-Google-Smtp-Source: ABdhPJyuJGrdaLIN185rxp/E+qN/lUiUmzLfVki/T9GgAqMBBda//Vs/0f69lr26OSGwYkuiqRPfKg== X-Received: by 2002:adf:f086:: with SMTP id n6mr12812080wro.208.1598456124926; Wed, 26 Aug 2020 08:35:24 -0700 (PDT) Return-Path: Received: from xora-monster ([2a02:8010:64d6::1d89]) by smtp.gmail.com with ESMTPSA id 128sm6554700wmz.43.2020.08.26.08.35.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Aug 2020 08:35:24 -0700 (PDT) Date: Wed, 26 Aug 2020 16:35:22 +0100 From: graeme@nuviainc.com To: Tanmay Jagdale Cc: leif@nuviainc.com, devel@edk2.groups.io, shashi.mallela@linaro.org Subject: Re: [PATCH v3 edk2-platforms 5/8] SbsaQemu: AcpiDxe: Create MADT table at runtime Message-ID: <20200826153522.pmsodj7a5r4pz6jf@xora-monster> References: <20200825133958.17372-1-tanmay.jagdale@linaro.org> <20200825133958.17372-6-tanmay.jagdale@linaro.org> MIME-Version: 1.0 In-Reply-To: <20200825133958.17372-6-tanmay.jagdale@linaro.org> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Aug 25, 2020 at 07:09:55PM +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 > --- > Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 20 ++- > Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h | 15 ++ > Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 156 ++++++++++++++++++++ > 3 files changed, 190 insertions(+), 1 deletion(-) > > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > index 3795a7e11639..8125e8ba7553 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 > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > index 75abdae3b8ce..16cb4e904e6f 100644 > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > @@ -6,11 +6,17 @@ > * SPDX-License-Identifier: BSD-2-Clause-Patent > * > **/ > +#include > +#include > +#include > +#include > #include > +#include > #include > #include > #include > #include > +#include > #include > #include > > @@ -61,6 +67,137 @@ 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 field to 0 since it is used as part of the calculation > + Buffer[ChecksumOffset] = 0; > + > + Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size); > +} > + > +/* > + * 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 ((DEBUG_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; This does not seem to be quite correct, if I dump the MPIDRs from ARM-TF when booting with 12 cpus this is what I get. NOTICE: MPIDR 0 NOTICE: MPIDR 1 NOTICE: MPIDR 2 NOTICE: MPIDR 3 NOTICE: MPIDR 4 NOTICE: MPIDR 5 NOTICE: MPIDR 6 NOTICE: MPIDR 7 NOTICE: MPIDR 100 NOTICE: MPIDR 101 NOTICE: MPIDR 102 NOTICE: MPIDR 103 I think this will make PSCI operations from CPU8 onwards fail. Thanks Graeme > + 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 ((DEBUG_ERROR, "Failed to install MADT table\n")); > + } > + > + return Status; > +} > + > EFI_STATUS > EFIAPI > InitializeSbsaQemuAcpiDxe ( > @@ -68,8 +205,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 ((DEBUG_ERROR, "Failed to locate ACPI Table Protocol\n")); > + return Status; > + } > + > + Status = AddMadtTable (AcpiTable); > + if (EFI_ERROR(Status)) { > + DEBUG ((DEBUG_ERROR, "Failed to add MADT table\n")); > + } > + > return EFI_SUCCESS; > } > -- > 2.28.0 >