On Mon, Mar 6, 2023 at 9:53 AM Ard Biesheuvel <ardb@kernel.org> wrote:

> On Mon, 6 Mar 2023 at 18:33, Tuan Phan <tphan@ventanamicro.com> wrote:
> >
> > The flash base address can be added to GCD before this driver run.
> > So only add it if it has not been done.
> >
>
> How do you end up in this situation?
>
> You cannot skip this registration, as it is required to get the region
> marked as EFI_MEMORY_RUNTIME, and without that, EFI variables will be
> broken when running under the OS.
>

Ard,
The patch only skips AddMemorySpace if it is already done in the early SEC
phase for RiscV platform.
The EFI_MEMORY_RUNTIME always be set in the next line with
SetMemorySpaceAttributes.


> > Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> > ---
> >  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c | 25 +++++++++++++++--------
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
> b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
> > index f9a41f6aab0f..8875824f3333 100644
> > --- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
> > +++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
> > @@ -372,10 +372,11 @@ NorFlashFvbInitialize (
> >    IN NOR_FLASH_INSTANCE  *Instance
> >    )
> >  {
> > -  EFI_STATUS     Status;
> > -  UINT32         FvbNumLba;
> > -  EFI_BOOT_MODE  BootMode;
> > -  UINTN          RuntimeMmioRegionSize;
> > +  EFI_STATUS                      Status;
> > +  UINT32                          FvbNumLba;
> > +  EFI_BOOT_MODE                   BootMode;
> > +  UINTN                           RuntimeMmioRegionSize;
> > +  EFI_GCD_MEMORY_SPACE_DESCRIPTOR Desc;
> >
> >    DEBUG ((DEBUG_BLKIO, "NorFlashFvbInitialize\n"));
> >    ASSERT ((Instance != NULL));
> > @@ -390,13 +391,19 @@ NorFlashFvbInitialize (
> >    //       is written as the base of the flash region (ie:
> Instance->DeviceBaseAddress)
> >    RuntimeMmioRegionSize = (Instance->RegionBaseAddress -
> Instance->DeviceBaseAddress) + Instance->Size;
> >
> > -  Status = gDS->AddMemorySpace (
> > -                  EfiGcdMemoryTypeMemoryMappedIo,
> > +  Status = gDS->GetMemorySpaceDescriptor (
> >                    Instance->DeviceBaseAddress,
> > -                  RuntimeMmioRegionSize,
> > -                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> > +                  &Desc
> >                    );
> > -  ASSERT_EFI_ERROR (Status);
> > +  if (Status == EFI_NOT_FOUND) {
> > +    Status = gDS->AddMemorySpace (
> > +                    EfiGcdMemoryTypeMemoryMappedIo,
> > +                    Instance->DeviceBaseAddress,
> > +                    RuntimeMmioRegionSize,
> > +                    EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> > +                    );
> > +    ASSERT_EFI_ERROR (Status);
> > +  }
> >
> >    Status = gDS->SetMemorySpaceAttributes (
> >                    Instance->DeviceBaseAddress,
> > --
> > 2.25.1
> >
> >
> >
> > ------------
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#100754):
> https://edk2.groups.io/g/devel/message/100754
> > Mute This Topic: https://groups.io/mt/97430554/1131722
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb@kernel.org]
> > ------------
> >
> >
>