Laszlo, You are correct. No functional change.
I think I was more concerned about source changes to the functions that were already well tested, and wanted to focus the source code changes on the functions that were being enabled for multiple segments. If we are all confident from the code review and testing that we have not introduced any regression, then it is a good change. Reviewed-by: Michael Kinney <[email protected]> Mike > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Laszlo > Ersek > Sent: Tuesday, August 23, 2016 7:26 PM > To: Zeng, Star <[email protected]>; Kinney, Michael D > <[email protected]> > Cc: [email protected]; Yao, Jiewen <[email protected]>; Chan Amy > <[email protected]>; Zhang, Chao B <[email protected]>; Wei, David > <[email protected]> > Subject: Re: [edk2] [PATCH 0/6] PiDxeS3BootScriptLib: Support multiple PCI > segment > > On 08/19/16 03:35, Star Zeng wrote: > > Support multiple PCI segment for PCI_CONFIG2 opcodes. > > > > PiDxeS3BootScriptLib needs to be updated to consume PciSegmentLib > > instead of PciLib. That means platforms need to add PciSegmentLib > > declaration like below in platform dsc if the PciSegmentLib was > > not declared in platform dsc before. > > > > PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf > > > > For platforms only have one segment, > > MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf is recommended > > to be used and declared in platform dsc for PiDxeS3BootScriptLib to have > > equivalent functionality with before. > > > > Cc: Jiewen Yao <[email protected]> > > Cc: Michael D Kinney <[email protected]> > > Cc: Chan Amy <[email protected]> > > Cc: Laszlo Ersek <[email protected]> > > Cc: Kelly Steele <[email protected]> > > Cc: David Wei <[email protected]> > > Cc: Chao Zhang <[email protected]> > > > > Star Zeng (6): > > MdeModulePkg PiDxeS3BootScriptLib: Remove the trailing white spaces > > MdeModulePkg PiDxeS3BootScriptLib: Support multiple PCI segment > > Vlv2TbltDevicePkg: Declare PciSegmentLib in platform dsc > > QuarkPlatformPkg: Declare PciSegmentLib in platform dsc > > QuarkSocPkg/QuarkSocPkg.dsc: Declare PciSegmentLib > > SecurityPkg/SecurityPkg.dsc: Declare PciSegmentLib > > > > .../PiDxeS3BootScriptLib/BootScriptExecute.c | 411 > > +++++++++---------- > > .../BootScriptInternalFormat.h | 2 +- > > .../Library/PiDxeS3BootScriptLib/BootScriptSave.c | 451 > > ++++++++++----------- > > .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf | 4 +- > > .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.uni | 2 +- > > .../PiDxeS3BootScriptLib/InternalBootScriptLib.h | 26 +- > > QuarkPlatformPkg/Quark.dsc | 1 + > > QuarkPlatformPkg/QuarkMin.dsc | 1 + > > QuarkSocPkg/QuarkSocPkg.dsc | 1 + > > SecurityPkg/SecurityPkg.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 + > > 13 files changed, 450 insertions(+), 453 deletions(-) > > > > For patches #1 and #2: > > Tested-by: Laszlo Ersek <[email protected]> > > (Also compared some logs.) > > I read the sub-thread under #2, but I don't understand Mike's concern. I > can be wrong of course, but in my understanding, the boot script's > internal representation does not change. The "saver" side only relaxes > the Segment=0 requirement. And, the "executor side" accommodates nonzero > segments in the Pci2 opcodes, and rebases the Pci[1] opcode functions on > top of Pci2 opcode functions (with hardcoded Segment=0). > > I don't understand why calling PciSegmentLib functions with a UINT64 > parameter where the segment bit-field is hardcoded to 0 is worse than > calling PciLib (uncapable of nonzero segments) with an UINTN parameter. > > Is this about the cost of a function call on Ia32? That is, assuming a > very long S3 boot script, the patch might noticeably slow down S3 resume > on Ia32? > > Thanks > Laszlo > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
