Re: [PATCH v4 5/7] OvmfPkg/RiscVVirt: Add VirtNorFlashDxe to APRIORI list

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

On Mon, Jul 03, 2023 at 11:45:45PM -0700, Tuan Phan wrote:
> As i said, VirtNorFlashDxe needed to be loaded before VariableRuntimeDxe so
> your suggestion will not work.
> 
Okay, at least for me, by removing APRIORI patch and adding this depex,
edk2 boots fine with your series. I am not sure what won't work.

Hi Ard, any thoughts? If no better way, may be we have to use APRIORI.

Thanks,
Sunil
> On Mon, Jul 3, 2023 at 10:07 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > On Wed, Jun 28, 2023 at 02:27:10PM -0700, Tuan Phan wrote:
> > > On Wed, Jun 28, 2023 at 9:47 AM Sunil V L <sunilvl@ventanamicro.com>
> > wrote:
> > >
> > > > On Fri, Jun 23, 2023 at 11:39:32AM -0700, Tuan Phan wrote:
> > > > > Make sure VirtNorFlashDxe loaded before VariableRuntimeDxe as it
> > > > > is the backend flash driver.
> > > > >
> > > > > Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> > > > > ---
> > > > >  OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf | 10 ++++++++++
> > > > >  1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
> > > > b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
> > > > > index 21e4ba67379f..9ab8eb3ba7d8 100644
> > > > > --- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
> > > > > +++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
> > > > > @@ -53,6 +53,16 @@ READ_STATUS        = TRUE
> > > > >  READ_LOCK_CAP      = TRUE
> > > > >  READ_LOCK_STATUS   = TRUE
> > > > >
> > > > > +APRIORI DXE {
> > > > > +  INF  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> > > > > +  INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> > > > > +  INF
> > > >
> > MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
> > > > > +  INF
> > > >
> > MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
> > > > > +  INF  EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
> > > > > +  INF  UefiCpuPkg/CpuDxeRiscV64/CpuDxeRiscV64.inf
> > > > > +  INF  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> > > > > +}
> > > > > +
> > > > Hi Tuan,
> > > >
> > > > Actually, Ard had recommended not to use APRIORI and hence we avoided
> > > > it when we upstreamed RiscVVirt. So, I am wondering whether this can be
> > > > avoided by using depex in CpuDxe on gEfiVariableArchProtocolGuid?
> > > >
> > > > Hi Sunil,
> > > Not sure what the reason behind avoiding APRIORI besides it is a
> > workaround
> > > for broken DEPEX. BTW, what we need is to put VirtNorFlashDxe loaded
> > before
> > > VariableRuntimeDxe which doesn't depend on any modules. I don't see any
> > > other clearer way than modifying VirNorFlashDxe as shown in the first
> > > version of this series.
> > >
> > > The CpuDxeRiscV64 in the aprioriy list as VirNorFlashDxe depends on it.
> > >
> > Hi Tuan,
> >
> > I couldn't locate old mail from Ard recommending to remove APRIORI in
> > RISC-V. But here is the recent mail on different context but those
> > reasons are still valid in any case.
> > https://edk2.groups.io/g/devel/message/104543
> >
> > IMO, there is no dependency between VirtNorFlashDxe and
> > VariableRuntimeDxe. I think what we need is CpuDxeRiscV64 loaded after
> > VariableRuntimeDxe and before VirtNorFlashDxe. A simple depex like I
> > suggested in previous mail should work. I still prefer this than
> > introducing APRIORI unless there are other issues I am now aware of.
> > What do you think?
> >
> > Thanks!
> > Sunil
> >