Re: [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary

["Gerd Hoffmann" <kraxel@redhat.com>]

> +++ b/OvmfPkg/CloudHv/CloudHvX64.fdf
> @@ -14,8 +14,8 @@
>  !include OvmfPkg/OvmfPkgDefines.fdf.inc
>  
>  #
> -# Build the variable store and the firmware code as one unified flash device
> -# image.
> +# This will allow the flash device image to be recognize as an ELF, with first
> +# an ELF headers, then the firmware code.
>  #
>  [FD.CLOUDHV]
>  BaseAddress   = $(FW_BASE_ADDRESS)
> @@ -24,7 +24,9 @@ ErasePolarity = 1
>  BlockSize     = $(BLOCK_SIZE)
>  NumBlocks     = $(FW_BLOCKS)
>  
> +DEFINE PVH_HEADER_CLOUDHV = TRUE

I'd place the define in CloudHvX64.dsc, in the [Defines] section, where
all the other DEFINE statements are too.

>  !include OvmfPkg/VarStore.fdf.inc
> +DEFINE PVH_HEADER_CLOUDHV = FALSE

No need to flip that back to FALSE.

> +!if $(PVH_HEADER_CLOUDHV) == TRUE
> +!include OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc
> +!else
>  DATA = {
>    ## This is the EFI_FIRMWARE_VOLUME_HEADER
>    # ZeroVector []
> @@ -79,6 +83,7 @@ DATA = {
>    # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
>    0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>  }
> +!endif

Oh.  So the ELF header *replaces* the firmware volume header.
Didn't notice that before.

Hmm.  With that the varstore initialization most likely fails.
There is a fallback path though, with ovmf storing variables
elsewhere (in normal ram instead of firmware flash I think).
Persistent variables don't work in that case.

When the persistent varstore doesn't work anyway (and you are
ok with that) you probably can drop the space reserved for it
from the firmware image and use just a single page for the pvh
elf header (and sharing VarStore.fdf doesn't make sense then).

take care,
  Gerd