From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::242; helo=mail-io0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x242.google.com (mail-io0-x242.google.com [IPv6:2607:f8b0:4001:c06::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9512E210F2CF8 for ; Fri, 15 Jun 2018 06:03:01 -0700 (PDT) Received: by mail-io0-x242.google.com with SMTP id f1-v6so10634490ioh.6 for ; Fri, 15 Jun 2018 06:03:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=ecsOWgJTvuf5xcfcfSRkEYEq2iFj5T+H81IZHrEfsZI=; b=HzihIhaLVozzO245yk4tfXarZrDoGu1xDHcOpgTBA7uQEdaphf+xwN0Znat7S6ANs6 I7VOxTWW71RGzAOcCke3DhvwZ3DIFnF9D3wkrVy6FPY71aarWV53nsOw7FMamMZQhrfA UqTjMJXiwWWz/lrE4ik+tDxe8f5xwZRerapgM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ecsOWgJTvuf5xcfcfSRkEYEq2iFj5T+H81IZHrEfsZI=; b=bGn06H5n2h6yhN0L70MghwsVen3AWvbQDlMg0orze65ReHOaBSpTdkAu4iTbg03Kfo +9oZM0LdQC11eFPM+qep76V/98baSZ34PJ4laasWZ2YDZVTrxCkFDZml90nu2brWyLb8 zfw+9s7+hbqRRW6ULNeeL8nW86QrxP2xc5/CPZdrMNuW+AW1b27s/4NqVT7TeeQNTRUN ajza2IOkGTBgWdZ2ozNjIlCoaqEN3GTUptQcZbwP3cr0oJ2bSYEmZp5ycoAJC9DkAhnP T7bI22TG1J2pnl+E4Hdww2GDou74B0FJoBg0zmPt18dlvPdUzX4UB06EQ5uXHpyq8724 dbFw== X-Gm-Message-State: APt69E3/gQ3ZIzyMid9lDlsETsc9smkdiT3DScA2XNV+Ds3Ef65v9NWz uWap85Nod2mJBwwE1MkSH+J59LjscC+5tofk5ohwww== X-Google-Smtp-Source: ADUXVKJuIAqounDuBwBBiIy6gPjspt+oi1mq/+ZZ9+dIr475pWOABq9NOKUD62G5bNuJ2qzo911ZxEwa5Qq6FxVKa/I= X-Received: by 2002:a6b:6709:: with SMTP id b9-v6mr1362997ioc.170.1529067780326; Fri, 15 Jun 2018 06:03:00 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Fri, 15 Jun 2018 06:02:59 -0700 (PDT) In-Reply-To: <1529064002-18779-2-git-send-email-chandni.cherukuri@arm.com> References: <1529064002-18779-1-git-send-email-chandni.cherukuri@arm.com> <1529064002-18779-2-git-send-email-chandni.cherukuri@arm.com> From: Ard Biesheuvel Date: Fri, 15 Jun 2018 15:02:59 +0200 Message-ID: To: Chandni Cherukuri Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH edk2-platforms v2 1/2] Platform/ARM/Sgi: Install a Platform ID HOB X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Jun 2018 13:03:01 -0000 Content-Type: text/plain; charset="UTF-8" On 15 June 2018 at 14:00, Chandni Cherukuri wrote: > On SGI platforms, the trusted firmware executes prior to > the SEC phase. It supplies to the SEC phase a pointer to > a fdt, that contains platform specific information such as > part number and config number of the SGI platform. The > platform code that executes during the SEC phase creates a > PPI that allows access to other PEIMs to obtain a copy of the > fdt pointer. > > Further, during the PEI phase, a Platform ID PEIM installs a > Platform ID HOB. The Platform ID HOB can be used by DXE > drivers to identify the platform it is executing on. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Chandni Cherukuri > --- > Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h | 23 ++++ > Platform/ARM/SgiPkg/Include/SgiPlatform.h | 13 +++ > Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S | 13 ++- > Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 10 ++ > Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 + > Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf | 40 +++++++ > Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c | 112 ++++++++++++++++++++ > Platform/ARM/SgiPkg/SgiPlatform.dec | 5 + > 8 files changed, 215 insertions(+), 4 deletions(-) > > diff --git a/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h b/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h > new file mode 100644 > index 0000000..5ffc69d > --- /dev/null > +++ b/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h > @@ -0,0 +1,23 @@ > +/** @file > +* > +* Copyright (c) 2018, ARM 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. > +* > +**/ > + > +#ifndef SGI_PLATFORMID_PPI_ > +#define SGI_PLATFORMID_PPI_ > + > +//HwConfig DT structure > +typedef struct { > + UINT64 *HwConfigDtAddr; > +} SGI_HW_CONFIG_INFO_PPI; > + > +#endif > diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h > index 00ca7e9..1454018 100644 > --- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h > +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h > @@ -68,4 +68,17 @@ > #define SGI_SYSPH_SYS_REG_FLASH 0x4C > #define SGI_SYSPH_SYS_REG_FLASH_RWEN 0x1 > > +// SGI575_VERSION values > +#define SGI575_CONF_NUM 0x3 > +#define SGI575_PART_NUM 0x783 > + > +#define SGI_CONFIG_MASK 0x0F > +#define SGI_CONFIG_SHIFT 0x1C > +#define SGI_PART_NUM_MASK 0xFFF > + > +// ARM platform description data. > +typedef struct { > + UINTN PlatformId; > +} SGI_PLATFORM_DESCRIPTOR; > + > #endif // __SGI_PLATFORM_H__ > diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S > index dab6c77..3662266 100644 > --- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S > +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S > @@ -22,15 +22,20 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction) > GCC_ASM_EXPORT(ArmPlatformGetCorePosition) > GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId) > GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore) > +GCC_ASM_IMPORT(HwConfigDtBlob) > > // > // First platform specific function to be called in the PEI phase > // > -// This function is actually the first function called by the PrePi > -// or PrePeiCore modules. It allows to retrieve arguments passed to > -// the UEFI firmware through the CPU registers. > -// > +// The trusted firmware passed the hw config DT blob in x1 register. > +// Keep a copy of it in a global variable. > +//VOID > +//ArmPlatformPeiBootAction ( > +// VOID > +// ); > ASM_PFX(ArmPlatformPeiBootAction): > + adr x10, HwConfigDtBlob > + str x1, [x10] > ret > > //UINTN > diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c > index ea3201a..c9ddbfb 100644 > --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c > +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c > @@ -15,6 +15,10 @@ > #include > #include > #include > +#include > + > +UINT64 HwConfigDtBlob; > +STATIC SGI_HW_CONFIG_INFO_PPI mHwConfigDtInfoPpi; > > STATIC ARM_CORE_INFO mCoreInfoTable[] = { > { > @@ -36,6 +40,7 @@ ArmPlatformInitialize ( > IN UINTN MpId > ) > { > + mHwConfigDtInfoPpi.HwConfigDtAddr = &HwConfigDtBlob; > return RETURN_SUCCESS; > } > > @@ -59,6 +64,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = { > EFI_PEI_PPI_DESCRIPTOR_PPI, > &gArmMpCoreInfoPpiGuid, > &mMpCoreInfoPpi > + }, > + { > + EFI_PEI_PPI_DESCRIPTOR_PPI, > + &gHwConfigDtInfoPpiGuid, > + &mHwConfigDtInfoPpi > } > }; > > diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf > index 42e14d5..5d4bf72 100644 > --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf > +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf > @@ -61,7 +61,10 @@ > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress > > [Guids] > + gArmSgiPlatformIdDescriptorGuid > gEfiHobListGuid ## CONSUMES ## SystemTable > + gFdtTableGuid > > [Ppis] > gArmMpCoreInfoPpiGuid > + gHwConfigDtInfoPpiGuid > diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf > new file mode 100644 > index 0000000..3d1bab7 > --- /dev/null > +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf > @@ -0,0 +1,40 @@ > +# > +# Copyright (c) 2018, ARM 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. > +# > + > +[Defines] > + INF_VERSION = 0x0001001A > + BASE_NAME = SgiPlatformIdPei > + FILE_GUID = a9936f3e-6a1b-11e8-8ce0-fffe69b86863 > + MODULE_TYPE = PEIM > + VERSION_STRING = 1.0 > + ENTRY_POINT = SgiPlatformPeim > + > +[Packages] > + EmbeddedPkg/EmbeddedPkg.dec > + MdePkg/MdePkg.dec > + Platform/ARM/SgiPkg/SgiPlatform.dec > + > +[LibraryClasses] > + FdtLib > + PeimEntryPoint > + > +[Sources] > + SgiPlatformPeim.c > + > +[Guids] > + gArmSgiPlatformIdDescriptorGuid > + > +[Ppis] > + gHwConfigDtInfoPpiGuid > + > +[Depex] > + TRUE This driver should depex on gHwConfigDtInfoPpiGuid, given that it will complain if it does not find it. It seems a bit convoluted to install a PPI first, and then invoke it to populate the HOB. OTOH, other PEIMs may need the platform ID as well, so I'm fine with keeping it. > diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c > new file mode 100644 > index 0000000..77499a6 > --- /dev/null > +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c > @@ -0,0 +1,112 @@ > +/** @file > +* > +* Copyright (c) 2018, ARM 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. > +* > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/** > + > + This function returns the Platform ID of the platform > + > + This functions locates the HwConfig PPI and gets the base address of HW CONFIG > + DT from which the platform ID is obtained using FDT helper functions > + > + @return returns the platform ID on success else returns 0 on error > + > +**/ > + > +STATIC > +UINT32 > +GetSgiPlatformId ( > + VOID > +) > +{ > + CONST UINT32 *Property; > + INT32 Offset; > + CONST VOID *HwCfgDtBlob; > + SGI_HW_CONFIG_INFO_PPI *HwConfigInfoPpi; > + EFI_STATUS Status; > + > + Status = PeiServicesLocatePpi (&gHwConfigDtInfoPpiGuid, 0, NULL, > + (VOID**)&HwConfigInfoPpi); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, > + "PeiServicesLocatePpi failed with error %r\n", Status)); > + return 0; > + } > + > + HwCfgDtBlob = (VOID *)(*(HwConfigInfoPpi->HwConfigDtAddr)); This need an intermediate (UINTN) cast, or instead, you can use a UINTN* for the address. In fact, why are you using a pointer here rather than the address of the DT itself? > + if (fdt_check_header (HwCfgDtBlob) != 0) { > + DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed\n", HwCfgDtBlob)); > + return 0; > + } > + > + Offset = fdt_subnode_offset (HwCfgDtBlob, 0, "system-id"); > + if (Offset == -FDT_ERR_NOTFOUND) { > + DEBUG ((DEBUG_ERROR, "Invalid DTB : system-id node not found\n")); > + return 0; > + } > + > + Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL); > + if (Property == NULL) { > + DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n")); > + return 0; > + } > + > + return fdt32_to_cpu (*Property); > +} > + > +/** > + > + This function creates a Platform ID HOB and assigns PlatformId as the > + HobData > + > + @return asserts on error and returns EFI_INVALID_PARAMETER. On success > + returns EFI_SUCCESS > + > +**/ > +EFI_STATUS > +EFIAPI > +SgiPlatformPeim ( > + IN EFI_PEI_FILE_HANDLE FileHandle, > + IN CONST EFI_PEI_SERVICES **PeiServices > +) > +{ > + SGI_PLATFORM_DESCRIPTOR *HobData; > + > + //Create platform descriptor HOB > + HobData = (SGI_PLATFORM_DESCRIPTOR *)BuildGuidHob ( > + &gArmSgiPlatformIdDescriptorGuid, > + sizeof (SGI_PLATFORM_DESCRIPTOR)); > + > + //Get the platform id from the platform specific hw_config device tree > + if (HobData == NULL) { > + DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n")); > + ASSERT (FALSE); > + return EFI_OUT_OF_RESOURCES; > + } > + > + HobData->PlatformId = GetSgiPlatformId (); > + if (HobData->PlatformId == 0) { > + ASSERT (FALSE); > + return EFI_INVALID_PARAMETER; > + } > + > + return EFI_SUCCESS; > +} > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec > index d995937..ea9f6c5 100644 > --- a/Platform/ARM/SgiPkg/SgiPlatform.dec > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec > @@ -29,9 +29,14 @@ > Include # Root include for the package > > [Guids.common] > + # ARM Sgi Platform ID descriptor > + gArmSgiPlatformIdDescriptorGuid = { 0xf56f152a, 0xad2a, 0x11e6, { 0xb1, 0xa7, 0x00, 0x50, 0x56, 0x3c, 0x44, 0xcc } } > gArmSgiTokenSpaceGuid = { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } } > gSgi575AcpiTablesiFileGuid = { 0xc712719a, 0x0aaf, 0x438c, { 0x9c, 0xdd, 0x35, 0xab, 0x4d, 0x60, 0x20, 0x7d } } > > [PcdsFeatureFlag.common] > # Set this PCD to TRUE to enable virtio support. > gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x00000001 > + > +[Ppis] > + gHwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } } > -- > 2.7.4 >