public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Liming" <liming.gao@intel.com>
To: "Duran, Leo" <leo.duran@amd.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Justen, Jordan L" <jordan.l.justen@intel.com>,
	"Singh, Brijesh" <brijesh.singh@amd.com>,
	"Fan, Jeff" <jeff.fan@intel.com>,
	"lersek@redhat.com" <lersek@redhat.com>
Subject: Re: [PATCH 0/8] IoLib class library
Date: Wed, 11 Jan 2017 04:06:30 +0000	[thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14D6CD269@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <DM5PR12MB12439CB4626C279A075664FBF9660@DM5PR12MB1243.namprd12.prod.outlook.com>

Leo:
edk2\UefiCpuPkg\CpuIo2Dxe\CpuIo2Dxe.c CpuIoServiceRead() bases on IoReadFifo8() for EfiCpuIoWidthFifoUint8 width. So, IoLibCupIo2 library instance IoReadFifo8() implementation can call mCpuIo->Io.Read (mCpuIo, EfiCpuIoWidthFifoUint8, Port, Count, Buffer);

Thanks
Liming
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo
Sent: Wednesday, January 11, 2017 11:37 AM
To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>; lersek@redhat.com
Subject: Re: [edk2] [PATCH 0/8] IoLib class library

Liming...

> -----Original Message-----
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Tuesday, January 10, 2017 7:42 PM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@lists.01.org
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; lersek@redhat.com; Fan, Jeff
> <jeff.fan@intel.com>
> Subject: RE: [edk2] [PATCH 0/8] IoLib class library
> 
> Leo:
>   Thanks for your update. Here is my comments.
> 1) PeiIoLib, DxeIoLibCpuIo, DxeIoLibCpuIo2 library instance can base on CPU
> IO service to do FifoIo operation. They don't implement them again.
> 2) IntelFrameworkModulePkg CpuIoDxe and UefiCpuPkg CpuIoPei driver can
> be updated to base on FifoIo API for their FifoIo implementation.

[Duran, Leo] I actually considered that, but the CPU I/O driver does not provide FiFiIo service in its implementation of the EFI_CPU_IO2_PROTOCOL.
The CPU I/O driver does use the Fifo routines internally, and it does so when that caller request IoRead/IoWrite... see the CPU I/O service routines below.

> 3) One coding style issue. We don't assign value to the variable declaration.
> UINT8 *Buffer8 = (UINT8 *)Buffer;
> ==>
> UINT8 *Buffer8;
> Buffer8 = (UINT8 *)Buffer;

[Duran, Leo] OK, I'll change that.

> 
> Thanks
> Liming
[Duran, Leo]

EFI_STATUS
EFIAPI
CpuIoServiceRead (
  IN  EFI_CPU_IO2_PROTOCOL       *This,
  IN  EFI_CPU_IO_PROTOCOL_WIDTH  Width,
  IN  UINT64                     Address,
  IN  UINTN                      Count,
  OUT VOID                       *Buffer
  )
{
  EFI_STATUS                 Status;
  UINT8                      InStride;
  UINT8                      OutStride;
  EFI_CPU_IO_PROTOCOL_WIDTH  OperationWidth;
  UINT8                      *Uint8Buffer;

  Status = CpuIoCheckParameter (FALSE, Width, Address, Count, Buffer);
  if (EFI_ERROR (Status)) {
    return Status;
  }

  //
  // Select loop based on the width of the transfer
  //
  InStride = mInStride[Width];
  OutStride = mOutStride[Width];
  OperationWidth = (EFI_CPU_IO_PROTOCOL_WIDTH) (Width & 0x03);

#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
  if (InStride == 0) {
    switch (OperationWidth) {
    case EfiCpuIoWidthUint8:
      IoReadFifo8 ((UINTN)Address, Count, Buffer);
      return EFI_SUCCESS;
    case EfiCpuIoWidthUint16:
      IoReadFifo16 ((UINTN)Address, Count, Buffer);
      return EFI_SUCCESS;
    case EfiCpuIoWidthUint32:
      IoReadFifo32 ((UINTN)Address, Count, Buffer);
      return EFI_SUCCESS;
    default:
      //
      // The CpuIoCheckParameter call above will ensure that this
      // path is not taken.
      //
      ASSERT (FALSE);
      break;
    }
  }
#endif

  for (Uint8Buffer = Buffer; Count > 0; Address += InStride, Uint8Buffer += OutStride, Count--) {
    if (OperationWidth == EfiCpuIoWidthUint8) {
      *Uint8Buffer = IoRead8 ((UINTN)Address);
    } else if (OperationWidth == EfiCpuIoWidthUint16) {
      *((UINT16 *)Uint8Buffer) = IoRead16 ((UINTN)Address);
    } else if (OperationWidth == EfiCpuIoWidthUint32) {
      *((UINT32 *)Uint8Buffer) = IoRead32 ((UINTN)Address);
    }
  }

  return EFI_SUCCESS;
}

