Re: [edk2-devel] [PATCH v3 7/7] OvmfPkg/RiscVVirt: SEC: Add IO memory resource hob for platform devices

["Ard Biesheuvel" <ardb@kernel.org>]

On Sat, 27 May 2023 at 01:18, Tuan Phan <tphan@ventanamicro.com> 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?

> Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> Reviewed-by: Andrei Warkentin <andrei.warkentin@intel.com>

> ---
>  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 <libfdt.h>
>  #include <Guid/FdtHob.h>
>
> +/**
> +  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]
> ------------
>
>