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 C9AE8940EB6 for ; Thu, 21 Dec 2023 03:48:20 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Z0VL0DYLykY2+rWWXNps2Gq7CZ9o/R1JG5gm7TbET7Q=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1703130499; v=1; b=Sew8Woo67XpUjVHGun4d+P4HvGnFmF2+dCQwCFXhlg7+nr4p/ii+RYA597ZwQDd4tdw7EJtD Gz6Fdot36WUajtpxTCUX4qwxyCqSgxQzepFDHeT7minSzzz0Hl+EMeDJGhRjIPbDJVOml0hJIfu 0cRtaDKcBTBCB4wA2rvitDMY= X-Received: by 127.0.0.2 with SMTP id VcEbYY7687511xtT9hPK6HQZ; Wed, 20 Dec 2023 19:48:19 -0800 X-Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by mx.groups.io with SMTP id smtpd.web10.44586.1703130497647770416 for ; Wed, 20 Dec 2023 19:48:18 -0800 X-Received: from loongson.cn (unknown [10.40.24.149]) by gateway (Coremail) with SMTP id _____8DxVPB9tYNlGEUDAA--.16672S3; Thu, 21 Dec 2023 11:48:14 +0800 (CST) X-Received: from [10.40.24.149] (unknown [10.40.24.149]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Cxvr57tYNloxcDAA--.9435S3; Thu, 21 Dec 2023 11:48:11 +0800 (CST) Message-ID: Date: Thu, 21 Dec 2023 11:48:11 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v4 19/37] UefiCpuPkg: Add MMIO method in CpuIo2Dxe To: Ard Biesheuvel , devel@edk2.groups.io, ray.ni@intel.com Cc: "Kumar, Rahul R" , Gerd Hoffmann , Leif Lindholm , Ard Biesheuvel , Sami Mujawar References: <20231212130932.2467028-1-lichao@loongson.cn> <17A017C201FEB90D.32321@groups.io> <9014a7b3-095c-42b1-a9ef-5a388818385d@loongson.cn> From: "Chao Li" In-Reply-To: X-CM-TRANSID: AQAAf8Cxvr57tYNloxcDAA--.9435S3 X-CM-SenderInfo: xolfxt3r6o00pqjv00gofq/1tbiAQAMCGWDoK4BmAABs1 X-Coremail-Antispam: 1Uk129KBj9fXoW3Zr4DCr1xAFWxuF48Ar17CFX_yoW8JrW3Ko W8W3WxWwn8Aws5G34rZryfJ3W7XF9Fgrn8Jr4jy340g3yFvrnxuas5Xa4UZ34rXw4q9ayr JFyUZ3ySqrZ3J34rl-sFpf9Il3svdjkaLaAFLSUrUUUUbb8apTn2vfkv8UJUUUU8wcxFpf 9Il3svdxBIdaVrnUUv73VFW2AGmfu7bjvjm3AaLaJ3UjIYCTnIWjp_UUU5_7kC6x804xWl 14x267AKxVWUJVW8JwAFc2x0x2IEx4CE42xK8VAvwI8IcIk0rVWrJVCq3wAFIxvE14AKwV WUJVWUGwA2ocxC64kIII0Yj41l84x0c7CEw4AK67xGY2AK021l84ACjcxK6xIIjxv20xvE 14v26r1j6r1xM28EF7xvwVC0I7IYx2IY6xkF7I0E14v26r1j6r4UM28EF7xvwVC2z280aV AFwI0_Cr0_Gr1UM28EF7xvwVC2z280aVCY1x0267AKxVW8Jr0_Cr1UM2AIxVAIcxkEcVAq 07x20xvEncxIr21l57IF6xkI12xvs2x26I8E6xACxx1lYx0E2Ix0cI8IcVAFwI0_Jrv_JF 1lYx0Ex4A2jsIE14v26r1j6r4UMcvjeVCFs4IE7xkEbVWUJVW8JwACjcxG0xvEwIxGrwCj r7xvwVCIw2I0I7xG6c02F41l42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr 1lx2IqxVAqx4xG67AKxVWUGVWUWwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE 14v26r1q6r43MIIYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7 IYx2IY6xkF7I0E14v26r1j6r4UMIIF0xvE42xK8VAvwI8IcIk0rVWUJVWUCwCI42IY6I8E 87Iv67AKxVWUJVW8JwCI42IY6I8E87Iv6xkF7I0E14v26r1j6r4UYxBIdaVFxhVjvjDU0x ZFpf9x07ULdb8UUUUU= 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,lichao@loongson.cn List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: przGlRumlZ9aRHcKD6H6zYJyx7686176AA= Content-Type: multipart/alternative; boundary="------------MxGRqHnwBLymvuVLVmZIPtmF" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=Sew8Woo6; dmarc=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 --------------MxGRqHnwBLymvuVLVmZIPtmF Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit  Hi Ray and Ard, I went out yesterday, sorry for the late reply. Thanks, Chao On 2023/12/20 15:41, Ard Biesheuvel wrote: > On Wed, 20 Dec 2023 at 02:57, Ni, Ray wrote: >> It’s new to me that PCI IO (not MMIO) space is accessed through MMIO. >> >> PCIE spec defines different TLP types for Memory and I/O request in Transaction Layer chapter. >> >> If CpuIoDxe driver issues Memory request to a IO space inside a PCIE device, 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. Ray, As Ard said, LoongArch also do not implement prot I/O, the MMIO method can covert the addresses to PCI I/O addresses. Most the unified addressing architecture do not have the IO instructions, and when they use the IO device, most of them sit under the PCIe or PCI bus, just like LPC or eSPI, so they will use the MMIO to access them. > > 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? Ard, PcdPciIoTranslationIsEnabled is only use for whether to trigger the Ffio read or write, it seem that only x86 or x64 need them, not others. When I was submitted the patch V2, CpuIo2Dxe was private to LoongArch, just like Arm and RISC-V. Gerd recommended finding a better place for ArmPciCpuIo2Dxe so that other ARCH can easily use it. And then I found the UefiCpuPkg/CpuIo2Dxe might be able to accommodate the MMIO methods, so I merged them togeter in this change. > > > >> 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=4584 >> >> >> >> 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/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.
>> >> 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 >> >> >> >> @@ -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.
>> >> +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 >> >> >> >> **/ >> >> 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.
>> >> # 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 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.
>> >> +// 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 >> >> // >> >> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112800): https://edk2.groups.io/g/devel/message/112800 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] -=-=-=-=-=-=-=-=-=-=-=- --------------MxGRqHnwBLymvuVLVmZIPtmF Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

 Hi Ray and Ard,

