On Thu, Dec 7, 2023 at 11:36 PM Andrei Warkentin wrote: > Hi Tuan, > > > > I noticed that the OvmfPkg RV Sec uses PopulateIoResources by adding > entries to GCD of type EFI_RESOURCE_MEMORY_MAPPED_IO. Contrast this with > edk2-platforms/Platforms/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.c, > which adds these as memory and then allocates them away as > EfiReservedMemoryType. I remember this came up during the upstreaming of > the Raspberry Pi port… > > > > Anything we add as MMIO will end up growing the Runtime Services mappings, > as MMIO are specifically non-memory mappings that need to be present during > OS use of RT services. It’s probably a good idea to avoid using MMIO > regions for all I/O used by Boot Services. > > > Agree. Will post a patch to fix it. > A > > > > > > *From:* Tuan Phan > *Sent:* Thursday, June 22, 2023 3:28 PM > *To:* devel@edk2.groups.io; tphan@ventanamicro.com > *Cc:* Ard Biesheuvel ; Kinney, Michael D < > michael.d.kinney@intel.com>; Gao, Liming ; Liu, > Zhiguang ; sunilvl@ventanamicro.com; > git@danielschaefer.me; Warkentin, Andrei > *Subject:* Re: [edk2-devel] [PATCH v3 7/7] OvmfPkg/RiscVVirt: SEC: Add IO > memory resource hob for platform devices > > > > > > > > On Thu, Jun 22, 2023 at 11:41 AM Tuan Phan wrote: > > > > > > On Tue, May 30, 2023 at 10:38 AM Tuan Phan via groups.io ventanamicro.com@groups.io> wrote: > > > > > > On Mon, May 29, 2023 at 7:07 AM Ard Biesheuvel wrote: > > On Sat, 27 May 2023 at 01:18, Tuan Phan wrote: > > > > Normally, DXE driver would add device resource to GCD before start using. > > But some key resources such as uart, flash base address are being > accessing > > directly in some core modules. > > > > Those resources should be populated to HOB in SEC phase so they are > > added to GCD before anyone can access them. > > > > Why should these be in the GCD to begin with? > > > > These resources should be in memory space so their addresses and size are > registered with MMU. If not when MMU enabled, illegal access exception when > someone access them. > > > > Hi Ard, > > Do you still have concerns about this patch? > > BTW, I will drop this patch and put VirtNorFlashDxe in APRIORI DXE list to > make sure it runs before VariableRuntimeDxe. > > > > Signed-off-by: Tuan Phan > > Reviewed-by: Andrei Warkentin > > > --- > > OvmfPkg/RiscVVirt/Sec/Platform.c | 62 +++++++++++++++++++++++++++++++ > > OvmfPkg/RiscVVirt/Sec/SecMain.inf | 1 + > > 2 files changed, 63 insertions(+) > > > > diff --git a/OvmfPkg/RiscVVirt/Sec/Platform.c > b/OvmfPkg/RiscVVirt/Sec/Platform.c > > index 3645c27b0b12..944b82c84a6e 100644 > > --- a/OvmfPkg/RiscVVirt/Sec/Platform.c > > +++ b/OvmfPkg/RiscVVirt/Sec/Platform.c > > @@ -21,6 +21,63 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include > > #include > > > > +/** > > + Build memory map I/O range resource HOB using the > > + base address and size. > > + > > + @param MemoryBase Memory map I/O base. > > + @param MemorySize Memory map I/O size. > > + > > +**/ > > +STATIC > > +VOID > > +AddIoMemoryBaseSizeHob ( > > + EFI_PHYSICAL_ADDRESS MemoryBase, > > + UINT64 MemorySize > > + ) > > +{ > > + /* Align to EFI_PAGE_SIZE */ > > + MemorySize = ALIGN_VALUE (MemorySize, EFI_PAGE_SIZE); > > + BuildResourceDescriptorHob ( > > + EFI_RESOURCE_MEMORY_MAPPED_IO, > > + EFI_RESOURCE_ATTRIBUTE_PRESENT | > > + EFI_RESOURCE_ATTRIBUTE_INITIALIZED | > > + EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE | > > + EFI_RESOURCE_ATTRIBUTE_TESTED, > > + MemoryBase, > > + MemorySize > > + ); > > +} > > + > > +/** > > + Populate IO resources from FDT that not added to GCD by its > > + driver in the DXE phase. > > + > > + @param FdtBase Fdt base address > > + @param Compatible Compatible string > > + > > +**/ > > +STATIC > > +VOID > > +PopulateIoResources ( > > + VOID *FdtBase, > > + CONST CHAR8* Compatible > > + ) > > +{ > > + UINT64 *Reg; > > + INT32 Node, LenP; > > + > > + Node = fdt_node_offset_by_compatible (FdtBase, -1, Compatible); > > + while (Node != -FDT_ERR_NOTFOUND) { > > + Reg = (UINT64 *)fdt_getprop (FdtBase, Node, "reg", &LenP); > > + if (Reg) { > > + ASSERT (LenP == (2 * sizeof (UINT64))); > > + AddIoMemoryBaseSizeHob (SwapBytes64 (Reg[0]), SwapBytes64 > (Reg[1])); > > + } > > + Node = fdt_node_offset_by_compatible (FdtBase, Node, Compatible); > > + } > > +} > > + > > /** > > @retval EFI_SUCCESS The address of FDT is passed in HOB. > > EFI_UNSUPPORTED Can't locate FDT. > > @@ -80,5 +137,10 @@ PlatformPeimInitialization ( > > > > BuildFvHob (PcdGet32 (PcdOvmfDxeMemFvBase), PcdGet32 > (PcdOvmfDxeMemFvSize)); > > > > + PopulateIoResources (Base, "ns16550a"); > > + PopulateIoResources (Base, "qemu,fw-cfg-mmio"); > > + PopulateIoResources (Base, "virtio,mmio"); > > + AddIoMemoryBaseSizeHob (PcdGet32 (PcdOvmfFdBaseAddress), PcdGet32 > (PcdOvmfFirmwareFdSize)); > > + > > return EFI_SUCCESS; > > } > > diff --git a/OvmfPkg/RiscVVirt/Sec/SecMain.inf > b/OvmfPkg/RiscVVirt/Sec/SecMain.inf > > index 0e2a5785e8a4..75d5b74b3d3f 100644 > > --- a/OvmfPkg/RiscVVirt/Sec/SecMain.inf > > +++ b/OvmfPkg/RiscVVirt/Sec/SecMain.inf > > @@ -62,6 +62,7 @@ > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress > > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize > > > > [Guids] > > gFdtHobGuid > > -- > > 2.25.1 > > > > > > > > ------------ > > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#105346): > https://edk2.groups.io/g/devel/message/105346 > > Mute This Topic: https://groups.io/mt/99160000/1131722 > > Group Owner: devel+owner@edk2.groups.io > > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb@kernel.org] > > ------------ > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112547): https://edk2.groups.io/g/devel/message/112547 Mute This Topic: https://groups.io/mt/99160000/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-