Re: [PATCH] MdeModulePkg/PciBusDxe: Fix a bug in ProcessOptionRomLight

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>


> -----Original Message-----
> From: Park, Aiden <aiden.park@intel.com>
> Sent: Friday, January 29, 2021 9:55 AM
> To: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>
> Subject: RE: [PATCH] MdeModulePkg/PciBusDxe: Fix a bug in ProcessOptionRomLight
> 
> Hello,
> 
> Can you please review this?
> 
> > -----Original Message-----
> > From: Park, Aiden <aiden.park@intel.com>
> > Sent: Wednesday, January 13, 2021 11:01 AM
> > To: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > devel@edk2.groups.io
> > Cc: Park, Aiden <aiden.park@intel.com>; Ma, Maurice
> > <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>
> > Subject: [PATCH] MdeModulePkg/PciBusDxe: Fix a bug in
> > ProcessOptionRomLight
> >
> > From: Aiden Park <aiden.park@intel.com>
> >
> > The ProcessOptionRomLight() assumes that OpRom has already been processed
> > in the previous full enumeration and updates AllOpRomProcessed flag to TRUE
> > by default. However, this may not be applicable with other pre-stage boot
> > firmwares.
> >
> > This will update AllOpRomProcessed flag properly by checking
> > PciRomGetImageMapping().
> >
> > Signed-off-by: Aiden Park <aiden.park@intel.com>
> > ---
> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > index 1b64924b7b..e8337e865e 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > @@ -1168,12 +1168,7 @@ ProcessOptionRomLight (
> >        ProcessOptionRomLight (Temp);
> >      }
> >
> > -    PciRomGetImageMapping (Temp);
> > -
> > -    //
> > -    // The OpRom has already been processed in the first round
> > -    //
> > -    Temp->AllOpRomProcessed = TRUE;
> > +    Temp->AllOpRomProcessed = PciRomGetImageMapping (Temp);
> >
> >      CurrentLink = CurrentLink->ForwardLink;
> >    }
> > --
> > 2.20.1
> 
> Best Regards,
> Aiden