public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: "Kinney, Michael D" <michael.d.kinney@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>,
	"Chan, Amy" <amy.chan@intel.com>,
	Laszlo Ersek <lersek@redhat.com>
Subject: Re: [PATCH 2/6] MdeModulePkg PiDxeS3BootScriptLib: Support multiple PCI segment
Date: Tue, 23 Aug 2016 08:57:55 +0000	[thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB048310036A67C9@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <E92EE9817A31E24EB0585FDF735412F5648028A4@ORSMSX113.amr.corp.intel.com>

Mike,

I am not sure about you said "so those functions can be simpler and smaller", could you help explain it more?

ScriptPciCfg2Read() and ScriptPciCfg2Write() are for PCI_CONFIG2 opcodes.
ScriptPciCfgRead() and ScriptPciCfgWrite() are for PCI_CONFIG opcodes.

In current patch, ScriptPciCfgRead()/ScriptPciCfgWrite() will reuse ScriptPciCfg2Read()/ScriptPciCfg2Write() by Segment=0.

If PiDxeS3BootScriptLib depends on both PciLib and PciSegmentLib, ScriptPciCfgRead()/ScriptPciCfgWrite() will consume PciLib and ScriptPciCfg2Read()/ScriptPciCfg2Write() will consume PciSegmentLib, how those functions can be *simpler and smaller*?
And if platform declares PciSegmentLib in platform dsc with other instance, but not MdePkg/Library/BasePciSegmentLibPci, then both interfaces in PciSegmentLib and PciLib will be compiled to final image, I think the image size will be bigger.



Thanks,
Star
-----Original Message-----
From: Kinney, Michael D 
Sent: Tuesday, August 23, 2016 11:45 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Chan, Amy <amy.chan@intel.com>; Laszlo Ersek <lersek@redhat.com>
Subject: RE: [PATCH 2/6] MdeModulePkg PiDxeS3BootScriptLib: Support multiple PCI segment

Star,

PciLib uses smaller address and fewer params, so those functions can be simpler and smaller.  We only want to use PciSegmentLib when we have to.

Mike

> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, August 22, 2016 7:09 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 
> edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Chan, Amy 
> <amy.chan@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star 
> <star.zeng@intel.com>
> Subject: RE: [PATCH 2/6] MdeModulePkg PiDxeS3BootScriptLib: Support 
> multiple PCI segment
> 
> Mike,
> 
> That means PiDxeS3BootScriptLib will depend on both PciLib and PciSegmentLib.
> Is there any negative impact for PiDxeS3BootScriptLib to only depend 
> on PciSegmentLib?
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Tuesday, August 23, 2016 9:59 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, 
> Michael D <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Chan Amy <chan.amy@intel.com>; 
> Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [PATCH 2/6] MdeModulePkg PiDxeS3BootScriptLib: Support 
> multiple PCI segment
> 
> Star,
> 
> I do not think all PCI opcodes should be updated to use the PciSegmentLib.
> 
> Only the "2" versions of the PCI opcodes should use the PciSegmentLib.
> 
> This means that functions like ScriptPciCfgRead() should not be 
> modified at all and should still use the PciLib.  You will need to add 
> a new MACRO for encoding a PCI address with a segment#, maybe called PCI_SEGMENT_ADDRESS_ENCODE(S, A).
> 
> Mike
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Friday, August 19, 2016 12:35 AM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen 
> > <jiewen.yao@intel.com>; Kinney, Michael D 
> > <michael.d.kinney@intel.com>; Chan Amy <chan.amy@intel.com>; Laszlo 
> > Ersek <lersek@redhat.com>
> > Subject: [PATCH 2/6] MdeModulePkg PiDxeS3BootScriptLib: Support 
> > multiple PCI segment
> >
> > 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/BasePciSegmentLibP
> > PciSegmentLib|ci
> > PciSegmentLib|.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>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> >  .../PiDxeS3BootScriptLib/BootScriptExecute.c       | 135 ++++++++++-----------
> >  .../Library/PiDxeS3BootScriptLib/BootScriptSave.c  |  15 +--
> >  .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf    |   2 +-
> >  .../PiDxeS3BootScriptLib/InternalBootScriptLib.h   |  18 +--
> >  4 files changed, 80 insertions(+), 90 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c
> > b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c
> > index 9e63273bc19c..b865d4452fc8 100644
> > --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c
> > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Interpret and execute the S3 data in S3 boot script.
> >
> > -  Copyright (c) 2006 - 2015, Intel Corporation. All rights 
> > reserved.<BR>
> > +  Copyright (c) 2006 - 2016, Intel Corporation. All rights 
> > + reserved.<BR>
> >
> >    This program and the accompanying materials
> >    are licensed and made available under the terms and conditions @@
> > -639,9 +639,10 @@ BootScriptExecuteMemoryWrite (
> >
> >  }
> >  /**
> > -  Performance PCI configuration read operation
> > +  Performance PCI configuration 2 read operation
> >
> >    @param  Width   Width of the operation.
> > +  @param  Segment Pci segment number
> >    @param  Address Address of the operation.
> >    @param  Count   Count of the number of accesses to perform.
> >    @param  Buffer  Pointer to the buffer read from PCI config space 
> > @@
> > -652,8 +653,9 @@ BootScriptExecuteMemoryWrite (
> >
> >  **/
> >  EFI_STATUS
> > - ScriptPciCfgRead (
> > +ScriptPciCfg2Read (
> >    IN  S3_BOOT_SCRIPT_LIB_WIDTH    Width,
> > +  IN  UINT16                       Segment,
> >    IN  UINT64                       Address,
> >    IN  UINTN                        Count,
> >    OUT VOID                        *Buffer
> > @@ -663,11 +665,11 @@ ScriptPciCfgRead (
> >    UINTN       AddressStride;
> >    UINTN       BufferStride;
> >    PTR         Out;
> > -  UINTN       PciAddress;
> > +  UINT64      PciAddress;
> >
> >    Out.Buf = (UINT8 *) Buffer;
> >
> > -  PciAddress = PCI_ADDRESS_ENCODE (Address);
> > +  PciAddress = PCI_ADDRESS_ENCODE (Segment, Address);
> >
> >    Status = BuildLoopData (Width, PciAddress, &AddressStride, &BufferStride);
> >    if (EFI_ERROR (Status)) {
> > @@ -679,42 +681,42 @@ ScriptPciCfgRead (
> >    for (; Count > 0; Count--, PciAddress += AddressStride, Out.Buf 
> > += BufferStride)
> {
> >      switch (Width) {
> >      case S3BootScriptWidthUint8:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint8 - 0x%08x\n", PciAddress));
> > -      *Out.Uint8 = PciRead8 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint8 - 0x%016lx\n", PciAddress));
> > +      *Out.Uint8 = PciSegmentRead8 (PciAddress);
> >        break;
> >      case S3BootScriptWidthFifoUint8:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint8 - 0x%08x\n", PciAddress));
> > -      *Out.Uint8 = PciRead8 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint8 - 0x%016lx\n", PciAddress));
> > +      *Out.Uint8 = PciSegmentRead8 (PciAddress);
> >        break;
> >      case S3BootScriptWidthFillUint8:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint8 - 0x%08x\n", PciAddress));
> > -      *Out.Uint8 = PciRead8 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint8 - 0x%016lx\n", PciAddress));
> > +      *Out.Uint8 = PciSegmentRead8 (PciAddress);
> >        break;
> >
> >      case S3BootScriptWidthUint16:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint16 - 0x%08x\n", PciAddress));
> > -      *Out.Uint16 = PciRead16 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint16 - 0x%016lx\n", PciAddress));
> > +      *Out.Uint16 = PciSegmentRead16 (PciAddress);
> >        break;
> >      case S3BootScriptWidthFifoUint16:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint16 - 0x%08x\n", PciAddress));
> > -      *Out.Uint16 = PciRead16 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint16 - 
> > + 0x%016lx\n",
> PciAddress));
> > +      *Out.Uint16 = PciSegmentRead16 (PciAddress);
> >        break;
> >      case S3BootScriptWidthFillUint16:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint16 - 0x%08x\n", PciAddress));
> > -      *Out.Uint16 = PciRead16 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint16 - 
> > + 0x%016lx\n",
> PciAddress));
> > +      *Out.Uint16 = PciSegmentRead16 (PciAddress);
> >        break;
> >
> >      case S3BootScriptWidthUint32:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint32 - 0x%08x\n", PciAddress));
> > -      *Out.Uint32 = PciRead32 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint32 - 0x%016lx\n", PciAddress));
> > +      *Out.Uint32 = PciSegmentRead32 (PciAddress);
> >        break;
> >      case S3BootScriptWidthFifoUint32:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint32 - 0x%08x\n", PciAddress));
> > -      *Out.Uint32 = PciRead32 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint32 - 
> > + 0x%016lx\n",
> PciAddress));
> > +      *Out.Uint32 = PciSegmentRead32 (PciAddress);
> >        break;
> >      case S3BootScriptWidthFillUint32:
> > -      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint32 - 0x%08x\n", PciAddress));
> > -      *Out.Uint32 = PciRead32 (PciAddress);
> > +      DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint32 - 
> > + 0x%016lx\n",
> PciAddress));
> > +      *Out.Uint32 = PciSegmentRead32 (PciAddress);
> >        break;
> >
> >      default:
> > @@ -725,9 +727,10 @@ ScriptPciCfgRead (  }
> >
> >  /**
> > -  Performance PCI configuration write operation
> > +  Performance PCI configuration 2 write operation
> >
> >    @param  Width   Width of the operation.
> > +  @param  Segment Pci segment number
> >    @param  Address Address of the operation.
> >    @param  Count   Count of the number of accesses to perform.
> >    @param  Buffer  Pointer to the buffer write to PCI config space 
> > @@
> > -738,8 +741,9 @@ ScriptPciCfgRead (
> >
> >  **/
> >  EFI_STATUS
> > -ScriptPciCfgWrite (
> > +ScriptPciCfg2Write (
> >    IN  S3_BOOT_SCRIPT_LIB_WIDTH     Width,
> > +  IN  UINT16                       Segment,
> >    IN  UINT64                       Address,
> >    IN  UINTN                        Count,
> >    IN  VOID                         *Buffer
> > @@ -748,14 +752,14 @@ ScriptPciCfgWrite (
> >    EFI_STATUS  Status;
> >    UINTN       AddressStride;
> >    UINTN       BufferStride;
> > -  UINTN       OriginalPciAddress;
> > +  UINT64      OriginalPciAddress;
> >    PTR         In;
> >    PTR         OriginalIn;
> > -  UINTN       PciAddress;
> > +  UINT64      PciAddress;
> >
> >    In.Buf = (UINT8 *) Buffer;
> >
> > -  PciAddress = PCI_ADDRESS_ENCODE (Address);
> > +  PciAddress = PCI_ADDRESS_ENCODE (Segment, Address);
> >
> >    Status = BuildLoopData (Width, PciAddress, &AddressStride, &BufferStride);
> >    if (EFI_ERROR (Status)) {
> > @@ -769,40 +773,40 @@ ScriptPciCfgWrite (
> >    for (; Count > 0; Count--, PciAddress += AddressStride, In.Buf += 
> > BufferStride)
> {
> >      switch (Width) {
> >        case S3BootScriptWidthUint8:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint8 - 0x%08x (0x%02x)\n",
> > PciAddress, (UINTN)*In.Uint8));
> > -        PciWrite8 (PciAddress, *In.Uint8);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint8 - 0x%016lx 
> > + (0x%02x)\n",
> > PciAddress, (UINTN)*In.Uint8));
> > +        PciSegmentWrite8 (PciAddress, *In.Uint8);
> >          break;
> >        case S3BootScriptWidthFifoUint8:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint8 - 0x%08x (0x%02x)\n",
> > OriginalPciAddress, (UINTN)*In.Uint8));
> > -        PciWrite8 (OriginalPciAddress, *In.Uint8);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint8 - 0x%016lx 
> > + (0x%02x)\n",
> > OriginalPciAddress, (UINTN)*In.Uint8));
> > +        PciSegmentWrite8 (OriginalPciAddress, *In.Uint8);
> >          break;
> >        case S3BootScriptWidthFillUint8:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint8 - 0x%08x (0x%02x)\n",
> > PciAddress, (UINTN)*OriginalIn.Uint8));
> > -        PciWrite8 (PciAddress, *OriginalIn.Uint8);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint8 - 0x%016lx 
> > + (0x%02x)\n",
> > PciAddress, (UINTN)*OriginalIn.Uint8));
> > +        PciSegmentWrite8 (PciAddress, *OriginalIn.Uint8);
> >          break;
> >        case S3BootScriptWidthUint16:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint16 - 0x%08x (0x%04x)\n",
> > PciAddress, (UINTN)*In.Uint16));
> > -        PciWrite16 (PciAddress, *In.Uint16);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint16 - 0x%016lx 
> > + (0x%04x)\n",
> > PciAddress, (UINTN)*In.Uint16));
> > +        PciSegmentWrite16 (PciAddress, *In.Uint16);
> >          break;
> >        case S3BootScriptWidthFifoUint16:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint16 - 0x%08x (0x%04x)\n",
> > OriginalPciAddress, (UINTN)*In.Uint16));
> > -        PciWrite16 (OriginalPciAddress, *In.Uint16);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint16 - 0x%016lx 
> > + (0x%04x)\n",
> > OriginalPciAddress, (UINTN)*In.Uint16));
> > +        PciSegmentWrite16 (OriginalPciAddress, *In.Uint16);
> >          break;
> >        case S3BootScriptWidthFillUint16:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint16 - 0x%08x (0x%04x)\n",
> > PciAddress, (UINTN)*OriginalIn.Uint16));
> > -        PciWrite16 (PciAddress, *OriginalIn.Uint16);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint16 - 0x%016lx 
> > + (0x%04x)\n",
> > PciAddress, (UINTN)*OriginalIn.Uint16));
> > +        PciSegmentWrite16 (PciAddress, *OriginalIn.Uint16);
> >          break;
> >        case S3BootScriptWidthUint32:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint32 - 0x%08x (0x%08x)\n",
> > PciAddress, (UINTN)*In.Uint32));
> > -        PciWrite32 (PciAddress, *In.Uint32);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthUint32 - 0x%016lx 
> > + (0x%08x)\n",
> > PciAddress, (UINTN)*In.Uint32));
> > +        PciSegmentWrite32 (PciAddress, *In.Uint32);
> >          break;
> >        case S3BootScriptWidthFifoUint32:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint32 - 0x%08x (0x%08x)\n",
> > OriginalPciAddress, (UINTN)*In.Uint32));
> > -        PciWrite32 (OriginalPciAddress, *In.Uint32);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFifoUint32 - 0x%016lx 
> > + (0x%08x)\n",
> > OriginalPciAddress, (UINTN)*In.Uint32));
> > +        PciSegmentWrite32 (OriginalPciAddress, *In.Uint32);
> >          break;
> >        case S3BootScriptWidthFillUint32:
> > -        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint32 - 0x%08x (0x%08x)\n",
> > (UINTN)PciAddress, (UINTN)*OriginalIn.Uint32));
> > -        PciWrite32 (PciAddress, *OriginalIn.Uint32);
> > +        DEBUG ((EFI_D_INFO, "S3BootScriptWidthFillUint32 - 0x%016lx 
> > + (0x%08x)\n",
> > (UINTN)PciAddress, (UINTN)*OriginalIn.Uint32));
> > +        PciSegmentWrite32 (PciAddress, *OriginalIn.Uint32);
> >          break;
> >        default:
> >          return EFI_INVALID_PARAMETER; @@ -811,10 +815,9 @@ 
> > ScriptPciCfgWrite (
> >    return EFI_SUCCESS;
> >  }
> >  /**
> > -  Performance PCI configuration 2 read operation
> > +  Performance PCI configuration read operation
> >
> >    @param     Width                      Width of the operation.
> > -  @param     Segment                    Pci segment number
> >    @param     Address                    Address of the operation.
> >    @param     Count                      Count of the number of accesses to
> perform.
> >    @param     Buffer                     Pointer to the buffer to read from PCI
> > config space.
> > @@ -824,27 +827,22 @@ ScriptPciCfgWrite (
> >                                          Buffer is NULL.
> >                                          The Buffer is not aligned 
> > for the given Width.
> >                                          Address is outside the 
> > legal range of I/O ports.
> > -  @note  A known Limitations in the implementation which is the 
> > 'Segment' parameter is assumed as
> > -         Zero, or else, assert.
> > +
> >  **/
> >  EFI_STATUS
> > -ScriptPciCfg2Read (
> > +ScriptPciCfgRead (
> >    IN  S3_BOOT_SCRIPT_LIB_WIDTH    Width,
> > -  IN  UINT16                   Segment,
> >    IN  UINT64                   Address,
> >    IN  UINTN                    Count,
> >    OUT VOID                     *Buffer
> >    )
> >  {
> > -  ASSERT (Segment==0);
> > -
> > -  return ScriptPciCfgRead (Width, Address, Count, Buffer);
> > +  return ScriptPciCfg2Read (Width, 0, Address, Count, Buffer);
> >  }
> >  /**
> > -  Performance PCI configuration 2 write operation
> > +  Performance PCI configuration write operation
> >
> >    @param     Width                      Width of the operation.
> > -  @param     Segment                    Pci segment number
> >    @param     Address                    Address of the operation.
> >    @param     Count                      Count of the number of accesses to
> perform.
> >    @param     Buffer                     Pointer to the buffer to write to PCI
> config
> > space.
> > @@ -854,22 +852,18 @@ ScriptPciCfg2Read (
> >                                          Buffer is NULL.
> >                                          The Buffer is not aligned 
> > for the given Width.
> >                                          Address is outside the 
> > legal range of I/O ports.
> > -  @note  A known Limitations in the implementation which is the 
> > 'Segment' parameter is assumed as
> > -         Zero, or else, assert.
> >
> >  **/
> >  EFI_STATUS
> >  EFIAPI
> > -ScriptPciCfg2Write (
> > +ScriptPciCfgWrite (
> >    IN  S3_BOOT_SCRIPT_LIB_WIDTH    Width,
> > -  IN  UINT16                   Segment,
> >    IN  UINT64                   Address,
> >    IN  UINTN                    Count,
> >    IN  VOID                     *Buffer
> >    )
> >  {
> > -  ASSERT (Segment==0);
> > -  return ScriptPciCfgWrite (Width, Address, Count, Buffer);
> > +  return ScriptPciCfg2Write (Width, 0, Address, Count, Buffer);
> >  }
> >  /**
> >    Interprete the boot script node with EFI_BOOT_SCRIPT_PCI_CONFIG_WRITE OP code.
> > @@ -896,7 +890,7 @@ BootScriptExecutePciCfgWrite (
> >    Count   = PciCfgWrite.Count;
> >    Buffer  = Script + sizeof(EFI_BOOT_SCRIPT_PCI_CONFIG_WRITE);
> >
> > -  DEBUG ((EFI_D_INFO, "BootScriptExecutePciCfgWrite - 0x%08x, 
> > 0x%08x, 0x%08x\n", PCI_ADDRESS_ENCODE (Address), Count, 
> > (UINTN)Width));
> > +  DEBUG ((EFI_D_INFO, "BootScriptExecutePciCfgWrite - 0x%016lx, 
> > + 0x%08x, 0x%08x\n",
> > PCI_ADDRESS_ENCODE (0, Address), Count, (UINTN)Width));
> >    return ScriptPciCfgWrite (Width, Address, Count, Buffer);  }
> >  /**
> > @@ -1012,7 +1006,7 @@ BootScriptExecutePciCfgReadWrite (
> >
> >    CopyMem((VOID*)&PciCfgReadWrite, (VOID*)Script, 
> > sizeof(EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE));
> >
> > -  DEBUG ((EFI_D_INFO, "BootScriptExecutePciCfgReadWrite - 0x%08x, 
> > 0x%016lx, 0x%016lx\n", PCI_ADDRESS_ENCODE (PciCfgReadWrite.Address), 
> > AndMask, OrMask));
> > +  DEBUG ((EFI_D_INFO, "BootScriptExecutePciCfgReadWrite - 0x%016lx, 
> > + 0x%016lx,
> > 0x%016lx\n", PCI_ADDRESS_ENCODE (0, PciCfgReadWrite.Address), 
> > AndMask, OrMask));
> >
> >    Status = ScriptPciCfgRead (
> >               (S3_BOOT_SCRIPT_LIB_WIDTH) PciCfgReadWrite.Width, @@
> > -1422,7 +1416,7 @@ BootScriptExecutePciCfg2Write (
> >    Count   = PciCfg2Write.Count;
> >    Buffer  = Script + sizeof(EFI_BOOT_SCRIPT_PCI_CONFIG2_WRITE);
> >
> > -  DEBUG ((EFI_D_INFO, "BootScriptExecutePciCfg2Write - 0x%04x, 
> > 0x%08x, 0x%08x, 0x%08x\n", Segment, PCI_ADDRESS_ENCODE (Address), 
> > Count, (UINTN)Width));
> > +  DEBUG ((EFI_D_INFO, "BootScriptExecutePciCfg2Write - 0x%016lx, 
> > + 0x%08x, 0x%08x\n",
> > PCI_ADDRESS_ENCODE (Segment, Address), Count, (UINTN)Width));
> >    return ScriptPciCfg2Write (Width, Segment, Address, Count, 
> > Buffer); }
> >
> > @@ -1452,7 +1446,7 @@ BootScriptExecutePciCfg2ReadWrite (
> >
> >    CopyMem ((VOID*)&PciCfg2ReadWrite, (VOID*)Script, 
> > sizeof(EFI_BOOT_SCRIPT_PCI_CONFIG2_READ_WRITE));
> >
> > -  DEBUG ((EFI_D_INFO, "BootScriptExecutePciCfg2ReadWrite - 0x%04x, 
> > 0x%08x, 0x%016lx, 0x%016lx\n", PciCfg2ReadWrite.Segment, 
> > PCI_ADDRESS_ENCODE (PciCfg2ReadWrite.Address), AndMask, OrMask));
> > +  DEBUG ((EFI_D_INFO, "BootScriptExecutePciCfg2ReadWrite - 
> > + 0x%016lx, 0x%016lx,
> > 0x%016lx\n", PCI_ADDRESS_ENCODE (PciCfg2ReadWrite.Segment, 
> > PciCfg2ReadWrite.Address), AndMask, OrMask));
> >
> >    Status = ScriptPciCfg2Read (
> >               (S3_BOOT_SCRIPT_LIB_WIDTH) PciCfg2ReadWrite.Width, @@
> > -1499,7 +1493,7 @@ BootScriptPciCfgPoll (
> >    EFI_BOOT_SCRIPT_PCI_CONFIG_POLL PciCfgPoll;
> >    CopyMem ((VOID*)&PciCfgPoll, (VOID*)Script, 
> > sizeof(EFI_BOOT_SCRIPT_PCI_CONFIG_POLL));
> >
> > -  DEBUG ((EFI_D_INFO, "BootScriptPciCfgPoll - 0x%08x, 0x%016lx, 
> > 0x%016lx\n", PCI_ADDRESS_ENCODE (PciCfgPoll.Address), AndMask, 
> > OrMask));
> > +  DEBUG ((EFI_D_INFO, "BootScriptPciCfgPoll - 0x%016lx, 0x%016lx, 
> > + 0x%016lx\n",
> > PCI_ADDRESS_ENCODE (0, PciCfgPoll.Address), AndMask, OrMask));
> >
> >    Data = 0;
> >    Status = ScriptPciCfgRead (
> > @@ -1561,7 +1555,7 @@ BootScriptPciCfg2Poll (
> >    Data = 0;
> >    CopyMem ((VOID*)&PciCfg2Poll, (VOID*)Script, 
> > sizeof(EFI_BOOT_SCRIPT_PCI_CONFIG2_POLL));
> >
> > -  DEBUG ((EFI_D_INFO, "BootScriptPciCfg2Poll - 0x%04x, 0x%08x, 
> > 0x%016lx, 0x%016lx\n", PciCfg2Poll.Segment, PCI_ADDRESS_ENCODE 
> > (PciCfg2Poll.Address), AndMask, OrMask));
> > +  DEBUG ((EFI_D_INFO, "BootScriptPciCfg2Poll - 0x%016lx, 0x%016lx, 
> > + 0x%016lx\n",
> > PCI_ADDRESS_ENCODE (PciCfg2Poll.Segment, PciCfg2Poll.Address), 
> > AndMask, OrMask));
> >
> >    Status = ScriptPciCfg2Read (
> >               (S3_BOOT_SCRIPT_LIB_WIDTH) PciCfg2Poll.Width, @@ 
> > -1604,9
> > +1598,6 @@ BootScriptPciCfg2Poll (
> >    @retval RETURN_SUCCESS           The boot script table was executed
> successfully.
> >    @retval RETURN_UNSUPPORTED       Invalid script table or opcode.
> >
> > -  @note  A known Limitations in the implementation: When 
> > interpreting the opcode EFI_BOOT_SCRIPT_PCI_CONFIG2_WRITE_OPCODE
> > -         EFI_BOOT_SCRIPT_PCI_CONFIG2_READ_WRITE_OPCODE and
> > EFI_BOOT_SCRIPT_PCI_CONFIG2_POLL_OPCODE, the 'Segment' parameter is assumed as
> > -         Zero, or else, assert.
> >  **/
> >  RETURN_STATUS
> >  EFIAPI
> > diff --git
> > a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> > b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> > index de3915511cec..9ff5b80e7a36 100644
> > --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> > @@ -1291,7 +1291,7 @@ S3BootScriptSavePciCfgReadWrite (
> >
> >    @retval RETURN_OUT_OF_RESOURCES  Not enough memory for the table do operation.
> >    @retval RETURN_SUCCESS           Opcode is added.
> > -  @note  A known Limitations in the implementation which is 
> > non-zero Segment and 64bits operations are not supported.
> > +  @note  A known Limitations in the implementation which is 64bits 
> > + operations are
> > not supported.
> >
> >  **/
> >  RETURN_STATUS
> > @@ -1309,8 +1309,7 @@ S3BootScriptSavePciCfg2Write (
> >    UINT8                 WidthInByte;
> >    EFI_BOOT_SCRIPT_PCI_CONFIG2_WRITE  ScriptPciWrite2;
> >
> > -  if (Segment != 0 ||
> > -      Width == S3BootScriptWidthUint64 ||
> > +  if (Width == S3BootScriptWidthUint64 ||
> >        Width == S3BootScriptWidthFifoUint64 ||
> >        Width == S3BootScriptWidthFillUint64) {
> >      return EFI_INVALID_PARAMETER;
> > @@ -1351,7 +1350,7 @@ S3BootScriptSavePciCfg2Write (
> >
> >    @retval RETURN_OUT_OF_RESOURCES  Not enough memory for the table do operation.
> >    @retval RETURN_SUCCESS           Opcode is added.
> > -  @note  A known Limitations in the implementation which is 
> > non-zero Segment and 64bits operations are not supported.
> > +  @note  A known Limitations in the implementation which is 64bits 
> > + operations are
> > not supported.
> >
> >  **/
> >  RETURN_STATUS
> > @@ -1369,8 +1368,7 @@ S3BootScriptSavePciCfg2ReadWrite (
> >    UINT8                 WidthInByte;
> >    EFI_BOOT_SCRIPT_PCI_CONFIG2_READ_WRITE  ScriptPciReadWrite2;
> >
> > -  if (Segment != 0 ||
> > -      Width == S3BootScriptWidthUint64 ||
> > +  if (Width == S3BootScriptWidthUint64 ||
> >        Width == S3BootScriptWidthFifoUint64 ||
> >        Width == S3BootScriptWidthFillUint64) {
> >      return EFI_INVALID_PARAMETER;
> > @@ -1946,7 +1944,7 @@ S3BootScriptSavePciPoll (
> >
> >   @retval RETURN_OUT_OF_RESOURCES  Not enough memory for the table do operation.
> >   @retval RETURN_SUCCESS           Opcode is added.
> > -  @note  A known Limitations in the implementation which is 
> > non-zero Segment and 64bits operations are not supported.
> > +  @note  A known Limitations in the implementation which is 64bits 
> > + operations are
> > not supported.
> >
> >  **/
> >  RETURN_STATUS
> > @@ -1965,8 +1963,7 @@ S3BootScriptSavePci2Poll (
> >    UINT8                    Length;
> >    EFI_BOOT_SCRIPT_PCI_CONFIG2_POLL  ScriptPci2Poll;
> >
> > -  if (Segment != 0 ||
> > -      Width == S3BootScriptWidthUint64 ||
> > +  if (Width == S3BootScriptWidthUint64 ||
> >        Width == S3BootScriptWidthFifoUint64 ||
> >        Width == S3BootScriptWidthFillUint64) {
> >      return EFI_INVALID_PARAMETER;
> > diff --git
> > a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> > b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> > index 4125cdd70f10..0feff3661233 100644
> > --- 
> > a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.i
> > +++ nf
> > @@ -52,7 +52,7 @@ [LibraryClasses]
> >    PcdLib
> >    UefiLib
> >    SmbusLib
> > -  PciLib
> > +  PciSegmentLib
> >    IoLib
> >    LockBoxLib
> >
> > diff --git
> > a/MdeModulePkg/Library/PiDxeS3BootScriptLib/InternalBootScriptLib.h
> > b/MdeModulePkg/Library/PiDxeS3BootScriptLib/InternalBootScriptLib.h
> > index 9b150480dbca..ffbf5d2688da 100644
> > ---
> > a/MdeModulePkg/Library/PiDxeS3BootScriptLib/InternalBootScriptLib.h
> > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/InternalBootScriptLib.
> > +++ h
> > @@ -2,7 +2,7 @@
> >    Support for S3 boot script lib. This file defined some internal 
> > macro and
> internal
> >    data structure
> >
> > -  Copyright (c) 2006 - 2015, Intel Corporation. All rights 
> > reserved.<BR>
> > +  Copyright (c) 2006 - 2016, Intel Corporation. All rights 
> > + reserved.<BR>
> >
> >    This program and the accompanying materials
> >    are licensed and made available under the terms and conditions @@
> > -33,7 +33,7 @@  #include <Library/PcdLib.h>  #include 
> > <Library/SmbusLib.h>  #include <Library/IoLib.h> -#include 
> > <Library/PciLib.h>
> > +#include <Library/PciSegmentLib.h>
> >  #include <Library/DebugLib.h>
> >  #include <Library/BaseMemoryLib.h>
> >  #include <Library/TimerLib.h>
> > @@ -45,13 +45,15 @@
> >  #define MAX_IO_ADDRESS 0xFFFF
> >
> >  //
> > -// Macro to convert a UEFI PCI address to a PCI Library PCI address
> > +// Macro to convert a UEFI PCI address + segment to a PCI Segment 
> > +Library PCI
> > address
> >  //
> > -#define PCI_ADDRESS_ENCODE(A) (UINTN)PCI_LIB_ADDRESS( \
> > -        ((((UINTN)(A))& 0xff000000) >> 24), ((((UINTN)(A)) &0x00ff0000) >> 16),
> > ((((UINTN)(A)) & 0xff00) >> 8), ((RShiftU64 ((A), 32) & 0xfff) | ((A)& 0xff)) \
> > -        )
> > -
> > -
> > +#define PCI_ADDRESS_ENCODE(S, A) PCI_SEGMENT_LIB_ADDRESS( \
> > +                                   S, \
> > +                                   ((((UINTN)(A)) & 0xff000000) >> 24), \
> > +                                   ((((UINTN)(A)) & 0x00ff0000) >> 16), \
> > +                                   ((((UINTN)(A)) & 0xff00) >> 8), \
> > +                                   ((RShiftU64 ((A), 32) & 0xfff) | 
> > +((A) & 0xff))
> \
> > +                                   )
> >
> >  typedef union {
> >    UINT8 volatile  *Buf;
> > --
> > 2.7.0.windows.1



  reply	other threads:[~2016-08-23  8:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-19  7:35 [PATCH 0/6] PiDxeS3BootScriptLib: Support multiple PCI segment Star Zeng
2016-08-19  7:35 ` [PATCH 1/6] MdeModulePkg PiDxeS3BootScriptLib: Remove the trailing white spaces Star Zeng
2016-08-19  7:35 ` [PATCH 2/6] MdeModulePkg PiDxeS3BootScriptLib: Support multiple PCI segment Star Zeng
2016-08-23  1:58   ` Kinney, Michael D
2016-08-23  2:09     ` Zeng, Star
2016-08-23  3:44       ` Kinney, Michael D
2016-08-23  8:57         ` Zeng, Star [this message]
2016-08-19  7:35 ` [PATCH 3/6] Vlv2TbltDevicePkg: Declare PciSegmentLib in platform dsc Star Zeng
2016-08-22  6:54   ` Wei, David
2016-08-19  7:35 ` [PATCH 4/6] QuarkPlatformPkg: " Star Zeng
2016-08-19  7:35 ` [PATCH 5/6] QuarkSocPkg/QuarkSocPkg.dsc: Declare PciSegmentLib Star Zeng
2016-08-19  7:35 ` [PATCH 6/6] SecurityPkg/SecurityPkg.dsc: " Star Zeng
2016-08-19 13:35 ` [PATCH 0/6] PiDxeS3BootScriptLib: Support multiple PCI segment Yao, Jiewen
2016-08-24  2:26 ` Laszlo Ersek
2016-09-01 15:57   ` Kinney, Michael D

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0C09AFA07DD0434D9E2A0C6AEB048310036A67C9@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox