From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id CAA247803CF for ; Tue, 21 May 2024 09:24:29 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=iqa2uZNvZbr3NSf5fABwDEN3NJVkp9KSUGXrgRKkCqU=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20240206; t=1716283468; v=1; b=G+ReHtRmxou51EZMmTLAgov9OYrrGfbqkVOd6Mu7+FComBwBHnJY+o2WfxMSDEUoNr2kz2Fl LtzSXoEnTzfEXWxtSDBe3TgvT969xbocsC+znlzu9SS7+KSeKg3zyMjDEEDS8ABKtJPaKGXVD3b lB4ZBg3xsoTjSzvEc0A7+gXpb9K90pxibunSWJuRrVhmYjrCH6O7jcbGaE0e9+gGgifi/GTy1zd C+0yq9RhQCkOfnr6tNNGcRncvWEB9FTeRfgFuNdk3yHQBDskkKIbOlMC6+JN2ASu56/xIHmss0W QMYBvhXCIXN0vGkYJHBF3nuXlT18H7iby4jpKOpWSD+oA== X-Received: by 127.0.0.2 with SMTP id HX1qYY7687511xDMZKU5wC9v; Tue, 21 May 2024 02:24:28 -0700 X-Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.15420.1716283467514984261 for ; Tue, 21 May 2024 02:24:27 -0700 X-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 2E535106F for ; Tue, 21 May 2024 02:24:51 -0700 (PDT) X-Received: from mail-pl1-f182.google.com (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1BA043FB9E for ; Tue, 21 May 2024 02:24:27 -0700 (PDT) X-Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-1eecc71311eso102098545ad.3 for ; Tue, 21 May 2024 02:24:27 -0700 (PDT) X-Gm-Message-State: LvMhKro2SmadZ3uuIB5sH75ex7686176AA= X-Google-Smtp-Source: AGHT+IHQz6hBPnNrgxmQ5DaSqhMzdsVHfjybfDRRaDdEgYrolE/Q9l+e+GLRN2cleBu0JVWBrFserEDhFihUINM/bVs= X-Received: by 2002:a17:90b:710:b0:2a5:7188:9590 with SMTP id 98e67ed59e1d1-2b6cc879befmr33570351a91.22.1716283466358; Tue, 21 May 2024 02:24:26 -0700 (PDT) MIME-Version: 1.0 References: <20240423055638.1271531-1-Sahil.Kaushal@arm.com> <20240423055638.1271531-12-Sahil.Kaushal@arm.com> <5c52b3a6-be91-497a-bea2-551188399602@arm.com> In-Reply-To: <5c52b3a6-be91-497a-bea2-551188399602@arm.com> From: "sahil" Date: Tue, 21 May 2024 14:54:14 +0530 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 11/14] Silicon/ARM/NeoverseN1Soc: NOR flash library for N1Sdp To: devel@edk2.groups.io, sami.mujawar@arm.com Cc: Sahil Kaushal , Ard Biesheuvel , Leif Lindholm , "nd@arm.com" Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Tue, 21 May 2024 02:24:27 -0700 Resent-From: sahil@arm.com Reply-To: devel@edk2.groups.io,sahil@arm.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=G+ReHtRm; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=arm.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io Hi Sami, Thank you for reviewing the patches. Please find my comments inline below marked as [SAHIL]. On Thu, 16 May 2024 at 20:54, Sami Mujawar via groups.io wrote: > > Hi Sahil, > > Thank you for this patch. > > I have some suggestions marked inline below, otherwise this patch looks good to me. > > With that fixed, > > Reviewed-by: Sami Mujawar > > Regards, > > Sami Mujawar > > On 23/04/2024 06:56 am, Sahil Kaushal wrote: > > From: sahil > > Add NOR flash library, this library provides APIs for getting the list > of NOR flash devices on the platform. > > [SAMI] I think the information in the commit message of patch 10/14 would be more useful here. > > Not mandatory, but it may be useful to have an ASCII diagram to explain the flash partitioning. > > [/SAMI] [SAHIL] I will add more information to the commit message. As for the ASCII diagram, as the size of the images can change, it will not be possible to make an accurate diagram. [/SAHIL] > > Signed-off-by: sahil > --- > Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf | 34 ++++++++++ > Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c | 65 ++++++++++++++++++++ > 2 files changed, 99 insertions(+) > > diff --git a/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf > new file mode 100644 > index 000000000000..fad3bca79d3a > --- /dev/null > +++ b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf > @@ -0,0 +1,34 @@ > +## @file > > +# NOR flash lib for ARM Neoverse N1 platform. > > +# > > +# Copyright (c) 2024, ARM Limited. All rights reserved.
> > +# > > +# SPDX-License-Identifier: BSD-2-Clause-Patent > > +# > > +## > > + > > +[Defines] > > + INF_VERSION = 0x0001001B > > + BASE_NAME = NorFlashNeoverseN1SocLib > > + FILE_GUID = 7006fcf1-a585-4272-92e3-b286b1dff5bb > > + MODULE_TYPE = DXE_DRIVER > > + VERSION_STRING = 1.0 > > + LIBRARY_CLASS = NorFlashPlatformLib > > + > > +[Sources.common] > > + NorFlashLib.c > > + > > +[Packages] > > + MdeModulePkg/MdeModulePkg.dec > > + MdePkg/MdePkg.dec > > + Platform/ARM/ARM.dec > > + Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec > > + > > +[LibraryClasses] > > + BaseLib > > + > > +[FixedPcd] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > > diff --git a/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c > new file mode 100644 > index 000000000000..a48db9c74548 > --- /dev/null > +++ b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c > @@ -0,0 +1,65 @@ > +/** @file > > +* NOR flash lib for ARM Neoverse N1 platform > > +* > > +* Copyright (c) 2024, ARM Limited. All rights reserved.
> > +* > > +* SPDX-License-Identifier: BSD-2-Clause-Patent > > +* > > +**/ > > + > > +#include > > +#include > > +#include > > + > > +#define FW_ENV_REGION_BASE FixedPcdGet32 (PcdFlashNvStorageVariableBase) > > +#define FW_ENV_REGION_SIZE (FixedPcdGet32 (PcdFlashNvStorageVariableSize) + \ > > + FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) + \ > > + FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize)) > > [SAMI] Would it be an issue if someone were to increase the storage variable sizes above? > > How can you prevent someone overwriting the flash region used by the SCP? > > Would it make sense to add a check in NorFlashPlatformInitialization() ? > > [/SAMI] [SAHIL] With the current code, the firmware images take ~2.5MB and the variable storage region starts at 15MB offset from the base. The firmware image's size is unlikely to change much. Therefore, it is unlikely to cause any issue if someone tries to increase the variable region by decreasing the PcdFlashNvStorageVariableBase. If we still want to add a check, I can add an assert checking PcdFlashNvStorageVariableBase offset is not <= to a particular offset (maybe 8MB) in NorFlashPlatformInitialization(). Would this be fine? [/SAHIL] > > + > > +STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = { > > + { > > + /// Environment variable region > > + NEOVERSEN1SOC_SCP_QSPI_AHB_BASE, ///< device base > > + FW_ENV_REGION_BASE, ///< region base > > + FW_ENV_REGION_SIZE, ///< region size > > + SIZE_4KB, ///< block size > > + }, > > +}; > > + > > +/** > > + Dummy implementation of NorFlashPlatformInitialization to > > + comply with NorFlashPlatformLib structure. > > + > > + @retval EFI_SUCCESS Success. > > +**/ > > +EFI_STATUS > > +NorFlashPlatformInitialization ( > > + VOID > > + ) > > +{ > > + return EFI_SUCCESS; > > +} > > + > > +/** > > + Get NOR flash region info > > + > > + @param[out] NorFlashDevices NOR flash regions info. > > + @param[out] Count number of flash instance. > > + > > + @retval EFI_SUCCESS Success. > > + @retval EFI_INVALID_PARAMETER The parameters specified are not valid. > > +**/ > > +EFI_STATUS > > +NorFlashPlatformGetDevices ( > > + OUT NOR_FLASH_DESCRIPTION **NorFlashDevices, > > + OUT UINT32 *Count > > + ) > > +{ > > + if ((NorFlashDevices == NULL) || (Count == NULL)) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + *NorFlashDevices = mNorFlashDevices; > > + *Count = ARRAY_SIZE (mNorFlashDevices); > > + return EFI_SUCCESS; > > +} > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119099): https://edk2.groups.io/g/devel/message/119099 Mute This Topic: https://groups.io/mt/105690946/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-