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

Yao, Jiewen jiewen.yao at intel.com
Thu May 9 12:34:54 UTC 2019


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 (#40334): https://edk2.groups.io/g/devel/message/40334
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