Re: [edk2-devel] [PATCH 2/2] MdePkg:simplify Fifo API in BaseIoLibIntrinsic

["Michael D Kinney" <michael.d.kinney@intel.com>]

Acked-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Wednesday, December 6, 2023 1:26 AM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Ni,
> Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [PATCH 2/2] MdePkg:simplify Fifo API in
> BaseIoLibIntrinsic
> 
> Hi Mike and Liming,
> 
> Could you please help to review this patch?
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Thursday, November 9, 2023 10:50 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Ni,
> Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH 2/2] MdePkg:simplify Fifo API in
> BaseIoLibIntrinsic
> 
> Simplify IoRead/WriteFifo implement by repeatedly calling IoRead/Write
> in the C code.
> This can avoid calling assembly code to use string I/O instructions.
> With this change Ia32/IoFifo.nasm and X64/IoFifo.nasm can be removed.
> Then the source files for IA32 and X64 are the same.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> ---
>  MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf |  10 ++------
> --
>  MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm       | 131 --------
> ------------------------------------------------------------------------
> ---------------------------------------------------
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c            | 220
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++
>  MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm        | 120 --------
> ------------------------------------------------------------------------
> ----------------------------------------
>  4 files changed, 222 insertions(+), 259 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> index aeb072ee95..b587e2cded 100644
> --- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> @@ -38,17 +38,11 @@
>    IoLibInternalTdxNull.c
>    IoLibTdx.h
> 
> -[Sources.IA32]
> +[Sources.IA32, Sources.X64]
>    IoLibGcc.c    | GCC
>    IoLibMsc.c    | MSFT
>    IoLib.c
> -  Ia32/IoFifo.nasm
> -
> -[Sources.X64]
> -  IoLibGcc.c    | GCC
> -  IoLibMsc.c    | MSFT
> -  IoLib.c
> -  X64/IoFifo.nasm
> +  IoLibFifo.c
> 
>  [Sources.EBC]
>    IoLibEbc.c
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> deleted file mode 100644
> index a4ae1a0053..0000000000
> --- a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> +++ /dev/null
> @@ -1,131 +0,0 @@
> -;----------------------------------------------------------------------
> --------
> -;
> -; Copyright (c) 2006 - 2012, Intel Corporation. All rights
> reserved.<BR> -; Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR> -; -; SPDX-License-Identifier: BSD-2-Clause-Patent -;
> -;----------------------------------------------------------------------
> --------
> -
> -    SECTION .text
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoReadFifo8 (
> -;    IN  UINTN                 Port,
> -;    IN  UINTN                 Size,
> -;    OUT VOID                  *Buffer
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoReadFifo8)
> -ASM_PFX(IoReadFifo8):
> -    push    edi
> -    cld
> -    mov     dx, [esp + 8]
> -    mov     ecx, [esp + 12]
> -    mov     edi, [esp + 16]
> -rep insb
> -    pop     edi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoReadFifo16 (
> -;    IN  UINTN                 Port,
> -;    IN  UINTN                 Size,
> -;    OUT VOID                  *Buffer
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoReadFifo16)
> -ASM_PFX(IoReadFifo16):
> -    push    edi
> -    cld
> -    mov     dx, [esp + 8]
> -    mov     ecx, [esp + 12]
> -    mov     edi, [esp + 16]
> -rep insw
> -    pop     edi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoReadFifo32 (
> -;    IN  UINTN                 Port,
> -;    IN  UINTN                 Size,
> -;    OUT VOID                  *Buffer
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoReadFifo32)
> -ASM_PFX(IoReadFifo32):
> -    push    edi
> -    cld
> -    mov     dx, [esp + 8]
> -    mov     ecx, [esp + 12]
> -    mov     edi, [esp + 16]
> -rep insd
> -    pop     edi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoWriteFifo8 (
> -;    IN UINTN                  Port,
> -;    IN UINTN                  Size,
> -;    IN VOID                   *Buffer
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoWriteFifo8)
> -ASM_PFX(IoWriteFifo8):
> -    push    esi
> -    cld
> -    mov     dx, [esp + 8]
> -    mov     ecx, [esp + 12]
> -    mov     esi, [esp + 16]
> -rep outsb
> -    pop     esi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoWriteFifo16 (
> -;    IN UINTN                  Port,
> -;    IN UINTN                  Size,
> -;    IN VOID                   *Buffer
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoWriteFifo16)
> -ASM_PFX(IoWriteFifo16):
> -    push    esi
> -    cld
> -    mov     dx, [esp + 8]
> -    mov     ecx, [esp + 12]
> -    mov     esi, [esp + 16]
> -rep outsw
> -    pop     esi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoWriteFifo32 (
> -;    IN UINTN                  Port,
> -;    IN UINTN                  Size,
> -;    IN VOID                   *Buffer
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoWriteFifo32)
> -ASM_PFX(IoWriteFifo32):
> -    push    esi
> -    cld
> -    mov     dx, [esp + 8]
> -    mov     ecx, [esp + 12]
> -    mov     esi, [esp + 16]
> -rep outsd
> -    pop     esi
> -    ret
> -
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
> b/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
> new file mode 100644
> index 0000000000..bd1d372d0f
> --- /dev/null
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
> @@ -0,0 +1,220 @@
> +/** @file
> +  IoFifo read/write routines.
> +
> +  Copyright (c) 2021 - 2023, Intel Corporation. All rights
> + reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "BaseIoLibIntrinsicInternal.h"
> +#include <Uefi/UefiBaseType.h>
> +
> +/**
> +  Reads an 8-bit I/O port fifo into a block of memory.
> +
> +  Reads the 8-bit I/O fifo port specified by Port.
> +  The port is read Count times, and the read data is  stored in the
> + provided Buffer.
> +
> +  This function must guarantee that all I/O read and write operations
> + are  serialized.
> +
> +  If 8-bit I/O port operations are not supported, then ASSERT().
> +
> +  In TDX a serial of TdIoRead8 is invoked to read the I/O port fifo.
> +
> +  @param  Port    The I/O port to read.
> +  @param  Count   The number of times to read I/O port.
> +  @param  Buffer  The buffer to store the read data into.
> +
> +**/
> +VOID
> +EFIAPI
> +IoReadFifo8 (
> +  IN      UINTN  Port,
> +  IN      UINTN  Count,
> +  OUT     VOID   *Buffer
> +  )
> +{
> +  UINT8  *Buffer8;
> +
> +  Buffer8 = (UINT8 *)Buffer;
> +  while (Count-- > 0) {
> +    *Buffer8++ = IoRead8 (Port);
> +  }
> +}
> +
> +/**
> +  Writes a block of memory into an 8-bit I/O port fifo.
> +
> +  Writes the 8-bit I/O fifo port specified by Port.
> +  The port is written Count times, and the write data is  retrieved
> + from the provided Buffer.
> +
> +  This function must guarantee that all I/O write and write operations
> + are  serialized.
> +
> +  If 8-bit I/O port operations are not supported, then ASSERT().
> +
> +  In TDX a serial of TdIoWrite8 is invoked to write data to the I/O
> port.
> +
> +  @param  Port    The I/O port to write.
> +  @param  Count   The number of times to write I/O port.
> +  @param  Buffer  The buffer to retrieve the write data from.
> +
> +**/
> +VOID
> +EFIAPI
> +IoWriteFifo8 (
> +  IN      UINTN  Port,
> +  IN      UINTN  Count,
> +  IN      VOID   *Buffer
> +  )
> +{
> +  UINT8  *Buffer8;
> +
> +  Buffer8 = (UINT8 *)Buffer;
> +  while (Count-- > 0) {
> +    IoWrite8 (Port, *Buffer8++);
> +  }
> +}
> +
> +/**
> +  Reads a 16-bit I/O port fifo into a block of memory.
> +
> +  Reads the 16-bit I/O fifo port specified by Port.
> +  The port is read Count times, and the read data is  stored in the
> + provided Buffer.
> +
> +  This function must guarantee that all I/O read and write operations
> + are  serialized.
> +
> +  If 16-bit I/O port operations are not supported, then ASSERT().
> +
> +  In TDX a serial of TdIoRead16 is invoked to read data from the I/O
> port.
> +
> +  @param  Port    The I/O port to read.
> +  @param  Count   The number of times to read I/O port.
> +  @param  Buffer  The buffer to store the read data into.
> +
> +**/
> +VOID
> +EFIAPI
> +IoReadFifo16 (
> +  IN      UINTN  Port,
> +  IN      UINTN  Count,
> +  OUT     VOID   *Buffer
> +  )
> +{
> +  UINT16  *Buffer16;
> +
> +  Buffer16 = (UINT16 *)Buffer;
> +  while (Count-- > 0) {
> +    *Buffer16++ = IoRead16 (Port);
> +  }
> +}
> +
> +/**
> +  Writes a block of memory into a 16-bit I/O port fifo.
> +
> +  Writes the 16-bit I/O fifo port specified by Port.
> +  The port is written Count times, and the write data is  retrieved
> + from the provided Buffer.
> +
> +  This function must guarantee that all I/O write and write operations
> + are  serialized.
> +
> +  If 16-bit I/O port operations are not supported, then ASSERT().
> +
> +  In TDX a serial of TdIoWrite16 is invoked to write data to the I/O
> port.
> +
> +  @param  Port    The I/O port to write.
> +  @param  Count   The number of times to write I/O port.
> +  @param  Buffer  The buffer to retrieve the write data from.
> +
> +**/
> +VOID
> +EFIAPI
> +IoWriteFifo16 (
> +  IN      UINTN  Port,
> +  IN      UINTN  Count,
> +  IN      VOID   *Buffer
> +  )
> +{
> +  UINT16  *Buffer16;
> +
> +  Buffer16 = (UINT16 *)Buffer;
> +  while (Count-- > 0) {
> +    IoWrite16 (Port, *Buffer16++);
> +  }
> +}
> +
> +/**
> +  Reads a 32-bit I/O port fifo into a block of memory.
> +
> +  Reads the 32-bit I/O fifo port specified by Port.
> +  The port is read Count times, and the read data is  stored in the
> + provided Buffer.
> +
> +  This function must guarantee that all I/O read and write operations
> + are  serialized.
> +
> +  If 32-bit I/O port operations are not supported, then ASSERT().
> +
> +  In TDX a serial of TdIoRead32 is invoked to read data from the I/O
> port.
> +
> +  @param  Port    The I/O port to read.
> +  @param  Count   The number of times to read I/O port.
> +  @param  Buffer  The buffer to store the read data into.
> +
> +**/
> +VOID
> +EFIAPI
> +IoReadFifo32 (
> +  IN      UINTN  Port,
> +  IN      UINTN  Count,
> +  OUT     VOID   *Buffer
> +  )
> +{
> +  UINT32  *Buffer32;
> +
> +  Buffer32 = (UINT32 *)Buffer;
> +  while (Count-- > 0) {
> +    *Buffer32++ = IoRead32 (Port);
> +  }
> +}
> +
> +/**
> +  Writes a block of memory into a 32-bit I/O port fifo.
> +
> +  Writes the 32-bit I/O fifo port specified by Port.
> +  The port is written Count times, and the write data is  retrieved
> + from the provided Buffer.
> +
> +  This function must guarantee that all I/O write and write operations
> + are  serialized.
> +
> +  If 32-bit I/O port operations are not supported, then ASSERT().
> +
> +  In TDX a serial of TdIoWrite32 is invoked to write data to the I/O
> port.
> +
> +  @param  Port    The I/O port to write.
> +  @param  Count   The number of times to write I/O port.
> +  @param  Buffer  The buffer to retrieve the write data from.
> +
> +**/
> +VOID
> +EFIAPI
> +IoWriteFifo32 (
> +  IN      UINTN  Port,
> +  IN      UINTN  Count,
> +  IN      VOID   *Buffer
> +  )
> +{
> +  UINT32  *Buffer32;
> +
> +  Buffer32 = (UINT32 *)Buffer;
> +  while (Count-- > 0) {
> +    IoWrite32 (Port, *Buffer32++);
> +  }
> +}
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> b/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> deleted file mode 100644
> index d459072f6e..0000000000
> --- a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> +++ /dev/null
> @@ -1,120 +0,0 @@
> -;----------------------------------------------------------------------
> --------
> -;
> -; Copyright (c) 2006 - 2012, Intel Corporation. All rights
> reserved.<BR> -; Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR> -; -; SPDX-License-Identifier: BSD-2-Clause-Patent -;
> -;----------------------------------------------------------------------
> --------
> -
> -    DEFAULT REL
> -    SECTION .text
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoReadFifo8 (
> -;    IN  UINTN                 Port,              // rcx
> -;    IN  UINTN                 Size,              // rdx
> -;    OUT VOID                  *Buffer            // r8
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoReadFifo8)
> -ASM_PFX(IoReadFifo8):
> -    cld
> -    xchg    rcx, rdx
> -    xchg    rdi, r8             ; rdi: buffer address; r8: save rdi
> -rep insb
> -    mov     rdi, r8             ; restore rdi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoReadFifo16 (
> -;    IN  UINTN                 Port,              // rcx
> -;    IN  UINTN                 Size,              // rdx
> -;    OUT VOID                  *Buffer            // r8
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoReadFifo16)
> -ASM_PFX(IoReadFifo16):
> -    cld
> -    xchg    rcx, rdx
> -    xchg    rdi, r8             ; rdi: buffer address; r8: save rdi
> -rep insw
> -    mov     rdi, r8             ; restore rdi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoReadFifo32 (
> -;    IN  UINTN                 Port,              // rcx
> -;    IN  UINTN                 Size,              // rdx
> -;    OUT VOID                  *Buffer            // r8
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoReadFifo32)
> -ASM_PFX(IoReadFifo32):
> -    cld
> -    xchg    rcx, rdx
> -    xchg    rdi, r8             ; rdi: buffer address; r8: save rdi
> -rep insd
> -    mov     rdi, r8             ; restore rdi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoWriteFifo8 (
> -;    IN UINTN                  Port,              // rcx
> -;    IN UINTN                  Size,              // rdx
> -;    IN VOID                   *Buffer            // r8
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoWriteFifo8)
> -ASM_PFX(IoWriteFifo8):
> -    cld
> -    xchg    rcx, rdx
> -    xchg    rsi, r8             ; rsi: buffer address; r8: save rsi
> -rep outsb
> -    mov     rsi, r8             ; restore rsi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoWriteFifo16 (
> -;    IN UINTN                  Port,              // rcx
> -;    IN UINTN                  Size,              // rdx
> -;    IN VOID                   *Buffer            // r8
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoWriteFifo16)
> -ASM_PFX(IoWriteFifo16):
> -    cld
> -    xchg    rcx, rdx
> -    xchg    rsi, r8             ; rsi: buffer address; r8: save rsi
> -rep outsw
> -    mov     rsi, r8             ; restore rsi
> -    ret
> -
> -;----------------------------------------------------------------------
> --------
> -;  VOID
> -;  EFIAPI
> -;  IoWriteFifo32 (
> -;    IN UINTN                  Port,              // rcx
> -;    IN UINTN                  Size,              // rdx
> -;    IN VOID                   *Buffer            // r8
> -;    );
> -;----------------------------------------------------------------------
> --------
> -global ASM_PFX(IoWriteFifo32)
> -ASM_PFX(IoWriteFifo32):
> -    cld
> -    xchg    rcx, rdx
> -    xchg    rsi, r8             ; rsi: buffer address; r8: save rsi
> -rep outsd
> -    mov     rsi, r8             ; restore rsi
> -    ret
> -
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112324): https://edk2.groups.io/g/devel/message/112324
Mute This Topic: https://groups.io/mt/103009916/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-