I went out yesterday, sorry for the late reply.


Thanks,
Chao
On 2023/12/20 15:41, Ard Biesheuvel wrote:
On Wed, 20 Dec 2023 at 02:57, Ni, Ray <ray.ni@intel.com> wrote:

It’s new to me that PCI IO (not MMIO) space is accessed through MMIO.

PCIE spec defines different TLP types for Memory and I/O request in Transaction Layer chapter.

If CpuIoDxe driver issues Memory request to a IO space inside a PCIE device, 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.

Ray,

As Ard said, LoongArch also do not implement prot I/O, the MMIO method can covert the addresses to PCI I/O addresses.

Most the unified addressing architecture do not have the IO instructions, and when they use the IO device, most of them sit under the PCIe or PCI bus, just like LPC or eSPI, so they will use the MMIO to access them.


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?

Ard,

PcdPciIoTranslationIsEnabled is only use for whether to trigger the Ffio read or write, it seem that only x86 or x64 need them, not others.

When I was submitted the patch V2, CpuIo2Dxe was private to LoongArch, just like Arm and RISC-V. Gerd recommended finding a better place for ArmPciCpuIo2Dxe so that other ARCH can easily use it. And then I found the UefiCpuPkg/CpuIo2Dxe might be able to accommodate the MMIO methods, so I merged them togeter in this change.





From: Chao Li <lichao@loongson.cn>
Sent: Tuesday, December 19, 2023 9:04 PM
To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
Cc: Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Leif Lindholm <quic_llindhol@quicinc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar <sami.mujawar@arm.com>
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=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>
_._,_._,_
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#112800) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------MxGRqHnwBLymvuVLVmZIPtmF--