Re: [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf OptionROM override

["Zhu, Yonghong" <yonghong.zhu@intel.com>]

Hi Pilar,

Will you update a V2 to cover Jaben's comment ?

Best Regards,
Zhu Yonghong


-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Carsey, Jaben
Sent: Monday, October 8, 2018 11:00 PM
To: Gao, Liming <liming.gao@intel.com>; Tomas Pilar (tpilar) <tpilar@solarflare.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf OptionROM override



> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Gao, Liming
> Sent: Sunday, October 07, 2018 6:42 PM
> To: Tomas Pilar (tpilar) <tpilar@solarflare.com>; 
> edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in 
> Fdf OptionROM override
> 
> Pilar:
>   The change is good. Could you also update INF and FDF spec for this usage?
> If you don't know how to update INF and FDF spec, please submit BZ. I 
> will provide the spec patch.
> 
>   Reviewed-by: Liming Gao <liming.gao@intel.com>
> 
> Thanks
> Liming
> >-----Original Message-----
> >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf 
> >Of Tomas Pilar (tpilar)
> >Sent: Tuesday, October 02, 2018 10:46 PM
> >To: edk2-devel@lists.01.org
> >Subject: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf 
> >OptionROM override
> >
> >Contributed-under: TianoCore Contribution Agreement 1.1
> >Signed-off-by: Tomas Pilar <tpilar@solarflare.com>
> >---
> > BaseTools/Source/Python/GenFds/FdfParser.py | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> >diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py
> >b/BaseTools/Source/Python/GenFds/FdfParser.py
> >index 63687e98bb..a65f2cfd2d 100644
> >--- a/BaseTools/Source/Python/GenFds/FdfParser.py
> >+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
> >@@ -4469,10 +4469,15 @@ class FdfParser:
> >                 if self.__IsKeyword( "PCI_DEVICE_ID"):
> >                     if not self.__IsToken( "="):
> >                         raise Warning("expected '='", self.FileName,
> >self.CurrentLineNumber)
> >-                    if not self.__GetNextHexNumber():
> >-                        raise Warning("expected Hex device id", self.FileName,
> >self.CurrentLineNumber)
> >
> >-                    Overrides.PciDeviceId = self.__Token
> >+                    # Get a list of PCI IDs
> >+                    Overrides.PciDeviceId = ""
> >+
> >+                    while self.__GetNextHexNumber():
> >+                        Overrides.PciDeviceId += " " + self.__Token

Can we change to minimize looping string concatenation here?  This in a loop will cause lots of memory allocation/deallocation and slow things down.

Maybe : 
Overrides.PciDeviceId = "{} {}".format(Overrides.PciDeviceId, self.__Token)


> >+
> >+                    if not Overrides.PciDeviceId:
> >+                        raise Warning("expected one or more Hex 
> >+ device ids",
> >self.FileName, self.CurrentLineNumber)
> >                     continue
> >
> >                 if self.__IsKeyword( "PCI_REVISION"):
> >--
> >2.17.1
> >
> >_______________________________________________
> >edk2-devel mailing list
> >edk2-devel@lists.01.org
> >https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel