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

["Tuan Phan" <tphan@ventanamicro.com>]

[-- Attachment #1: Type: text/plain, Size: 4925 bytes --]

On Tue, May 30, 2023 at 10:38 AM Tuan Phan via groups.io <tphan=
ventanamicro.com@groups.io> wrote:

>
>
> On Mon, May 29, 2023 at 7:07 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
>> 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?
>>
>
> 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?

>
>> > 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]
>> > ------------
>> >
>> >
>>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 7180 bytes --]