Re: [edk2-devel] [PATCH v2 08/14] OvmfPkg/QemuKernelLoaderFsDxe: add support for the kernel setup block

["Laszlo Ersek" <lersek@redhat.com>]

On 03/04/20 10:52, Ard Biesheuvel wrote:
> On x86, the kernel image consists of a setup block and the actual kernel,
> and QEMU presents these as separate blobs, whereas on disk (and in terms
> of PE/COFF image signing), they consist of a single image.
> 
> So add support to our FS loader driver to expose files via the abstract
> file system that consist of up to two concatenated blobs, and redefine
> the kernel file so it consists of the setup and kernel blobs, on every
> architecture (on non-x86, the setup block is simply 0 bytes and is
> therefore ignored implicitly)
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 74 ++++++++++++++------
>  1 file changed, 53 insertions(+), 21 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks,
Laszlo

> diff --git a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
> index dc86a48af378..8ccb1983170c 100644
> --- a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
> +++ b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
> @@ -34,16 +34,29 @@ typedef enum {
>  } KERNEL_BLOB_TYPE;
>  
>  typedef struct {
> -  FIRMWARE_CONFIG_ITEM CONST SizeKey;
> -  FIRMWARE_CONFIG_ITEM CONST DataKey;
> -  CONST CHAR16 *       CONST Name;
> -  UINT32                     Size;
> -  UINT8                      *Data;
> +  CONST CHAR16                  Name[8];
> +  struct {
> +    FIRMWARE_CONFIG_ITEM CONST  SizeKey;
> +    FIRMWARE_CONFIG_ITEM CONST  DataKey;
> +    UINT32                      Size;
> +  }                             FwCfgItem[2];
> +  UINT32                        Size;
> +  UINT8                         *Data;
>  } KERNEL_BLOB;
>  
>  STATIC KERNEL_BLOB mKernelBlob[KernelBlobTypeMax] = {
> -  { QemuFwCfgItemKernelSize,      QemuFwCfgItemKernelData,      L"kernel"  },
> -  { QemuFwCfgItemInitrdSize,      QemuFwCfgItemInitrdData,      L"initrd"  },
> +  {
> +    L"kernel",
> +    {
> +      { QemuFwCfgItemKernelSetupSize, QemuFwCfgItemKernelSetupData, },
> +      { QemuFwCfgItemKernelSize,      QemuFwCfgItemKernelData,      },
> +    }
> +  }, {
> +    L"initrd",
> +    {
> +      { QemuFwCfgItemInitrdSize,      QemuFwCfgItemInitrdData,      },
> +    }
> +  }
>  };
>  
>  STATIC UINT64 mTotalBlobBytes;
> @@ -850,12 +863,21 @@ FetchBlob (
>    )
>  {
>    UINT32 Left;
> +  UINTN  Idx;
> +  UINT8  *ChunkData;
>  
>    //
>    // Read blob size.
>    //
> -  QemuFwCfgSelectItem (Blob->SizeKey);
> -  Blob->Size = QemuFwCfgRead32 ();
> +  Blob->Size = 0;
> +  for (Idx = 0; Idx < ARRAY_SIZE (Blob->FwCfgItem); Idx++) {
> +    if (Blob->FwCfgItem[Idx].SizeKey == 0) {
> +      break;
> +    }
> +    QemuFwCfgSelectItem (Blob->FwCfgItem[Idx].SizeKey);
> +    Blob->FwCfgItem[Idx].Size = QemuFwCfgRead32 ();
> +    Blob->Size += Blob->FwCfgItem[Idx].Size;
> +  }
>    if (Blob->Size == 0) {
>      return EFI_SUCCESS;
>    }
> @@ -872,18 +894,28 @@ FetchBlob (
>  
>    DEBUG ((DEBUG_INFO, "%a: loading %Ld bytes for \"%s\"\n", __FUNCTION__,
>      (INT64)Blob->Size, Blob->Name));
> -  QemuFwCfgSelectItem (Blob->DataKey);
> -
> -  Left = Blob->Size;
> -  do {
> -    UINT32 Chunk;
> -
> -    Chunk = (Left < SIZE_1MB) ? Left : SIZE_1MB;
> -    QemuFwCfgReadBytes (Chunk, Blob->Data + (Blob->Size - Left));
> -    Left -= Chunk;
> -    DEBUG ((DEBUG_VERBOSE, "%a: %Ld bytes remaining for \"%s\"\n",
> -      __FUNCTION__, (INT64)Left, Blob->Name));
> -  } while (Left > 0);
> +
> +  ChunkData = Blob->Data;
> +  for (Idx = 0; Idx < ARRAY_SIZE (Blob->FwCfgItem); Idx++) {
> +    if (Blob->FwCfgItem[Idx].DataKey == 0) {
> +      break;
> +    }
> +    QemuFwCfgSelectItem (Blob->FwCfgItem[Idx].DataKey);
> +
> +    Left = Blob->FwCfgItem[Idx].Size;
> +    while (Left > 0) {
> +      UINT32 Chunk;
> +
> +      Chunk = (Left < SIZE_1MB) ? Left : SIZE_1MB;
> +      QemuFwCfgReadBytes (Chunk, ChunkData + Blob->FwCfgItem[Idx].Size - Left);
> +      Left -= Chunk;
> +      DEBUG ((DEBUG_VERBOSE, "%a: %Ld bytes remaining for \"%s\" (%d)\n",
> +        __FUNCTION__, (INT64)Left, Blob->Name, (INT32)Idx));
> +    }
> +
> +    ChunkData += Blob->FwCfgItem[Idx].Size;
> +  }
> +
>    return EFI_SUCCESS;
>  }
>  
>