EFI_STATUS
EFIAPI
CpuIoServiceWrite (
  IN EFI_CPU_IO2_PROTOCOL       *This,
  IN EFI_CPU_IO_PROTOCOL_WIDTH  Width,
  IN UINT64                     Address,
  IN UINTN                      Count,
  IN VOID                       *Buffer
  )
{
  EFI_STATUS                 Status;
  UINT8                      InStride;
  UINT8                      OutStride;
  EFI_CPU_IO_PROTOCOL_WIDTH  OperationWidth;
  UINT8                      *Uint8Buffer;

  //
  // Make sure the parameters are valid
  //
  Status = CpuIoCheckParameter (FALSE, Width, Address, Count, Buffer);
  if (EFI_ERROR (Status)) {
    return Status;
  }

  //
  // Select loop based on the width of the transfer
  //
  InStride = mInStride[Width];
  OutStride = mOutStride[Width];
  OperationWidth = (EFI_CPU_IO_PROTOCOL_WIDTH) (Width & 0x03);

#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
  if (InStride == 0) {
    switch (OperationWidth) {
    case EfiCpuIoWidthUint8:
      IoWriteFifo8 ((UINTN)Address, Count, Buffer);
      return EFI_SUCCESS;
    case EfiCpuIoWidthUint16:
      IoWriteFifo16 ((UINTN)Address, Count, Buffer);
      return EFI_SUCCESS;
    case EfiCpuIoWidthUint32:
      IoWriteFifo32 ((UINTN)Address, Count, Buffer);
      return EFI_SUCCESS;
    default:
      //
      // The CpuIoCheckParameter call above will ensure that this
      // path is not taken.
      //
      ASSERT (FALSE);
      break;
    }
  }
#endif

  for (Uint8Buffer = (UINT8 *)Buffer; Count > 0; Address += InStride, Uint8Buffer += OutStride, Count--) {
    if (OperationWidth == EfiCpuIoWidthUint8) {
      IoWrite8 ((UINTN)Address, *Uint8Buffer);
    } else if (OperationWidth == EfiCpuIoWidthUint16) {
      IoWrite16 ((UINTN)Address, *((UINT16 *)Uint8Buffer));
    } else if (OperationWidth == EfiCpuIoWidthUint32) {
      IoWrite32 ((UINTN)Address, *((UINT32 *)Uint8Buffer));
    }
  }
  
  return EFI_SUCCESS;
}
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Leo Duran
> Sent: Wednesday, January 11, 2017 7:56 AM
> To: edk2-devel@lists.01.org
> Cc: brijesh.singh@amd.com; Justen, Jordan L <jordan.l.justen@intel.com>;
> Gao, Liming <liming.gao@intel.com>; Leo Duran <leo.duran@amd.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; lersek@redhat.com; Fan,
> Jeff <jeff.fan@intel.com>
> Subject: [edk2] [PATCH 0/8] IoLib class library
> 
> The UefiCpuPkg/CpuIo2Dxe driver and the QemuCfgLib library have
> duplicate implementations of I/O Fifo routines. The patch series moves the
> I/O Fifo routines into the BaseIoLibIntrinsic library and expands the IoLib class
> to include the ported I/O Fifo routines.
> 
> The Fifo routines moved from the UefiCpuPkg/CpuIo2Dxe driver support
> IA32 and X64 natively, and other architectures are supported by simply
> looping through the basic IoRead/IoWrite routines as appropiate.
> 
> The intent of this patch series is twofold:
> 1) Integrate I/O Fifo routines into the IoLib class library.
> 2) Allow override of IoLib as may be required to support specific hardware
> implementations, such as AMD's Secure Encrypted Virtualization (SEV).
> 
> Leo Duran (8):
>   MdePkg: Expand BaseIoLibIntrinsic (IoLib class) library
>   MdePkg/DxeIoLibCpuIo2: Add new Fifo routines in IoLib class
>   MdePkg/DxeIoLibEsal: Add new Fifo routines in IoLib class
>   MdePkg/PeiIoLibCpuIo: Add new Fifo routines in IoLib class
>   MdePkg/SmmIoLibSmmCpuIo2: Add new Fifo routines in IoLib class
>   IntelFrameworkPkg/DxeIoLibCpuIo: Add new Fifo routines in IoLib class
>   UefiCpuPkg: Modify CpuIo2Dxe to use new IoLib class library
>   OvmfPkg: Modify QemuFwCfgLib to use new IoLib class library
> 
>  IntelFrameworkPkg/Library/DxeIoLibCpuIo/IoLib.c    | 203
> +++++++++++++++++++++
>  MdePkg/Include/Library/IoLib.h                     | 158 ++++++++++++++++
>  .../BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf      |   6 +-
>  .../Library/BaseIoLibIntrinsic}/Ia32/IoFifo.asm    |   1 +
>  .../Library/BaseIoLibIntrinsic}/Ia32/IoFifo.nasm   |   1 +
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c       | 182
> ++++++++++++++++++
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibEbc.c       | 179
> ++++++++++++++++++
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibIpf.c       | 201
> ++++++++++++++++++++
>  .../Library/BaseIoLibIntrinsic}/X64/IoFifo.asm     |   1 +
>  .../Library/BaseIoLibIntrinsic}/X64/IoFifo.nasm    |   1 +
>  MdePkg/Library/DxeIoLibCpuIo2/IoLib.c              | 203
> +++++++++++++++++++++
>  MdePkg/Library/DxeIoLibEsal/IoLib.c                | 203
> +++++++++++++++++++++
>  MdePkg/Library/PeiIoLibCpuIo/IoLib.c               | 203
> +++++++++++++++++++++
>  MdePkg/Library/SmmIoLibSmmCpuIo2/IoLib.c           | 203
> +++++++++++++++++++++
>  OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm  |  55 ------
>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c        |  54 +-----
>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf      |   7 +-
>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf   |   7 +-
>  OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm   |  52 ------
>  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c                   |   3 +-
>  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf                 |  11 +-
>  UefiCpuPkg/CpuIo2Dxe/IoFifo.h                      | 176 ------------------
>  22 files changed, 1751 insertions(+), 359 deletions(-)  rename
> {UefiCpuPkg/CpuIo2Dxe =>
> MdePkg/Library/BaseIoLibIntrinsic}/Ia32/IoFifo.asm (94%)  rename
> {UefiCpuPkg/CpuIo2Dxe =>
> MdePkg/Library/BaseIoLibIntrinsic}/Ia32/IoFifo.nasm (94%)  rename
> {UefiCpuPkg/CpuIo2Dxe =>
> MdePkg/Library/BaseIoLibIntrinsic}/X64/IoFifo.asm (95%)  rename
> {UefiCpuPkg/CpuIo2Dxe =>
> MdePkg/Library/BaseIoLibIntrinsic}/X64/IoFifo.nasm (95%)  delete mode
> 100644 OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm
>  delete mode 100644
> OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm
>  delete mode 100644 UefiCpuPkg/CpuIo2Dxe/IoFifo.h
> 
> --
> 1.9.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2017-01-11  4:06 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-10 23:55 [PATCH 0/8] IoLib class library Leo Duran
2017-01-10 23:55 ` [PATCH 1/8] MdePkg: Expand BaseIoLibIntrinsic (IoLib class) library Leo Duran
2017-01-12  1:33   ` Fan, Jeff
2017-01-12  1:36     ` Duran, Leo
2017-01-10 23:55 ` [PATCH 2/8] MdePkg/DxeIoLibCpuIo2: Add new Fifo routines in IoLib class Leo Duran
2017-01-10 23:55 ` [PATCH 3/8] MdePkg/DxeIoLibEsal: " Leo Duran
2017-01-10 23:55 ` [PATCH 4/8] MdePkg/PeiIoLibCpuIo: " Leo Duran
2017-01-10 23:55 ` [PATCH 5/8] MdePkg/SmmIoLibSmmCpuIo2: " Leo Duran
2017-01-10 23:55 ` [PATCH 6/8] IntelFrameworkPkg/DxeIoLibCpuIo: " Leo Duran
2017-01-10 23:55 ` [PATCH 7/8] UefiCpuPkg: Modify CpuIo2Dxe to use new IoLib class library Leo Duran
2017-01-10 23:55 ` [PATCH 8/8] OvmfPkg: Modify QemuFwCfgLib " Leo Duran
2017-01-11  1:41 ` [PATCH 0/8] " Gao, Liming
2017-01-11  1:45   ` Fan, Jeff
2017-01-11  1:52     ` Gao, Liming
2017-01-11  3:37   ` Duran, Leo
2017-01-11  4:06     ` Gao, Liming [this message]
2017-01-11 16:29       ` Duran, Leo
2017-01-12  1:22         ` Gao, Liming
2017-01-12  1:34           ` Duran, Leo
2017-01-12  1:39             ` Gao, Liming
2017-01-12  1:51               ` Duran, Leo
2017-01-12  4:25                 ` Gao, Liming
2017-01-12 17:13                   ` Jordan Justen
2017-01-17  2:50                   ` Gao, Liming
2017-01-12  0:25       ` Duran, Leo

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=4A89E2EF3DFEDB4C8BFDE51014F606A14D6CD269@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