Re: [PATCH 0/6] PiDxeS3BootScriptLib: Support multiple PCI segment

[Laszlo Ersek <lersek@redhat.com>]

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 <jiewen.yao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Chan Amy <chan.amy@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Kelly Steele <kelly.steele@intel.com>
> Cc: David Wei <david.wei@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> 
> 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 <lersek@redhat.com>

(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