[edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache

Liming Gao liming.gao at intel.com
Thu May 9 14:04:40 UTC 2019


Jiewen:
  Yes. This is current tool design to leave the configuration instead of hard code. I would like to let 
tool owner to check this logic. 

Thanks
Liming
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, May 9, 2019 8:35 PM
> To: Shi, Steven <steven.shi at intel.com>; Gao, Liming <liming.gao at intel.com>; devel at edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng at intel.com>; Rodriguez, Christian <christian.rodriguez at intel.com>; Johnson, Michael
> <michael.johnson at intel.com>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache
> 
> Thanks to explain that.
> I think my general concern is: why we hardcode file suffix "INC" or "MCB" in the tool source code.
> 
> > > > > -            if '.inc' in str(f_ext):
> > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > >                  return False
> 
> What happen if we need skip another type of file later? update tool source code again ?
> 
> My thought is that: the tool should provide capability to skip *some* type of file.
> What "some" type of file to be skipped should be configured outside of tool source code.
> As such, when we need skip more type of file, we just update the configuration, instead of tool source code.
> 
> Just like we have tools_def and build_rule today, when we need update those configuration, we don't need update tool source code.
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: Shi, Steven
> > Sent: Wednesday, May 8, 2019 10:54 PM
> > To: Gao, Liming <liming.gao at intel.com>; Yao, Jiewen
> > <jiewen.yao at intel.com>; devel at edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng at intel.com>; Rodriguez, Christian
> > <christian.rodriguez at intel.com>; Johnson, Michael
> > <michael.johnson at intel.com>
> > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> > Cache
> >
> > > > Can we have better way to describe the binary cache?
> > > Yes. Steven provides some wiki pages for it.
> > https://raw.githubusercontent.com/shijunjing/WikiDoc/master/BuildCache/
> > MultipleBuildCache.jpg
> >
> > I agree it sounds a bit confusing.  Binary Cache is the legacy name for the
> > current model level build cache implementation. The current implementation
> > try only save and recovery the module binary files (e.g. only cache .efi but
> > no .obj, no .lib) which are necessary for the FV generation. But if we add .lib
> > cache and .fv cache support in the future, we could rename it as Build Cache
> > directly.
> >
> >
> > Thanks
> > Steven Shi
> >
> >
> > > -----Original Message-----
> > > From: Gao, Liming
> > > Sent: Thursday, May 9, 2019 1:47 PM
> > > To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io; Shi,
> > Steven
> > > <steven.shi at intel.com>
> > > Cc: Feng, Bob C <bob.c.feng at intel.com>; Rodriguez, Christian
> > > <christian.rodriguez at intel.com>; Johnson, Michael
> > > <michael.johnson at intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > Binary
> > > Cache
> > >
> > > Jiewen:
> > >
> > > > -----Original Message-----
> > > > From: Yao, Jiewen
> > > > Sent: Thursday, May 9, 2019 1:13 PM
> > > > To: devel at edk2.groups.io; Shi, Steven <steven.shi at intel.com>
> > > > Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming
> > > <liming.gao at intel.com>; Rodriguez, Christian
> > > <christian.rodriguez at intel.com>;
> > > > Johnson, Michael <michael.johnson at intel.com>
> > > > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > Binary
> > > Cache
> > > >
> > > > Hi
> > > > Can we have better way to describe the binary cache?
> > > Yes. Steven provides some wiki pages for it.
> > > >
> > > > I have 2 concern:
> > > > 1) we have other type of binary besides MCB. How can we indicate that
> > > without update base tool source code ?
> > > MCB will be combined to single bin file. The problem is that the platform
> > uses
> > > the bin file from output IA32 or X64
> > > directory instead of the module output directory. Binary cache is for the
> > > module output directory.
> > > I suggest to update the platform to consume the module output file. But, I
> > > meet with one tool issue to support
> > > RAW FILE type, because Microcode uses RAW file type. BZ
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=1765 is submitted.
> > > I suggest to fix this tool issue first.
> > >
> > > > 2) .inc might be text file, instead of binary. How can we support that?
> > > .inc is similar to .c source file. Its output is .mcb, then be combined to .bin
> > > file. .bin file will be cached.
> > > .inc is not required to be cached. On next build, build tool will compare the
> > > hash value of source file, if
> > > hash value is not changed. Previous cached .bin file will be used.
> > >
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf
> > Of
> > > > > Steven Shi
> > > > > Sent: Wednesday, May 8, 2019 10:00 PM
> > > > > To: devel at edk2.groups.io
> > > > > Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming
> > > > > <liming.gao at intel.com>; Rodriguez, Christian
> > > > > <christian.rodriguez at intel.com>; Johnson, Michael
> > > > > <michael.johnson at intel.com>
> > > > > Subject: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > Binary
> > > > > Cache
> > > > >
> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1723
> > > > >
> > > > > Current Kabylake open platform will build fail if enabled
> > > > > to consume the binary cache, because the binary cache doesn't
> > > > > support to recovery the .mcb microcode file,
> > > > > e.g. m80406E8_00000026.mcb, in a platform level folder which
> > > > > is outside of the module output folder. In normal build
> > > > > without cache, the .mcb file is copied through OS copy/move
> > > > > commands defined in build rules which are not supported by
> > > > > Binary Cache.
> > > > > Change the Binary Cache to skip the .mcb file type module and
> > > > > always rebuild the module to apply the full build rules if
> > > > > it contains .mcb file.
> > > > > ---
> > > > >  BaseTools/Source/Python/AutoGen/AutoGen.py | 8 ++++----
> > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > index 31721a6f9f..6b596c8a65 100644
> > > > > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > @@ -3925,9 +3925,9 @@ class ModuleAutoGen(AutoGen):
> > > > >          # If library or Module is binary do not skip by hash
> > > > >          if self.IsBinaryModule:
> > > > >              return False
> > > > > -        # .inc is contains binary information so do not skip by hash as
> > well
> > > > > +        # .inc and .mcb is contains binary information so do not skip
> > by
> > > > > hash as well
> > > > >          for f_ext in self.SourceFileList:
> > > > > -            if '.inc' in str(f_ext):
> > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > >                  return False
> > > > >          FileDir = path.join(GlobalData.gBinCacheSource,
> > > > > self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain,
> > self.Arch,
> > > > > self.SourceDir, self.MetaFile.BaseName)
> > > > >          HashFile = path.join(FileDir, self.Name + '.hash')
> > > > > @@ -4138,9 +4138,9 @@ class ModuleAutoGen(AutoGen):
> > > > >          # If library or Module is binary do not skip by hash
> > > > >          if self.IsBinaryModule:
> > > > >              return False
> > > > > -        # .inc is contains binary information so do not skip by hash as
> > well
> > > > > +        # .inc or '.mcb' is contains binary information so do not skip
> > by
> > > > > hash as well
> > > > >          for f_ext in self.SourceFileList:
> > > > > -            if '.inc' in str(f_ext):
> > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > >                  return False
> > > > >          if GlobalData.gUseHashCache:
> > > > >              # If there is a valid hash or function generated a valid
> > hash;
> > > > > function will return False
> > > > > --
> > > > > 2.17.1.windows.2
> > > > >
> > > > >
> > > > > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#40361): https://edk2.groups.io/g/devel/message/40361
Mute This Topic: https://groups.io/mt/31552068/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list