From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.8170.1593083961205736905 for ; Thu, 25 Jun 2020 04:19:21 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) 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 0EE821FB; Thu, 25 Jun 2020 04:19:20 -0700 (PDT) Received: from [192.168.43.142] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3699D3F73C; Thu, 25 Jun 2020 04:19:18 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v3 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib To: devel@edk2.groups.io, philmd@redhat.com, Sami Mujawar Cc: 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: <20200624133458.61920-1-sami.mujawar@arm.com> <20200624133458.61920-7-sami.mujawar@arm.com> From: "Ard Biesheuvel" Message-ID: Date: Thu, 25 Jun 2020 13:19:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 6/25/20 10:45 AM, Philippe Mathieu-Daud=C3=A9 via groups.io wrote: > Hi Sami, >=20 > On 6/24/20 3:34 PM, 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. >> >> UEFI takes ownership of the CFI flash hardware, and >> exposes its functionality through the UEFI Runtime >> Variable Service. Therefore, disable the device tree >> node for the CFI flash used for storing the UEFI >> variables, to prevent the OS from attaching its device >> driver as well. >> >> Signed-off-by: Sami Mujawar >> Acked-by: Laszlo Ersek >> --- >> >> Notes: >> v3: >> - ASSERT is sufficient to test Locating [Ar= d] >> gFdtClientProtocolGuid as DEPEX ensures that this is >> guaranteed to succeed. >> - Removed additional error handling based on review [Sa= mi] >> feedback. >> - Fix confusion caused by use of macro MAX_FLASH_BANKS. [Ph= ilippe] >> - Renamed MAX_FLASH_BANKS to MAX_FLASH_DEVICES. [Sa= mi] >> - Use macro to define block size for flash. [Ph= ilippe] >> - Defined macro KVMTOOL_NOR_BLOCK_SIZE and also configured [Sa= mi] >> to reflect the correct block size 64KB. >> - Disable the DT flash node used for UEFI variable storage [Sa= mi] >> as UEFI takes ownership of the flash device. >> Ref: https://edk2.groups.io/g/devel/topic/74200914#60341 >> =20 >> v2: >> - Library to read CFI flash base address from DT and initialise= [Sami] >> PCDs used for NOR flash variables. >> >> ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c | 330 += +++++++++++++++++++ >> ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf | 49 += ++ >> 2 files changed, 379 insertions(+) >> >> diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c b= /ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c >> new file mode 100644 >> index 0000000000000000000000000000000000000000..8e9dcf31691b4b12b9c7ba= c1ad4ba8d3a534a1d8 >> --- /dev/null >> +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c >> @@ -0,0 +1,330 @@ >> +/** @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 NOR block size configured in Kvmtool. >> +*/ >> +#define KVMTOOL_NOR_BLOCK_SIZE SIZE_64KB >> + >> +/** Macro defining the maximum number of Flash devices. >> +*/ >> +#define MAX_FLASH_DEVICES 4 >=20 > I am sorry but I am still confused... >=20 > This is about the QEMU Virt machine, right? >=20 > This machine was supposed to have 1 single flash, see QEMU commit > f5fdcd6e58 ("hw/arm: Add 'virt' platform") from Nov 2013: >=20 > /* Addresses and sizes of our components. > * 0..128MB is space for a flash device so we can run bootrom code > such as UEFI. > ... >=20 > Due to limitations in the QEMU cfi-flash model, instead of using > a single flash device (with proper sector/bank protection), two > devices were added in QEMU commit acf82361c6 ("hw/arm/virt: Provide > flash devices for boot ROMs") Sep 2014: >=20 > Add two flash devices to the virt board, so that it can be used fo= r > running guests which want a bootrom image such as UEFI. We provide > two flash devices to make it more convenient to provide both a > read-only UEFI image and a read-write place to store guest-set > UEFI config variables. The '-bios' command line option is set up > to provide an image for the first of the two flash devices. >=20 > What do you declare maximum 4 devices? >=20 Kvmtool !=3D QEMU, so whatever QEMU does is irrelevant for this series.