From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 71997780091 for ; Wed, 20 Dec 2023 07:41:24 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=MeoeSaKsYGZP9px2qWjY38YxhECoh4hnyO+I9sC0BRs=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1703058083; v=1; b=Eq7VcAGFRbu6StDQTuXPGU1Gv9ZzRATZhy+7cXkObomLaW71uFThiLgyNMSh7iC4bh/NeFqX dx/WQj7+mCpjF7YVFA8PIm/2I8icDS4dlnJHsyx33Fjke20qJoMat2ASaMJvwyQYQPWyglewEob pzyV9b1DV2cMVqjt2SfYHtus= X-Received: by 127.0.0.2 with SMTP id K7GYYY7687511xuxkpgrqXFd; Tue, 19 Dec 2023 23:41:23 -0800 X-Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web10.17244.1703058082069336496 for ; Tue, 19 Dec 2023 23:41:22 -0800 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by ams.source.kernel.org (Postfix) with ESMTP id C3B18B81BD9 for ; Wed, 20 Dec 2023 07:41:19 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 27107C433D9 for ; Wed, 20 Dec 2023 07:41:19 +0000 (UTC) X-Received: by mail-lf1-f43.google.com with SMTP id 2adb3069b0e04-50e39ac39bcso4100228e87.3 for ; Tue, 19 Dec 2023 23:41:19 -0800 (PST) X-Gm-Message-State: FFZrh7Su1JkT6FnlKIoz1P0lx7686176AA= X-Google-Smtp-Source: AGHT+IH3Sji6dN/Df+jUf/ly0A7b7O02EkNIanuDd6jqUcJ/DuxRUhUB2CweFv9olk9DFdrBEIyUH5F7SgklKedYUoE= X-Received: by 2002:a05:6512:1045:b0:50e:15db:5e36 with SMTP id c5-20020a056512104500b0050e15db5e36mr3264370lfb.32.1703058077284; Tue, 19 Dec 2023 23:41:17 -0800 (PST) MIME-Version: 1.0 References: <20231212130932.2467028-1-lichao@loongson.cn> <17A017C201FEB90D.32321@groups.io> <9014a7b3-095c-42b1-a9ef-5a388818385d@loongson.cn> In-Reply-To: From: "Ard Biesheuvel" Date: Wed, 20 Dec 2023 08:41:06 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v4 19/37] UefiCpuPkg: Add MMIO method in CpuIo2Dxe To: devel@edk2.groups.io, ray.ni@intel.com Cc: "lichao@loongson.cn" , "Kumar, Rahul R" , Gerd Hoffmann , Leif Lindholm , Ard Biesheuvel , Sami Mujawar Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=Eq7VcAGF; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Wed, 20 Dec 2023 at 02:57, Ni, Ray wrote: > > It=E2=80=99s new to me that PCI IO (not MMIO) space is accessed through M= MIO. > > PCIE spec defines different TLP types for Memory and I/O request in Trans= action Layer chapter. > > If CpuIoDxe driver issues Memory request to a IO space inside a PCIE devi= ce, how does PCIE device claim the TPL packet and response? > Hello Ray, On the opposite side of the PCI host bridge, these are all port I/O transactions, and the endpoint is not aware of the distinction between native port I/O and translated port I/O. ARM CPUs do not implement port I/O at all, and so every host bridge that implements port I/O support on the PCI side does so by exposing a special MMIO resource window in the CPU physical address space that gets translated to port I/O accesses at the opposite side. This means that an implementation of the CpuIo2 protocol can only be provided in a meaningful way if there are port I/O capable PCI host bridges in the system, and some bookkeeping is needed to keep track of the mapping between the special MMIO ranges on the CPU side and the port I/O ranges on the PCI side. Note that this is not so different from MMIO translation, where the mapping between MMIO is not 1:1 between CPU and PCI. That said, I am not sure I follow why PcdPciIoTranslationIsEnabled is needed. MMIO translation and IO translation are both properties of the PCI host bridge implementation, so having a system wide PCD for this seems unnecessary to me. But perhaps I missed something? > > From: Chao Li > Sent: Tuesday, December 19, 2023 9:04 PM > To: devel@edk2.groups.io; Ni, Ray > Cc: Kumar, Rahul R ; Gerd Hoffmann ; Leif Lindholm ; Ard Biesheuvel ; Sami Mujawar > Subject: Re: [edk2-devel] [PATCH v4 19/37] UefiCpuPkg: Add MMIO method in= CpuIo2Dxe > > > > 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=3D4584 > > > > Cc: Ray Ni > > Cc: Rahul Kumar > > Cc: Gerd Hoffmann > > Cc: Leif Lindholm > > Cc: Ard Biesheuvel > > Cc: Sami Mujawar > > Signed-off-by: Chao Li > > --- > > 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/CpuI= o2Dxe.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.
> > Copyright (c) 2017, AMD Incorporated. All rights reserved.
> > +Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > +Copyright (c) 2023 Loongson Technology Corporation Limited. All rights r= eserved.
> > > > 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 ra= nge > > - // 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 tha= t > > // 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 =3D CpuIoCheckParameter (FALSE, Width, Address, Count, Buffer); > > if (EFI_ERROR (Status)) { > > return Status; > > @@ -410,37 +424,48 @@ CpuIoServiceRead ( > > OutStride =3D mOutStride[Width]; > > OperationWidth =3D (EFI_CPU_IO_PROTOCOL_WIDTH)(Width & 0x03); > > > > - // > > - // Fifo operations supported for (mInStride[Width] =3D=3D 0) > > - // > > - if (InStride =3D=3D 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) =3D=3D FALSE) { > > + // > > + // Fifo operations supported for (mInStride[Width] =3D=3D 0) > > + // > > + if (InStride =3D=3D 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 =3D IoRead8; > > + CpuIoRead16 =3D IoRead16; > > + CpuIoRead32 =3D IoRead32; > > + } else { > > + Address +=3D PcdGet64 (PcdPciIoTranslation); > > + CpuIoRead8 =3D MmioRead8; > > + CpuIoRead16 =3D MmioRead16; > > + CpuIoRead32 =3D MmioRead32; > > } > > > > for (Uint8Buffer =3D Buffer; Count > 0; Address +=3D InStride, Uint8Bu= ffer +=3D OutStride, Count--) { > > if (OperationWidth =3D=3D EfiCpuIoWidthUint8) { > > - *Uint8Buffer =3D IoRead8 ((UINTN)Address); > > + *Uint8Buffer =3D CpuIoRead8 ((UINTN)Address); > > } else if (OperationWidth =3D=3D EfiCpuIoWidthUint16) { > > - *((UINT16 *)Uint8Buffer) =3D IoRead16 ((UINTN)Address); > > + *((UINT16 *)Uint8Buffer) =3D CpuIoRead16 ((UINTN)Address); > > } else if (OperationWidth =3D=3D EfiCpuIoWidthUint32) { > > - *((UINT32 *)Uint8Buffer) =3D IoRead32 ((UINTN)Address); > > + *((UINT32 *)Uint8Buffer) =3D 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 =3D mOutStride[Width]; > > OperationWidth =3D (EFI_CPU_IO_PROTOCOL_WIDTH)(Width & 0x03); > > > > - // > > - // Fifo operations supported for (mInStride[Width] =3D=3D 0) > > - // > > - if (InStride =3D=3D 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) =3D=3D FALSE) { > > + // > > + // Fifo operations supported for (mInStride[Width] =3D=3D 0) > > + // > > + if (InStride =3D=3D 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 =3D IoWrite8; > > + CpuIoWrite16 =3D IoWrite16; > > + CpuIoWrite32 =3D IoWrite32; > > + } else { > > + Address +=3D PcdGet64 (PcdPciIoTranslation); > > + CpuIoWrite8 =3D MmioWrite8; > > + CpuIoWrite16 =3D MmioWrite16; > > + CpuIoWrite32 =3D MmioWrite32; > > } > > > > for (Uint8Buffer =3D (UINT8 *)Buffer; Count > 0; Address +=3D InStride= , Uint8Buffer +=3D OutStride, Count--) { > > if (OperationWidth =3D=3D EfiCpuIoWidthUint8) { > > - IoWrite8 ((UINTN)Address, *Uint8Buffer); > > + CpuIoWrite8 ((UINTN)Address, *Uint8Buffer); > > } else if (OperationWidth =3D=3D EfiCpuIoWidthUint16) { > > - IoWrite16 ((UINTN)Address, *((UINT16 *)Uint8Buffer)); > > + CpuIoWrite16 ((UINTN)Address, *((UINT16 *)Uint8Buffer)); > > } else if (OperationWidth =3D=3D EfiCpuIoWidthUint32) { > > - IoWrite32 ((UINTN)Address, *((UINT32 *)Uint8Buffer)); > > + CpuIoWrite32 ((UINTN)Address, *((UINT32 *)Uint8Buffer)); > > } > > } > > > > diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h b/UefiCpuPkg/CpuIo2Dxe/CpuI= o2Dxe.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.
> > +Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > +Copyright (c) 2023 Loongson Technology Corporation Limited. All rights r= eserved.
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf b/UefiCpuPkg/CpuIo2Dxe/Cp= uIo2Dxe.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.
> > # Copyright (c) 2017, AMD Incorporated. All rights reserved.
> > +# Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > +# Copyright (c) 2023 Loongson Technology Corporation Limited. All rights= reserved.
> > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -20,7 +22,7 @@ > > # > > # The following information is for reference only and not required by th= e build tools. > > # > > -# VALID_ARCHITECTURES =3D IA32 X64 EBC > > +# VALID_ARCHITECTURES =3D IA32 X64 EBC ARM AARCH64 LOONGARCH6= 4 > > # > > > > [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/Cp= uIo2Dxe.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 Libr= ary. > > // > > // Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved. > > +// Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > +// Copyright (c) 2023 Loongson Technology Corporation Limited. All right= s reserved.
> > // > > // SPDX-License-Identifier: BSD-2-Clause-Patent > > // > >=20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112749): https://edk2.groups.io/g/devel/message/112749 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-