Re: [edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Ard,


On 19/10/2022 02:14 pm, Ard Biesheuvel wrote:
> On Wed, 19 Oct 2022 at 15:06, Sami Mujawar <sami.mujawar@arm.com> wrote:
>> Hi Ard,
>>
>> Please see my query inline marked [SAMI].
>>
>> Regards,
>>
>> Sami Mujawar
>>
>> On 19/10/2022 01:19 pm, Ard Biesheuvel via groups.io wrote:
>>> On Mon, 10 Oct 2022 at 12:13, Sunil V L <sunilvl@ventanamicro.com> wrote:
>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076
>>>>
>>>> This is copied from ArmVirtPkg since it is required for
>>>> other architectures also.
>>>>
>>>> It also adds the instance for single flash drive which has
>>>> both code and variables. This is copied from SbsaQemu.
>>>>
>>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>> Cc: Daniel Schaefer <git@danielschaefer.me>
>>>> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
>>> Let's call these
>>>
>>> QemuNorFlashPlatformLib [for the library class]
>>>
>>> QemuNorFlashDeviceTreeLib
>>> QemuNorFlashStaticLib
>>>
>>> and for the driver
>>>
>>> QemuNorFlashDxe
>> [SAMI] We use the NorFlashDxe for the Kvmtool guest firmware, see
>> https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/ArmVirtKvmTool.dsc#L294
>>
>> Considering this, should QemuNorFlashDxe be called OvmfNorFlashDxe?
>>
>> [/SAMI]
>>
> Ah yes, good point. So using Qemu as a prefix is slightly problematic.
>
> My intent is for this driver to be optimized towards NOR flash
> emulation the way QEMU implements it. The main difference between the
> platforms is that some of them (notably, ArmVirtQemu.dsc) also execute
> from the emulated region, which requires an executable (read-only)
> memslot in KVM, as instruction fetches (as opposed to explicit loads
> and stores) cannot be emulated by KVM. KvmTool only uses the NOR flash
> for variables, so it doesn't really care how the emulation is
> implemented, as long as the loads and stores are carried out in the
> expected way.
>
> I don't think the Ovmf prefix makes sense here either - OVMF is still
> primarily x86, which uses a different flash emulation altogether.
> Maybe VirtNorFlashDxe, to emphasize that it does not expect to be
> dealing with actual NOR flash?

VirtNorFlashDxe sounds good to me.

Regards,

Sami Mujawar

>
> Suggestions welcome :-)