Hi Ray,

Can you please review this patch? Thank you!


Thanks,
Chao
On 2023/12/12 21:12, Chao Li wrote:
> CpuIo2Dxe only supports IO to access PCI IO. Some ARCH requires
> MMIO to access PCI IO, add the MMIO access method in CpuIo2Dxe.
>
> The MMIO methods depend on PcdPciIoTranslationIsEnabled and
> PcdPciIoTransLation. The code is referenced from ArmPkg.
>
> Build-tested only (with "OvmfPkgX64.dsc").
>
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4584
>
> Cc: Ray Ni<ray.ni@intel.com>
> Cc: Rahul Kumar<rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann<kraxel@redhat.com>
> Cc: Leif Lindholm<quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel<ardb+tianocore@kernel.org>
> Cc: Sami Mujawar<sami.mujawar@arm.com>
> Signed-off-by: Chao Li<lichao@loongson.cn>
> ---
>   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c   | 149 +++++++++++++++++++----------
>   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h   |   2 +
>   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf |   8 +-
>   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.uni |   2 +
>   4 files changed, 111 insertions(+), 50 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c
> index 87f4f805ca..cd31977af2 100644
> --- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c
> +++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c
> @@ -3,6 +3,8 @@
>   
>   Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
>   Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> +Copyright (c) 2023 Loongson Technology Corporation Limited. All rights reserved.<BR>
>   
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>   
> @@ -149,7 +151,7 @@ CpuIoCheckParameter (
>     //
>     // Since MAX_ADDRESS can be the maximum integer value supported by the CPU and Count
>     // can also be the maximum integer value supported by the CPU, this range
> -  // check must be adjusted to avoid all oveflow conditions.
> +  // check must be adjusted to avoid all overflow conditions.
>     //
>     // The following form of the range check is equivalent but assumes that
>     // MAX_ADDRESS and MAX_IO_PORT_ADDRESS are of the form (2^n - 1).
> @@ -398,6 +400,18 @@ CpuIoServiceRead (
>     EFI_CPU_IO_PROTOCOL_WIDTH  OperationWidth;
>     UINT8                      *Uint8Buffer;
>   
> +  UINT8 EFIAPI  (*CpuIoRead8) (
> +    UINTN
> +    );
> +
> +  UINT16 EFIAPI  (*CpuIoRead16) (
> +    UINTN
> +    );
> +
> +  UINT32 EFIAPI  (*CpuIoRead32) (
> +    UINTN
> +    );
> +
>     Status = CpuIoCheckParameter (FALSE, Width, Address, Count, Buffer);
>     if (EFI_ERROR (Status)) {
>       return Status;
> @@ -410,37 +424,48 @@ CpuIoServiceRead (
>     OutStride      = mOutStride[Width];
>     OperationWidth = (EFI_CPU_IO_PROTOCOL_WIDTH)(Width & 0x03);
>   
> -  //
> -  // Fifo operations supported for (mInStride[Width] == 0)
> -  //
> -  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;
> +  if (FeaturePcdGet (PcdPciIoTranslationIsEnabled) == FALSE) {
> +    //
> +    // Fifo operations supported for (mInStride[Width] == 0)
> +    //
> +    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;
> +      }
>       }
> +
> +    CpuIoRead8  = IoRead8;
> +    CpuIoRead16 = IoRead16;
> +    CpuIoRead32 = IoRead32;
> +  } else {
> +    Address    += PcdGet64 (PcdPciIoTranslation);
> +    CpuIoRead8  = MmioRead8;
> +    CpuIoRead16 = MmioRead16;
> +    CpuIoRead32 = MmioRead32;
>     }
>   
>     for (Uint8Buffer = Buffer; Count > 0; Address += InStride, Uint8Buffer += OutStride, Count--) {
>       if (OperationWidth == EfiCpuIoWidthUint8) {
> -      *Uint8Buffer = IoRead8 ((UINTN)Address);
> +      *Uint8Buffer = CpuIoRead8 ((UINTN)Address);
>       } else if (OperationWidth == EfiCpuIoWidthUint16) {
> -      *((UINT16 *)Uint8Buffer) = IoRead16 ((UINTN)Address);
> +      *((UINT16 *)Uint8Buffer) = CpuIoRead16 ((UINTN)Address);
>       } else if (OperationWidth == EfiCpuIoWidthUint32) {
> -      *((UINT32 *)Uint8Buffer) = IoRead32 ((UINTN)Address);
> +      *((UINT32 *)Uint8Buffer) = CpuIoRead32 ((UINTN)Address);
>       }
>     }
>   
> @@ -502,6 +527,21 @@ CpuIoServiceWrite (
>     EFI_CPU_IO_PROTOCOL_WIDTH  OperationWidth;
>     UINT8                      *Uint8Buffer;
>   
> +  UINT8 EFIAPI  (*CpuIoWrite8) (
> +    UINTN,
> +    UINT8
> +    );
> +
> +  UINT16 EFIAPI  (*CpuIoWrite16) (
> +    UINTN,
> +    UINT16
> +    );
> +
> +  UINT32 EFIAPI  (*CpuIoWrite32) (
> +    UINTN,
> +    UINT32
> +    );
> +
>     //
>     // Make sure the parameters are valid
>     //
> @@ -517,37 +557,48 @@ CpuIoServiceWrite (
>     OutStride      = mOutStride[Width];
>     OperationWidth = (EFI_CPU_IO_PROTOCOL_WIDTH)(Width & 0x03);
>   
> -  //
> -  // Fifo operations supported for (mInStride[Width] == 0)
> -  //
> -  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;
> +  if (FeaturePcdGet (PcdPciIoTranslationIsEnabled) == FALSE) {
> +    //
> +    // Fifo operations supported for (mInStride[Width] == 0)
> +    //
> +    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;
> +      }
>       }
> +
> +    CpuIoWrite8  = IoWrite8;
> +    CpuIoWrite16 = IoWrite16;
> +    CpuIoWrite32 = IoWrite32;
> +  } else {
> +    Address     += PcdGet64 (PcdPciIoTranslation);
> +    CpuIoWrite8  = MmioWrite8;
> +    CpuIoWrite16 = MmioWrite16;
> +    CpuIoWrite32 = MmioWrite32;
>     }
>   
>     for (Uint8Buffer = (UINT8 *)Buffer; Count > 0; Address += InStride, Uint8Buffer += OutStride, Count--) {
>       if (OperationWidth == EfiCpuIoWidthUint8) {
> -      IoWrite8 ((UINTN)Address, *Uint8Buffer);
> +      CpuIoWrite8 ((UINTN)Address, *Uint8Buffer);
>       } else if (OperationWidth == EfiCpuIoWidthUint16) {
> -      IoWrite16 ((UINTN)Address, *((UINT16 *)Uint8Buffer));
> +      CpuIoWrite16 ((UINTN)Address, *((UINT16 *)Uint8Buffer));
>       } else if (OperationWidth == EfiCpuIoWidthUint32) {
> -      IoWrite32 ((UINTN)Address, *((UINT32 *)Uint8Buffer));
> +      CpuIoWrite32 ((UINTN)Address, *((UINT32 *)Uint8Buffer));
>       }
>     }
>   
> diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h
> index 7ebde0759b..5256a583e1 100644
> --- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h
> +++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h
> @@ -2,6 +2,8 @@
>     Internal include file for the CPU I/O 2 Protocol.
>   
>   Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> +Copyright (c) 2023 Loongson Technology Corporation Limited. All rights reserved.<BR>
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>   
>   **/
> diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
> index 499258491f..271c47371b 100644
> --- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
> +++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
> @@ -3,6 +3,8 @@
>   #
>   # Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
>   # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> +# Copyright (c) 2023 Loongson Technology Corporation Limited. All rights reserved.<BR>
>   #
>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -20,7 +22,7 @@
>   #
>   # The following information is for reference only and not required by the build tools.
>   #
> -#  VALID_ARCHITECTURES           = IA32 X64 EBC
> +#  VALID_ARCHITECTURES           = IA32 X64 EBC ARM AARCH64 LOONGARCH64
>   #
>   
>   [Sources]
> @@ -37,6 +39,10 @@
>     IoLib
>     UefiBootServicesTableLib
>   
> +[Pcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslationIsEnabled
> +  gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslation
> +
>   [Protocols]
>     gEfiCpuIo2ProtocolGuid                         ## PRODUCES
>   
> diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.uni b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.uni
> index 8d4e5dd6b4..14a36ff888 100644
> --- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.uni
> +++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.uni
> @@ -4,6 +4,8 @@
>   // Produces the CPU I/O 2 Protocol by using the services of the I/O Library.
>   //
>   // Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> +// Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> +// Copyright (c) 2023 Loongson Technology Corporation Limited. All rights reserved.<BR>
>   //
>   // SPDX-License-Identifier: BSD-2-Clause-Patent
>   //


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