Re: [edk2-staging/RiscV64QemuVirt PATCH 26/29] OvmfPkg: Add generic Qemu NOR flash DXE driver

["Sunil V L" <sunilvl@ventanamicro.com>]

On Mon, Oct 10, 2022 at 05:29:15PM +0200, Ard Biesheuvel wrote:
> On Mon, 10 Oct 2022 at 17:19, Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > On Mon, Oct 10, 2022 at 12:39:21PM +0200, Ard Biesheuvel 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
> > > >
> > > > RISC-V needs NorFlashDxe driver for qemu virt machine. The
> > > > ArmPlatformPkg has this driver but migrating it to generic
> > > > package like MdeModulePkg introduces circular dependencies.
> > > > So, add a simplified version of the NorFlashDxe driver in
> > > > OvmfPkg.
> > > >
> > >
> > > So what is the difference between this simplified version and the old
> > > version? And it is backed in QEMU by the same NOR flash emulation,
> > > shouldn't we use this driver for ArmVirtQemu as well?
> >
> > I agree. If we can break the dependency on EmbeddedPkg due to
> > NvVarStoreFormattedLib, then we can migrate to MdeModulePkg and all
> > consumers can use the same driver which would be the best solution IMO.
> >
> > Could you please let me know  why NvVarStoreFormattedLib is added
> > in EmbeddedPkg instead of MdePkg or MdeModulePkg? Is it only for
> > non-server class platforms? I don't see it doing much so not sure
> > its use case.
> >
> 
> I think that library as well as the definition of
> gEdkiiNvVarStoreFormattedGuid should be moved to MdeModulePkg.
> 
> Then, we can look at moving NorFlashDxe in there as well.

Great. Let me rework these patches then.
> 
> But you haven't answered my question regarding how your version was simplified.
> 

Oh sorry. I forgot to modify the commit message but what I really meant
was removing the code which depends upon PcdNorFlashCheckBlockLocked for
virtual platforms. I thought it is useful only for real platforms and
qemu doesn't emulate it. Let me know if I am wrong.
Since the driver is copied for OVMF, I didn't want to add the PCD unnecessarily.

> Note that there is some room for improvement in that driver in
> relation to execution under KVM: switching between programming mode
> and array mode involves setting up/tearing down the KVM memslot, and
> currently, the driver is far from optimized when it comes to
> minimizing the number of transitions between read mode and write mode.

I was not looking at optimizing it at this point of time.

Thanks
Sunil