From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web12.4538.1590580801416653393 for ; Wed, 27 May 2020 05:00:01 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=AT4nCpSI; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: philmd@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1590580800; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:autocrypt:autocrypt; bh=FzHoMl4wJ8KWTM23k/wfvDrDF1gRZNSo8BZMC67E9gA=; b=AT4nCpSIRrvSd1scEIlV3qnD+Wby2w1LYJxLrZQjQ1gzX+8Po7QzlPHUV1Kal+fGxAacU6 UnFR6K+5Kd2wB39EjGms8DmB26oUwjjNquzl8wP7gc1ArWy0CvEQARg12ulTnL9UOy16ih bvRFueUvYGyXc0bR5zci8eAgA05nAQs= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-193-Jgs_iuX2MvunLl1evEp6iw-1; Wed, 27 May 2020 07:59:58 -0400 X-MC-Unique: Jgs_iuX2MvunLl1evEp6iw-1 Received: by mail-ed1-f72.google.com with SMTP id bo26so9955761edb.22 for ; Wed, 27 May 2020 04:59:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=FzHoMl4wJ8KWTM23k/wfvDrDF1gRZNSo8BZMC67E9gA=; b=dhdBR0bwKM3ptCn/5v2V5zYbsTunQcm5QT9bm/HVGHQfIhKfo36Vh2yrvV2zMDsrNY QbylY+TBjiarpRzwCHyG63RNHUt5UOERRY1lqZEgG0nunHEGFoa18PY51TIsT9VsfqUd EDK/RJwWKoEcdSK8jEoXSnY41xJ18zvKknlU0ER2YTciBfxbLUG8Id+iWZnROA1vvz+Y crBZa0qe/rM2EIiLcW1ouzNG9eeN9iytZFMD+3zdMfj+YTRvDkUL9xJsLGMqpgr2NWUr MhN/+K+/VjJlAKQP0oZLAJ9c8zoeiDaF8fPZH3uatzWLkCK9AYrkiIkxFN6e5swriShk tbXA== X-Gm-Message-State: AOAM533LZugg/iv5wRdo+nNM6nPzesOpKR9n073deePpJuea+X5y1GoN N4bEd7+yyTROrW7DnmhGE2jv8dl94T17OD835jHXFMNQD4yeINi+YAPZiOEvpirIgLfV0JT0KFm uNqHveNxew5EeGQ== X-Received: by 2002:a05:6402:3cd:: with SMTP id t13mr24326370edw.285.1590580795941; Wed, 27 May 2020 04:59:55 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx4fpr60mQF0iSWpavrlTWUmfHpHK+GqjiYUF0GNI1kWJGfoDaYDhmCOavEIVR4cKcaZvsbMg== X-Received: by 2002:a05:6402:3cd:: with SMTP id t13mr24326326edw.285.1590580795500; Wed, 27 May 2020 04:59:55 -0700 (PDT) Return-Path: Received: from [192.168.1.36] (71.red-88-21-204.staticip.rima-tde.net. [88.21.204.71]) by smtp.gmail.com with ESMTPSA id b11sm2774977ejl.3.2020.05.27.04.59.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 27 May 2020 04:59:54 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v1 08/11] ArmVirtPkg: Add Kvmtool NOR flash lib To: devel@edk2.groups.io, sami.mujawar@arm.com Cc: ard.biesheuvel@arm.com, leif@nuviainc.com, lersek@redhat.com, Alexandru.Elisei@arm.com, Andre.Przywara@arm.com, Matteo.Carlini@arm.com, Laura.Moretta@arm.com, nd@arm.com References: <20200514084019.71368-1-sami.mujawar@arm.com> <20200514084019.71368-9-sami.mujawar@arm.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Autocrypt: addr=philmd@redhat.com; keydata= mQINBDXML8YBEADXCtUkDBKQvNsQA7sDpw6YLE/1tKHwm24A1au9Hfy/OFmkpzo+MD+dYc+7 bvnqWAeGweq2SDq8zbzFZ1gJBd6+e5v1a/UrTxvwBk51yEkadrpRbi+r2bDpTJwXc/uEtYAB GvsTZMtiQVA4kRID1KCdgLa3zztPLCj5H1VZhqZsiGvXa/nMIlhvacRXdbgllPPJ72cLUkXf z1Zu4AkEKpccZaJspmLWGSzGu6UTZ7UfVeR2Hcc2KI9oZB1qthmZ1+PZyGZ/Dy+z+zklC0xl XIpQPmnfy9+/1hj1LzJ+pe3HzEodtlVA+rdttSvA6nmHKIt8Ul6b/h1DFTmUT1lN1WbAGxmg CH1O26cz5nTrzdjoqC/b8PpZiT0kO5MKKgiu5S4PRIxW2+RA4H9nq7nztNZ1Y39bDpzwE5Sp bDHzd5owmLxMLZAINtCtQuRbSOcMjZlg4zohA9TQP9krGIk+qTR+H4CV22sWldSkVtsoTaA2 qNeSJhfHQY0TyQvFbqRsSNIe2gTDzzEQ8itsmdHHE/yzhcCVvlUzXhAT6pIN0OT+cdsTTfif MIcDboys92auTuJ7U+4jWF1+WUaJ8gDL69ThAsu7mGDBbm80P3vvUZ4fQM14NkxOnuGRrJxO qjWNJ2ZUxgyHAh5TCxMLKWZoL5hpnvx3dF3Ti9HW2dsUUWICSQARAQABtDJQaGlsaXBwZSBN YXRoaWV1LURhdWTDqSAoUGhpbCkgPHBoaWxtZEByZWRoYXQuY29tPokCVQQTAQgAPwIbDwYL CQgHAwIGFQgCCQoLBBYCAwECHgECF4AWIQSJweePYB7obIZ0lcuio/1u3q3A3gUCXsfWwAUJ KtymWgAKCRCio/1u3q3A3ircD/9Vjh3aFNJ3uF3hddeoFg1H038wZr/xi8/rX27M1Vj2j9VH 0B8Olp4KUQw/hyO6kUxqkoojmzRpmzvlpZ0cUiZJo2bQIWnvScyHxFCv33kHe+YEIqoJlaQc JfKYlbCoubz+02E2A6bFD9+BvCY0LBbEj5POwyKGiDMjHKCGuzSuDRbCn0Mz4kCa7nFMF5Jv piC+JemRdiBd6102ThqgIsyGEBXuf1sy0QIVyXgaqr9O2b/0VoXpQId7yY7OJuYYxs7kQoXI 6WzSMpmuXGkmfxOgbc/L6YbzB0JOriX0iRClxu4dEUg8Bs2pNnr6huY2Ft+qb41RzCJvvMyu gS32LfN0bTZ6Qm2A8ayMtUQgnwZDSO23OKgQWZVglGliY3ezHZ6lVwC24Vjkmq/2yBSLakZE 6DZUjZzCW1nvtRK05ebyK6tofRsx8xB8pL/kcBb9nCuh70aLR+5cmE41X4O+MVJbwfP5s/RW 9BFSL3qgXuXso/3XuWTQjJJGgKhB6xXjMmb1J4q/h5IuVV4juv1Fem9sfmyrh+Wi5V1IzKI7 RPJ3KVb937eBgSENk53P0gUorwzUcO+ASEo3Z1cBKkJSPigDbeEjVfXQMzNt0oDRzpQqH2vp apo2jHnidWt8BsckuWZpxcZ9+/9obQ55DyVQHGiTN39hkETy3Emdnz1JVHTU0Q== Message-ID: <6fca2acc-f9f2-59cd-92a4-5276b30c0f91@redhat.com> Date: Wed, 27 May 2020 13:59:52 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200514084019.71368-9-sami.mujawar@arm.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Hi Sami, On 5/14/20 10:40 AM, Sami Mujawar wrote: > Kvmtool places the base address of the CFI flash in > the device tree it passes to UEFI. This library > parses the kvmtool device tree to read the CFI base > address and initialise the PCDs use by the NOR flash > driver and the variable storage. > > Signed-off-by: Sami Mujawar > --- > ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c | 265 ++++++++++++++++++++ > ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf | 50 ++++ > 2 files changed, 315 insertions(+) > > diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c > new file mode 100644 > index 0000000000000000000000000000000000000000..2e43c2e21bc9ef7dd1dd198eebbd70c3b0b96d1c > --- /dev/null > +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c > @@ -0,0 +1,265 @@ > +/** @file > + An instance of the NorFlashPlatformLib for Kvmtool platform. > + > + Copyright (c) 2020, ARM Ltd. All rights reserved.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > + **/ > + > +#include > +#include > +#include > +#include > +#include > + > +/** Macro defining the maximum number of Flash Banks. > + */ > +#define MAX_FLASH_BANKS 4 > + > +STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[MAX_FLASH_BANKS]; This is confuse, the macro define 4 banks to a flash device, but then you declare an array of 4 flash devices. I'm even more confused because I'm only aware of 2 devices on the Virt machine. What am I missing? > +STATIC UINTN mNorFlashDeviceCount = 0; > + > +/** This function performs platform specific actions to initialise > + the NOR flash, if required. > + > + @retval EFI_SUCCESS Success. > +**/ > +EFI_STATUS > +NorFlashPlatformInitialization ( > + VOID > + ) > +{ > + DEBUG ((DEBUG_INFO, "NorFlashPlatformInitialization\n")); > + // Nothing to do here > + return EFI_SUCCESS; > +} > + > +/** Initialise Non volatile Flash storage variables. > + > + @param [in] FlashDevice Pointer to the NOR Flash device. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_INVALID_PARAMETER A parameter is invalid. > + @retval EFI_OUT_OF_RESOURCES Insufficient flash storage space. > +**/ > +EFI_STATUS > +SetupVariableStore ( > + IN NOR_FLASH_DESCRIPTION * FlashDevice > + ) > +{ > + UINTN FlashRegion; > + UINTN FlashNvStorageVariableBase; > + UINTN FlashNvStorageFtwWorkingBase; > + UINTN FlashNvStorageFtwSpareBase; > + UINTN FlashNvStorageVariableSize; > + UINTN FlashNvStorageFtwWorkingSize; > + UINTN FlashNvStorageFtwSpareSize; > + > + FlashNvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize); > + FlashNvStorageFtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize); > + FlashNvStorageFtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize); > + > + if ((FlashNvStorageVariableSize == 0) || > + (FlashNvStorageFtwWorkingSize == 0) || > + (FlashNvStorageFtwSpareSize == 0)) { > + DEBUG ((DEBUG_ERROR, "FlashNvStorage size not defined\n")); > + return EFI_INVALID_PARAMETER; > + } > + > + // Setup the variable store > + FlashRegion = FlashDevice->DeviceBaseAddress; > + > + FlashNvStorageVariableBase = FlashRegion; > + FlashRegion += PcdGet32 (PcdFlashNvStorageVariableSize); > + > + FlashNvStorageFtwWorkingBase = FlashRegion; > + FlashRegion += PcdGet32 (PcdFlashNvStorageFtwWorkingSize); > + > + FlashNvStorageFtwSpareBase = FlashRegion; > + FlashRegion += PcdGet32 (PcdFlashNvStorageFtwSpareSize); > + > + if (FlashRegion > (FlashDevice->DeviceBaseAddress + FlashDevice->Size)) { > + DEBUG ((DEBUG_ERROR, "Insufficient flash storage size\n")); > + return EFI_OUT_OF_RESOURCES; > + } > + > + PcdSet32S ( > + PcdFlashNvStorageVariableBase, > + FlashNvStorageVariableBase > + ); > + > + PcdSet32S ( > + PcdFlashNvStorageFtwWorkingBase, > + FlashNvStorageFtwWorkingBase > + ); > + > + PcdSet32S ( > + PcdFlashNvStorageFtwSpareBase, > + FlashNvStorageFtwSpareBase > + ); > + > + DEBUG (( > + DEBUG_INFO, > + "PcdFlashNvStorageVariableBase = 0x%x\n", > + FlashNvStorageVariableBase > + )); > + DEBUG (( > + DEBUG_INFO, > + "PcdFlashNvStorageVariableSize = 0x%x\n", > + FlashNvStorageVariableSize > + )); > + DEBUG (( > + DEBUG_INFO, > + "PcdFlashNvStorageFtwWorkingBase = 0x%x\n", > + FlashNvStorageFtwWorkingBase > + )); > + DEBUG (( > + DEBUG_INFO, > + "PcdFlashNvStorageFtwWorkingSize = 0x%x\n", > + FlashNvStorageFtwWorkingSize > + )); > + DEBUG (( > + DEBUG_INFO, > + "PcdFlashNvStorageFtwSpareBase = 0x%x\n", > + FlashNvStorageFtwSpareBase > + )); > + DEBUG (( > + DEBUG_INFO, > + "PcdFlashNvStorageFtwSpareSize = 0x%x\n", > + FlashNvStorageFtwSpareSize > + )); > + > + return EFI_SUCCESS; > +} > + > +/** Return the Flash devices on the platform. > + > + @param [out] NorFlashDescriptions Pointer to the Flash device description. > + @param [out] Count Number of Flash devices. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_NOT_FOUND Flash device not found. > +**/ > +EFI_STATUS > +NorFlashPlatformGetDevices ( > + OUT NOR_FLASH_DESCRIPTION **NorFlashDescriptions, > + OUT UINT32 *Count > + ) > +{ > + if (mNorFlashDeviceCount > 0) { > + *NorFlashDescriptions = mNorFlashDevices; > + *Count = mNorFlashDeviceCount; > + return EFI_SUCCESS; > + } > + return EFI_NOT_FOUND; > +} > + > +/** Entrypoint for NorFlashPlatformLib. > + > + @param [in] ImageHandle The handle to the image. > + @param [in] SystemTable Pointer to the System Table. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_INVALID_PARAMETER A parameter is invalid. > + @retval EFI_NOT_FOUND Flash device not found. > +**/ > +EFI_STATUS > +EFIAPI > +NorFlashPlatformLibConstructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE * SystemTable > + ) > +{ > + FDT_CLIENT_PROTOCOL *FdtClient; > + INT32 Node; > + EFI_STATUS Status; > + EFI_STATUS FindNodeStatus; > + CONST UINT32 *Reg; > + UINT32 PropSize; > + UINT64 Base; > + UINT64 Size; > + > + if (mNorFlashDeviceCount != 0) { > + return EFI_SUCCESS; > + } > + > + Status = gBS->LocateProtocol ( > + &gFdtClientProtocolGuid, > + NULL, > + (VOID **)&FdtClient > + ); > + if (EFI_ERROR (Status)) { > + ASSERT_EFI_ERROR (Status); > + return Status; > + } > + > + for (FindNodeStatus = FdtClient->FindCompatibleNode ( > + FdtClient, > + "cfi-flash", > + &Node > + ); > + !EFI_ERROR (FindNodeStatus) && (mNorFlashDeviceCount < MAX_FLASH_BANKS); > + FindNodeStatus = FdtClient->FindNextCompatibleNode ( > + FdtClient, > + "cfi-flash", > + Node, > + &Node > + )) { > + Status = FdtClient->GetNodeProperty ( > + FdtClient, > + Node, > + "reg", > + (CONST VOID **)&Reg, > + &PropSize > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: GetNodeProperty () failed (Status == %r)\n", > + __FUNCTION__, Status)); > + continue; > + } > + > + ASSERT ((PropSize % (4 * sizeof (UINT32))) == 0); > + > + while ((PropSize >= (4 * sizeof (UINT32))) && > + (mNorFlashDeviceCount < MAX_FLASH_BANKS)) { > + Base = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0])); > + Size = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2])); > + Reg += 4; > + > + PropSize -= 4 * sizeof (UINT32); > + > + // > + // Disregard any flash devices that overlap with the primary FV. > + // The firmware is not updatable from inside the guest anyway. > + // > + if ((PcdGet64 (PcdFvBaseAddress) + PcdGet32 (PcdFvSize) > Base) && > + (Base + Size) > PcdGet64 (PcdFvBaseAddress)) { > + continue; > + } > + > + DEBUG (( > + DEBUG_INFO, > + "NOR%d : Base = 0x%lx, Size = 0x%lx\n", > + mNorFlashDeviceCount, > + Base, > + Size > + )); > + > + mNorFlashDevices[mNorFlashDeviceCount].DeviceBaseAddress = (UINTN)Base; > + mNorFlashDevices[mNorFlashDeviceCount].RegionBaseAddress = (UINTN)Base; > + mNorFlashDevices[mNorFlashDeviceCount].Size = (UINTN)Size; > + mNorFlashDevices[mNorFlashDeviceCount].BlockSize = SIZE_256KB; Hmm I'm worried that there is no contract enforcing the Virt machine to use 256KiB sectors. Can we add a definition elsewhere and use it here, instead of burying the fixed sector size here? > + mNorFlashDeviceCount++; > + } > + } > + > + // Setup the variable store in the last bank > + if ((mNorFlashDeviceCount > 0) && > + (mNorFlashDevices[mNorFlashDeviceCount - 1].DeviceBaseAddress != 0)) { > + return SetupVariableStore (&mNorFlashDevices[mNorFlashDeviceCount - 1]); > + } > + > + return EFI_NOT_FOUND; > +} > + > diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf > new file mode 100644 > index 0000000000000000000000000000000000000000..8bd6f730dcb52e597b418e59766c1566a9519789 > --- /dev/null > +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf > @@ -0,0 +1,50 @@ > +#/** @file > +# > +# Copyright (c) 2020, ARM Ltd. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +#**/ > + > +[Defines] > + INF_VERSION = 0x0001001B > + BASE_NAME = NorFlashKvmtoolLib > + FILE_GUID = E75F07A1-B160-4893-BDD4-09E32FF847DC > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = NorFlashPlatformLib > + CONSTRUCTOR = NorFlashPlatformLibConstructor > + > +[Sources.common] > + NorFlashKvmtool.c > + > +[Packages] > + ArmPkg/ArmPkg.dec > + ArmPlatformPkg/ArmPlatformPkg.dec > + ArmVirtPkg/ArmVirtPkg.dec > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + PcdLib > + UefiBootServicesTableLib > + > +[Protocols] > + gFdtClientProtocolGuid ## CONSUMES > + > +[Pcd] > + gArmTokenSpaceGuid.PcdFvBaseAddress > + gArmTokenSpaceGuid.PcdFvSize > + > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > + > + > +[Depex] > + gFdtClientProtocolGuid > + >