Re: [edk2-devel] [PATCH v3 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 6/25/20 10:45 AM, Philippe Mathieu-Daudé via groups.io wrote:
> Hi Sami,
> 
> 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 <sami.mujawar@arm.com>
>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>      v3:
>>        - ASSERT is sufficient to test Locating                     [Ard]
>>          gFdtClientProtocolGuid as DEPEX ensures that this is
>>          guaranteed to succeed.
>>        - Removed additional error handling based on review         [Sami]
>>          feedback.
>>        - Fix confusion caused by use of macro MAX_FLASH_BANKS.     [Philippe]
>>        - Renamed MAX_FLASH_BANKS to MAX_FLASH_DEVICES.             [Sami]
>>        - Use macro to define block size for flash.                 [Philippe]
>>        - Defined macro KVMTOOL_NOR_BLOCK_SIZE and also configured  [Sami]
>>          to reflect the correct block size 64KB.
>>        - Disable the DT flash node used for UEFI variable storage  [Sami]
>>          as UEFI takes ownership of the flash device.
>>          Ref: https://edk2.groups.io/g/devel/topic/74200914#60341
>>      
>>      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..8e9dcf31691b4b12b9c7bac1ad4ba8d3a534a1d8
>> --- /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.<BR>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> + **/
>> +
>> +#include <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/NorFlashPlatformLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Protocol/FdtClient.h>
>> +
>> +/** 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
> 
> I am sorry but I am still confused...
> 
> This is about the QEMU Virt machine, right?
> 
> This machine was supposed to have 1 single flash, see QEMU commit
> f5fdcd6e58 ("hw/arm: Add 'virt' platform") from Nov 2013:
> 
>    /* Addresses and sizes of our components.
>     * 0..128MB is space for a flash device so we can run bootrom code
> such as UEFI.
>     ...
> 
> 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:
> 
>      Add two flash devices to the virt board, so that it can be used for
>      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.
> 
> What do you declare maximum 4 devices?
> 

Kvmtool != QEMU, so whatever QEMU does is irrelevant for this series.