one possible issue with ovmf fvb

one possible issue with ovmf fvb

[maobibo]

Hi Gerd,

  I am porting Loongarch Qemu uefi bios, and I want to use reuse ovmf code. And I encounter one problem
when using OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c

  here is piece of code:
-----------------------------------------------------------
  Initialize = TRUE;
  if (PcdGet64 (PcdEmuVariableNvStoreReserved) != 0) {
    Ptr = (VOID*)(UINTN) PcdGet64 (PcdEmuVariableNvStoreReserved);
    DEBUG ((
      DEBUG_INFO,
      "EMU Variable FVB: Using pre-reserved block at %p\n",
      Ptr
      ));
    Status = ValidateFvHeader (Ptr);
    if (!EFI_ERROR (Status)) {
      DEBUG ((DEBUG_INFO, "EMU Variable FVB: Found valid pre-existing FV\n"));
      Initialize = FALSE;
    }
  } else {
    Ptr = AllocateRuntimePages (EFI_SIZE_TO_PAGES (EMU_FVB_SIZE));
  }

  mEmuVarsFvb.BufferPtr = Ptr;

  //
  // Initialize the main FV header and variable store header
  //
  if (Initialize) {
    SetMem (Ptr, EMU_FVB_SIZE, ERASED_UINT8);
    InitializeFvAndVariableStoreHeaders (Ptr);
  }
  PcdStatus = PcdSet64S (PcdFlashNvStorageVariableBase64, (UINT32)(UINTN) Ptr);
  ASSERT_RETURN_ERROR (PcdStatus);

On my tcg vm, Ptr will be 64-bit physical address if memory exceeds 4G, I do not know whether there is similar issue on x64 ovmf.
ditto for the following PcdFlashNvStorageFtwSpareBase/PcdFlashNvStorageFtwWorkingBase

Can uefi bios manage memory beyond 4G?


regards
bibo, mao

Re: one possible issue with ovmf fvb

[Gerd Hoffmann]

  Hi,

>   PcdStatus = PcdSet64S (PcdFlashNvStorageVariableBase64, (UINT32)(UINTN) Ptr);
>   ASSERT_RETURN_ERROR (PcdStatus);

I guess you are referring to that UINT32 cast?
Not sure why it is there.  I think you can just drop it.

> Can uefi bios manage memory beyond 4G?

The X64 builds can do that just fine, although they usually store
everything below 4G, so issues like the one above go unnoticed.

HTH,
  Gerd

Re: [edk2-devel] one possible issue with ovmf fvb

[maobibo]

On 11/25/2021 06:38 PM, Gerd Hoffmann wrote:
>   Hi,
> 
>>   PcdStatus = PcdSet64S (PcdFlashNvStorageVariableBase64, (UINT32)(UINTN) Ptr);
>>   ASSERT_RETURN_ERROR (PcdStatus);
> 
> I guess you are referring to that UINT32 cast?
> Not sure why it is there.  I think you can just drop it.
Yes, it is. There will be problem if the loongarch bios uses ovmf fvb dxe and
there is only one flat memory hob. UEFI bios memory may be allocated with 64bit
address, so there will be problem.

I will submit one patch to fix this in order to use these common code.

regards
bibo, mao

> 
>> Can uefi bios manage memory beyond 4G?
> 
> The X64 builds can do that just fine, although they usually store
> everything below 4G, so issues like the one above go unnoticed.
> 
> HTH,
>   Gerd
> 
> 
> 
> 
> 

Re: [edk2-devel] one possible issue with ovmf fvb

[Andrew Fish]

> On Nov 25, 2021, at 4:43 AM, maobibo <maobibo@loongson.cn> wrote:
> 
> Hi Gerd,
> 
>  I am porting Loongarch Qemu uefi bios, and I want to use reuse ovmf code. And I encounter one problem
> when using OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> 
>  here is piece of code:
> -----------------------------------------------------------
>  Initialize = TRUE;
>  if (PcdGet64 (PcdEmuVariableNvStoreReserved) != 0) {
>    Ptr = (VOID*)(UINTN) PcdGet64 (PcdEmuVariableNvStoreReserved);
>    DEBUG ((
>      DEBUG_INFO,
>      "EMU Variable FVB: Using pre-reserved block at %p\n",
>      Ptr
>      ));
>    Status = ValidateFvHeader (Ptr);
>    if (!EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_INFO, "EMU Variable FVB: Found valid pre-existing FV\n"));
>      Initialize = FALSE;
>    }
>  } else {
>    Ptr = AllocateRuntimePages (EFI_SIZE_TO_PAGES (EMU_FVB_SIZE));
>  }
> 
>  mEmuVarsFvb.BufferPtr = Ptr;
> 
>  //
>  // Initialize the main FV header and variable store header
>  //
>  if (Initialize) {
>    SetMem (Ptr, EMU_FVB_SIZE, ERASED_UINT8);
>    InitializeFvAndVariableStoreHeaders (Ptr);
>  }
>  PcdStatus = PcdSet64S (PcdFlashNvStorageVariableBase64, (UINT32)(UINTN) Ptr);
>  ASSERT_RETURN_ERROR (PcdStatus);
> 
> On my tcg vm, Ptr will be 64-bit physical address if memory exceeds 4G, I do not know whether there is similar issue on x64 ovmf.
> ditto for the following PcdFlashNvStorageFtwSpareBase/PcdFlashNvStorageFtwWorkingBase
> 
> Can uefi bios manage memory beyond 4G?
> 

Bibo,

You are finding implementation bugs. Please file Bugzillas when you see these bugs. 

For X64 (x86-64) the reset vector is 0xFFFFFFF0 (just below 4 GiB) so the ROM (usually a NOR FLASH) is always located under 4 GiB. So that is why these bugs have not been noticed on X64. For AArch64, RiscV64, or loongarch this limitation does not exist and we should fix the code. 

On real X64 hardware you can’t be in 64-bit mode without pageables, and you can’t put pageables in the ROM. Thus PEI ends up being IA32 (i386) and this usually means the DXE Core gets loaded < 4 GiB, and the initial memory map for DXE is memory < 4 GiB. This pattern could also hide some bugs of memory > 4 GiB,, but these are more likely to have been found in the AArch64 port. But if you hit something it is likely due to this. 

Thanks,

Andrew Fish

> 
> regards
> bibo, mao
> 
> 
> 
> 
> 
> 

Re: [edk2-devel] one possible issue with ovmf fvb

[Gerd Hoffmann]

  Hi,

> On real X64 hardware you can’t be in 64-bit mode without pageables,
> and you can’t put pageables in the ROM. Thus PEI ends up being IA32
> (i386) and this usually means the DXE Core gets loaded < 4 GiB, and
> the initial memory map for DXE is memory < 4 GiB.

Well, to exact there are two variants, one with PEI being IA32
(OvmfPkgIa32X64.dsc) and the other with PEI being X64 (OvmfPkgX64.dsc).
The latter does page table setup and switch to long mode in ResetVector
code.

But, yes, on both cases the firmware volume with PEI+DXE ends up being
loaded below 4 GiB.

take care,
  Gerd