public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v4 0/7] CloudHv: Rely on PVH boot specification
@ 2022-02-25 10:45 Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 1/7] OvmfPkg: Make the Xen ELF header generator more flexible Boeuf, Sebastien
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 10:45 UTC (permalink / raw)
  To: devel; +Cc: jiewen.yao, jordan.l.justen, kraxel, sebastien.boeuf

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

Cloud Hypervisor aims at emulating the minimal amount of legacy devices
and this is why the PVH boot specification is supported. The point is to
be able to share some information with the guest without the need for
emulating devices that would be present on real hardware.

Since Cloud Hypervisor supports loading a PVH ELF binary, the CloudHv
target is updated to be generated as such. Relying on the PVH boot
specification, we don't need to hardcode the location of the ACPI tables
anymore since we can retrieve the RSDP address from the hvm_start_info
structure. Same thing for the RAM below 4G, we can find this information
through the PVH memmap entries rather than relying on the emulated CMOS.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>

Sebastien Boeuf (7):
  OvmfPkg: Make the Xen ELF header generator more flexible
  OvmfPkg: Xen: Use a new fdf include for the PVH ELF header
  OvmfPkg: Xen: Generate fdf include file from ELF header generator
  OvmfPkg: Generate CloudHv as a PVH ELF binary
  OvmfPkg: CloudHv: Retrieve RSDP address from PVH
  OvmfPkg: CloudHv: Rely on PVH memmap instead of CMOS
  OvmfPkg: CloudHv: Add README

 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf |   2 +
 OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c       |  39 ++++--
 OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc    |  54 ++++++++
 OvmfPkg/CloudHv/CloudHvX64.dsc              |   2 +-
 OvmfPkg/CloudHv/CloudHvX64.fdf              |  11 +-
 OvmfPkg/CloudHv/README                      |  67 ++++++++++
 OvmfPkg/Include/IndustryStandard/CloudHv.h  |   5 -
 OvmfPkg/OvmfXen.fdf                         |  57 +-------
 OvmfPkg/OvmfXenElfHeaderGenerator.c         | 141 +++++++++++++++-----
 OvmfPkg/PlatformPei/MemDetect.c             |  73 ++++++++++
 OvmfPkg/PlatformPei/PlatformPei.inf         |   2 +
 OvmfPkg/VarStore.fdf.inc                    |   5 +
 OvmfPkg/XenElfHeader.fdf.inc                |  42 ++++++
 13 files changed, 392 insertions(+), 108 deletions(-)
 create mode 100644 OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc
 create mode 100644 OvmfPkg/CloudHv/README
 create mode 100644 OvmfPkg/XenElfHeader.fdf.inc

-- 
2.32.0

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4 1/7] OvmfPkg: Make the Xen ELF header generator more flexible
  2022-02-25 10:45 [PATCH v4 0/7] CloudHv: Rely on PVH boot specification Boeuf, Sebastien
@ 2022-02-25 10:45 ` Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 2/7] OvmfPkg: Xen: Use a new fdf include for the PVH ELF header Boeuf, Sebastien
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 10:45 UTC (permalink / raw)
  To: devel; +Cc: jiewen.yao, jordan.l.justen, kraxel, sebastien.boeuf

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

Adding some flexibility to the program through optional parameters and
global define, so that other targets can use the generator.

* A global define is added so that we can choose at build time if we
  want to use 32-bit or 64-bit base structures.
* A first optional parameter is added so the user can provide the
  expected blob size of the generated binary.
* A second optional parameter is added so the user can specify an output
  file to which the generated output will be printed.

The default behavior isn't modified.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 OvmfPkg/OvmfXenElfHeaderGenerator.c | 141 +++++++++++++++++++++-------
 1 file changed, 109 insertions(+), 32 deletions(-)

diff --git a/OvmfPkg/OvmfXenElfHeaderGenerator.c b/OvmfPkg/OvmfXenElfHeaderGenerator.c
index 489060cdad..672129b85d 100644
--- a/OvmfPkg/OvmfXenElfHeaderGenerator.c
+++ b/OvmfPkg/OvmfXenElfHeaderGenerator.c
@@ -10,19 +10,31 @@
 **/
 
 #include "elf.h"
-#include "stdio.h"
+#include "fcntl.h"
+#include "stdbool.h"
 #include "stddef.h"
+#include "stdio.h"
+#include "stdlib.h"
 
 void
 print_hdr (
+  FILE    *file,
   void    *s,
-  size_t  size
+  size_t  size,
+  bool    end_delimiter
   )
 {
   char  *c = s;
 
-  while (size--) {
-    printf ("0x%02hhx, ", *(c++));
+  fprintf (file, "  ");
+  while (size-- > 1) {
+    fprintf (file, "0x%02hhx, ", *(c++));
+  }
+
+  if (end_delimiter) {
+    fprintf (file, "0x%02hhx,", *c);
+  } else {
+    fprintf (file, "0x%02hhx", *c);
   }
 }
 
@@ -36,34 +48,79 @@ typedef struct {
   uint32_t    desc;
 } xen_elfnote_phys32_entry;
 
+#define LICENSE_HDR  "\
+## @file\r\n\
+#  FDF include file that defines a PVH ELF header.\r\n\
+#\r\n\
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.\r\n\
+#\r\n\
+#  SPDX-License-Identifier: BSD-2-Clause-Patent\r\n\
+#\r\n\
+##\r\n\
+\r\n\
+"
+
 int
 main (
-  void
+  int   argc,
+  char  *argv[]
   )
 {
   /* FW_SIZE */
   size_t  ovmf_blob_size = 0x00200000;
   /* Load OVMF at 1MB when running as PVH guest */
   uint32_t  ovmf_base_address = 0x00100000;
+  uint32_t  ovmfxen_pvh_entry_point;
+  size_t    offset_into_file = 0;
+  char      *endptr, *str;
+  long      param;
+  FILE      *file = stdout;
+
+  /* Parse the size parameter */
+  if (argc > 1) {
+    str   = argv[1];
+    param = strtol (str, &endptr, 10);
+    if (endptr != str) {
+      ovmf_blob_size = (size_t)param;
+    }
+  }
+
+  /* Parse the filepath parameter */
+  if (argc > 2) {
+    file = fopen (argv[2], "w");
+    fprintf (file, LICENSE_HDR);
+  }
+
   /* Xen PVH entry point */
-  uint32_t  ovmfxen_pvh_entry_point = ovmf_base_address + ovmf_blob_size - 0x30;
-  size_t    offset_into_file        = 0;
+  ovmfxen_pvh_entry_point = ovmf_base_address + ovmf_blob_size - 0x30;
 
   /* ELF file header */
+ #ifdef PVH64
+  Elf64_Ehdr  hdr = {
+ #else
   Elf32_Ehdr  hdr = {
-    .e_ident     = ELFMAG,
-    .e_type      = ET_EXEC,
-    .e_machine   = EM_386,
-    .e_version   = EV_CURRENT,
-    .e_entry     = ovmfxen_pvh_entry_point,
-    .e_flags     = R_386_NONE,
-    .e_ehsize    = sizeof (hdr),
+ #endif
+    .e_ident   = ELFMAG,
+    .e_type    = ET_EXEC,
+    .e_machine = EM_386,
+    .e_version = EV_CURRENT,
+    .e_entry   = ovmfxen_pvh_entry_point,
+    .e_flags   = R_386_NONE,
+    .e_ehsize  = sizeof (hdr),
+ #ifdef PVH64
+    .e_phentsize = sizeof (Elf64_Phdr),
+ #else
     .e_phentsize = sizeof (Elf32_Phdr),
+ #endif
   };
 
   offset_into_file += sizeof (hdr);
 
-  hdr.e_ident[EI_CLASS]   = ELFCLASS32;
+ #ifdef PVH64
+  hdr.e_ident[EI_CLASS] = ELFCLASS64;
+ #else
+  hdr.e_ident[EI_CLASS] = ELFCLASS32;
+ #endif
   hdr.e_ident[EI_DATA]    = ELFDATA2LSB;
   hdr.e_ident[EI_VERSION] = EV_CURRENT;
   hdr.e_ident[EI_OSABI]   = ELFOSABI_LINUX;
@@ -71,14 +128,22 @@ main (
   hdr.e_phoff = sizeof (hdr);
 
   /* program header */
+ #ifdef PVH64
+  Elf64_Phdr  phdr_load = {
+ #else
   Elf32_Phdr  phdr_load = {
+ #endif
     .p_type   = PT_LOAD,
     .p_offset = 0, /* load everything */
     .p_paddr  = ovmf_base_address,
     .p_filesz = ovmf_blob_size,
     .p_memsz  = ovmf_blob_size,
     .p_flags  = PF_X | PF_W | PF_R,
+ #ifdef PVH64
+    .p_align  = 4,
+ #else
     .p_align  = 0,
+ #endif
   };
 
   phdr_load.p_vaddr = phdr_load.p_paddr;
@@ -98,12 +163,20 @@ main (
       sizeof (xen_elfnote_phys32_entry) -
       offsetof (xen_elfnote_phys32_entry, desc),
   };
-  Elf32_Phdr                phdr_note = {
+ #ifdef PVH64
+  Elf64_Phdr  phdr_note = {
+ #else
+  Elf32_Phdr  phdr_note = {
+ #endif
     .p_type   = PT_NOTE,
     .p_filesz = sizeof (xen_elf_note),
     .p_memsz  = sizeof (xen_elf_note),
     .p_flags  = PF_R,
+ #ifdef PVH64
+    .p_align  = 4,
+ #else
     .p_align  = 0,
+ #endif
   };
 
   hdr.e_phnum       += 1;
@@ -120,31 +193,35 @@ main (
   size_t  hdr_size  = sizeof (hdr);
   size_t  entry_off = offsetof (typeof(hdr), e_entry);
 
-  printf ("# ELF file header\n");
-  print_hdr (&hdr, entry_off);
-  printf ("\n");
-  print_hdr (&hdr.e_entry, sizeof (hdr.e_entry));
-  printf (" # hdr.e_entry\n");
-  print_hdr (&hdr.e_entry + 1, hdr_size - entry_off - sizeof (hdr.e_entry));
+  fprintf (file, "DATA = {\r\n");
 
-  printf ("\n\n# ELF Program segment headers\n");
-  printf ("# - Load segment\n");
+  fprintf (file, "  # ELF file header\r\n");
+  print_hdr (file, &hdr, entry_off, true);
+  fprintf (file, "\r\n");
+  print_hdr (file, &hdr.e_entry, sizeof (hdr.e_entry), true);
+  fprintf (file, " # hdr.e_entry\r\n");
+  print_hdr (file, &hdr.e_entry + 1, hdr_size - entry_off - sizeof (hdr.e_entry), true);
+
+  fprintf (file, "\r\n\r\n  # ELF Program segment headers\r\n");
+  fprintf (file, "  # - Load segment\r\n");
   for (i = 0; i < sizeof (phdr_load); i += 4) {
-    print_hdr (((char *)&phdr_load) + i, 4);
-    printf ("\n");
+    print_hdr (file, ((char *)&phdr_load) + i, 4, true);
+    fprintf (file, "\r\n");
   }
 
-  printf ("# - ELFNOTE segment\n");
+  fprintf (file, "  # - ELFNOTE segment\r\n");
   for (i = 0; i < sizeof (phdr_note); i += 4) {
-    print_hdr (((char *)&phdr_note) + i, 4);
-    printf ("\n");
+    print_hdr (file, ((char *)&phdr_note) + i, 4, true);
+    fprintf (file, "\r\n");
   }
 
-  printf ("\n# XEN_ELFNOTE_PHYS32_ENTRY\n");
+  fprintf (file, "\r\n  # XEN_ELFNOTE_PHYS32_ENTRY\r\n");
   for (i = 0; i < sizeof (xen_elf_note); i += 4) {
-    print_hdr (((char *)&xen_elf_note) + i, 4);
-    printf ("\n");
+    print_hdr (file, ((char *)&xen_elf_note) + i, 4, (sizeof (xen_elf_note) - i) > 4);
+    fprintf (file, "\r\n");
   }
 
+  fprintf (file, "}\r\n");
+
   return 0;
 }
-- 
2.32.0

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 2/7] OvmfPkg: Xen: Use a new fdf include for the PVH ELF header
  2022-02-25 10:45 [PATCH v4 0/7] CloudHv: Rely on PVH boot specification Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 1/7] OvmfPkg: Make the Xen ELF header generator more flexible Boeuf, Sebastien
@ 2022-02-25 10:45 ` Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 3/7] OvmfPkg: Xen: Generate fdf include file from ELF header generator Boeuf, Sebastien
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 10:45 UTC (permalink / raw)
  To: devel; +Cc: jiewen.yao, jordan.l.justen, kraxel, sebastien.boeuf

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

Instead of having the PVH ELF header part of the fdf file directly, we
move it to a dedicated include file. This is the first step in
automating the generation of the header.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 OvmfPkg/OvmfXen.fdf          | 57 ++------------------------------
 OvmfPkg/XenElfHeader.fdf.inc | 64 ++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 55 deletions(-)
 create mode 100644 OvmfPkg/XenElfHeader.fdf.inc

diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf
index a6acf3b835..2e67db5d32 100644
--- a/OvmfPkg/OvmfXen.fdf
+++ b/OvmfPkg/OvmfXen.fdf
@@ -31,61 +31,8 @@ NumBlocks     = $(FW_BLOCKS)
 !if $(FD_SIZE_IN_KB) == 4096
 0x00000000|0x00040000
 !endif
-DATA = {
-  #
-  # This hex array have been generated by OvmfPkg/OvmfXenElfHeaderGenerator.c
-  # and copied manually.
-  #
-  # ELF file header
-  0x7f, 0x45, 0x4c, 0x46, # e_ident[0..3]: Magic number
-  0x01, # File class: 32-bit objects
-  0x01, # Data encoding: 2's complement, little endian
-  0x01, # File version
-  0x03, # OS ABI identification: Object uses GNU ELF extensions
-  0x00, # ABI version
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  # e_ident[EI_PAD...]
-  0x02, 0x00, # e_type = Executable file
-  0x03, 0x00, # e_machine = Intel 80386
-  0x01, 0x00, 0x00, 0x00, # e_version
-  0xd0, 0xff, 0x2f, 0x00, # e_entry: Entry point virtual address
-  0x34, 0x00, 0x00, 0x00, # e_phoff: Program header table file offset
-  0x00, 0x00, 0x00, 0x00, # e_shoff: Section header table file offset
-  0x00, 0x00, 0x00, 0x00, # e_flags: Processor-specific flags
-  0x34, 0x00, #    e_ehsize: ELF header size
-  0x20, 0x00, # e_phentsize: Program header table entry size
-  0x02, 0x00, #     e_phnum: Program header table entry count
-  0x00, 0x00, # e_shentsize: Section header table entry size
-  0x00, 0x00, #     e_shnum: Section header table entry count
-  0x00, 0x00, # e_shstrndx
-
-  # ELF Program segment headers
-  # - Load segment
-  0x01, 0x00, 0x00, 0x00, # p_type = Loadable program segment
-  0x00, 0x00, 0x00, 0x00, # p_offset
-  0x00, 0x00, 0x10, 0x00, # p_vaddr: Segment virtual address
-  0x00, 0x00, 0x10, 0x00, # p_paddr: Segment physical address
-  0x00, 0x00, 0x20, 0x00, # p_filesz: Segment size in file
-  0x00, 0x00, 0x20, 0x00, # p_memsz: Segment size in memory
-  0x07, 0x00, 0x00, 0x00, # p_flags = Segment is executable | writable | readable
-  0x00, 0x00, 0x00, 0x00, # p_align
-  # - ELFNOTE segment
-  0x04, 0x00, 0x00, 0x00, # p_type = PT_NOTE
-  0x74, 0x00, 0x00, 0x00, # p_offset = point to XEN_ELFNOTE_PHYS32_ENTRY below
-  0x74, 0x00, 0x10, 0x00,
-  0x74, 0x00, 0x10, 0x00,
-  0x14, 0x00, 0x00, 0x00,
-  0x14, 0x00, 0x00, 0x00,
-  0x04, 0x00, 0x00, 0x00, # p_flags = Segment is readable
-  0x00, 0x00, 0x00, 0x00,
-
-  # XEN_ELFNOTE_PHYS32_ENTRY
-  0x04, 0x00, 0x00, 0x00, # name size
-  0x04, 0x00, 0x00, 0x00, # desc size
-  0x12, 0x00, 0x00, 0x00, # type = XEN_ELFNOTE_PHYS32_ENTRY
-  0x58, 0x65, 0x6e, 0x00, # name = "Xen"
-  0xd0, 0xff, 0x2f, 0x00, # desc: PVH entry point
-  0x00
-}
+
+!include XenElfHeader.fdf.inc
 
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
 0x0000e000|0x00001000
diff --git a/OvmfPkg/XenElfHeader.fdf.inc b/OvmfPkg/XenElfHeader.fdf.inc
new file mode 100644
index 0000000000..dbc1305d25
--- /dev/null
+++ b/OvmfPkg/XenElfHeader.fdf.inc
@@ -0,0 +1,64 @@
+## @file
+#  FDF include file that defines a PVH ELF header.
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+DATA = {
+  #
+  # This hex array have been generated by OvmfPkg/OvmfXenElfHeaderGenerator.c
+  # and copied manually.
+  #
+  # ELF file header
+  0x7f, 0x45, 0x4c, 0x46, # e_ident[0..3]: Magic number
+  0x01, # File class: 32-bit objects
+  0x01, # Data encoding: 2's complement, little endian
+  0x01, # File version
+  0x03, # OS ABI identification: Object uses GNU ELF extensions
+  0x00, # ABI version
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  # e_ident[EI_PAD...]
+  0x02, 0x00, # e_type = Executable file
+  0x03, 0x00, # e_machine = Intel 80386
+  0x01, 0x00, 0x00, 0x00, # e_version
+  0xd0, 0xff, 0x2f, 0x00, # e_entry: Entry point virtual address
+  0x34, 0x00, 0x00, 0x00, # e_phoff: Program header table file offset
+  0x00, 0x00, 0x00, 0x00, # e_shoff: Section header table file offset
+  0x00, 0x00, 0x00, 0x00, # e_flags: Processor-specific flags
+  0x34, 0x00, #    e_ehsize: ELF header size
+  0x20, 0x00, # e_phentsize: Program header table entry size
+  0x02, 0x00, #     e_phnum: Program header table entry count
+  0x00, 0x00, # e_shentsize: Section header table entry size
+  0x00, 0x00, #     e_shnum: Section header table entry count
+  0x00, 0x00, # e_shstrndx
+
+  # ELF Program segment headers
+  # - Load segment
+  0x01, 0x00, 0x00, 0x00, # p_type = Loadable program segment
+  0x00, 0x00, 0x00, 0x00, # p_offset
+  0x00, 0x00, 0x10, 0x00, # p_vaddr: Segment virtual address
+  0x00, 0x00, 0x10, 0x00, # p_paddr: Segment physical address
+  0x00, 0x00, 0x20, 0x00, # p_filesz: Segment size in file
+  0x00, 0x00, 0x20, 0x00, # p_memsz: Segment size in memory
+  0x07, 0x00, 0x00, 0x00, # p_flags = Segment is executable | writable | readable
+  0x00, 0x00, 0x00, 0x00, # p_align
+  # - ELFNOTE segment
+  0x04, 0x00, 0x00, 0x00, # p_type = PT_NOTE
+  0x74, 0x00, 0x00, 0x00, # p_offset = point to XEN_ELFNOTE_PHYS32_ENTRY below
+  0x74, 0x00, 0x10, 0x00,
+  0x74, 0x00, 0x10, 0x00,
+  0x14, 0x00, 0x00, 0x00,
+  0x14, 0x00, 0x00, 0x00,
+  0x04, 0x00, 0x00, 0x00, # p_flags = Segment is readable
+  0x00, 0x00, 0x00, 0x00,
+
+  # XEN_ELFNOTE_PHYS32_ENTRY
+  0x04, 0x00, 0x00, 0x00, # name size
+  0x04, 0x00, 0x00, 0x00, # desc size
+  0x12, 0x00, 0x00, 0x00, # type = XEN_ELFNOTE_PHYS32_ENTRY
+  0x58, 0x65, 0x6e, 0x00, # name = "Xen"
+  0xd0, 0xff, 0x2f, 0x00, # desc: PVH entry point
+  0x00
+}
-- 
2.32.0

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 3/7] OvmfPkg: Xen: Generate fdf include file from ELF header generator
  2022-02-25 10:45 [PATCH v4 0/7] CloudHv: Rely on PVH boot specification Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 1/7] OvmfPkg: Make the Xen ELF header generator more flexible Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 2/7] OvmfPkg: Xen: Use a new fdf include for the PVH ELF header Boeuf, Sebastien
@ 2022-02-25 10:45 ` Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary Boeuf, Sebastien
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 10:45 UTC (permalink / raw)
  To: devel; +Cc: jiewen.yao, jordan.l.justen, kraxel, sebastien.boeuf

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

Updating the fdf include file based on the run of the ELF header
generator. The diff from this patch is the result of:

$ gcc -o elf_gen OvmfPkg/OvmfXenElfHeaderGenerator.c
$ ./elf_gen 2097152 OvmfPkg/XenElfHeader.fdf.inc

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 OvmfPkg/XenElfHeader.fdf.inc | 60 ++++++++++++------------------------
 1 file changed, 19 insertions(+), 41 deletions(-)

diff --git a/OvmfPkg/XenElfHeader.fdf.inc b/OvmfPkg/XenElfHeader.fdf.inc
index dbc1305d25..c4f04ad28b 100644
--- a/OvmfPkg/XenElfHeader.fdf.inc
+++ b/OvmfPkg/XenElfHeader.fdf.inc
@@ -8,57 +8,35 @@
 ##
 
 DATA = {
-  #
-  # This hex array have been generated by OvmfPkg/OvmfXenElfHeaderGenerator.c
-  # and copied manually.
-  #
   # ELF file header
-  0x7f, 0x45, 0x4c, 0x46, # e_ident[0..3]: Magic number
-  0x01, # File class: 32-bit objects
-  0x01, # Data encoding: 2's complement, little endian
-  0x01, # File version
-  0x03, # OS ABI identification: Object uses GNU ELF extensions
-  0x00, # ABI version
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  # e_ident[EI_PAD...]
-  0x02, 0x00, # e_type = Executable file
-  0x03, 0x00, # e_machine = Intel 80386
-  0x01, 0x00, 0x00, 0x00, # e_version
-  0xd0, 0xff, 0x2f, 0x00, # e_entry: Entry point virtual address
-  0x34, 0x00, 0x00, 0x00, # e_phoff: Program header table file offset
-  0x00, 0x00, 0x00, 0x00, # e_shoff: Section header table file offset
-  0x00, 0x00, 0x00, 0x00, # e_flags: Processor-specific flags
-  0x34, 0x00, #    e_ehsize: ELF header size
-  0x20, 0x00, # e_phentsize: Program header table entry size
-  0x02, 0x00, #     e_phnum: Program header table entry count
-  0x00, 0x00, # e_shentsize: Section header table entry size
-  0x00, 0x00, #     e_shnum: Section header table entry count
-  0x00, 0x00, # e_shstrndx
+  0x7f, 0x45, 0x4c, 0x46, 0x01, 0x01, 0x01, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x03, 0x00, 0x01, 0x00, 0x00, 0x00,
+  0xd0, 0xff, 0x2f, 0x00, # hdr.e_entry
+  0x34, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x34, 0x00, 0x20, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 
   # ELF Program segment headers
   # - Load segment
-  0x01, 0x00, 0x00, 0x00, # p_type = Loadable program segment
-  0x00, 0x00, 0x00, 0x00, # p_offset
-  0x00, 0x00, 0x10, 0x00, # p_vaddr: Segment virtual address
-  0x00, 0x00, 0x10, 0x00, # p_paddr: Segment physical address
-  0x00, 0x00, 0x20, 0x00, # p_filesz: Segment size in file
-  0x00, 0x00, 0x20, 0x00, # p_memsz: Segment size in memory
-  0x07, 0x00, 0x00, 0x00, # p_flags = Segment is executable | writable | readable
-  0x00, 0x00, 0x00, 0x00, # p_align
+  0x01, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x10, 0x00,
+  0x00, 0x00, 0x10, 0x00,
+  0x00, 0x00, 0x20, 0x00,
+  0x00, 0x00, 0x20, 0x00,
+  0x07, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
   # - ELFNOTE segment
-  0x04, 0x00, 0x00, 0x00, # p_type = PT_NOTE
-  0x74, 0x00, 0x00, 0x00, # p_offset = point to XEN_ELFNOTE_PHYS32_ENTRY below
+  0x04, 0x00, 0x00, 0x00,
+  0x74, 0x00, 0x00, 0x00,
   0x74, 0x00, 0x10, 0x00,
   0x74, 0x00, 0x10, 0x00,
   0x14, 0x00, 0x00, 0x00,
   0x14, 0x00, 0x00, 0x00,
-  0x04, 0x00, 0x00, 0x00, # p_flags = Segment is readable
+  0x04, 0x00, 0x00, 0x00,
   0x00, 0x00, 0x00, 0x00,
 
   # XEN_ELFNOTE_PHYS32_ENTRY
-  0x04, 0x00, 0x00, 0x00, # name size
-  0x04, 0x00, 0x00, 0x00, # desc size
-  0x12, 0x00, 0x00, 0x00, # type = XEN_ELFNOTE_PHYS32_ENTRY
-  0x58, 0x65, 0x6e, 0x00, # name = "Xen"
-  0xd0, 0xff, 0x2f, 0x00, # desc: PVH entry point
-  0x00
+  0x04, 0x00, 0x00, 0x00,
+  0x04, 0x00, 0x00, 0x00,
+  0x12, 0x00, 0x00, 0x00,
+  0x58, 0x65, 0x6e, 0x00,
+  0xd0, 0xff, 0x2f, 0x00
 }
-- 
2.32.0

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-02-25 10:45 [PATCH v4 0/7] CloudHv: Rely on PVH boot specification Boeuf, Sebastien
                   ` (2 preceding siblings ...)
  2022-02-25 10:45 ` [PATCH v4 3/7] OvmfPkg: Xen: Generate fdf include file from ELF header generator Boeuf, Sebastien
@ 2022-02-25 10:45 ` Boeuf, Sebastien
  2022-02-25 13:00   ` Gerd Hoffmann
  2022-02-25 10:45 ` [PATCH v4 5/7] OvmfPkg: CloudHv: Retrieve RSDP address from PVH Boeuf, Sebastien
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 10:45 UTC (permalink / raw)
  To: devel; +Cc: jiewen.yao, jordan.l.justen, kraxel, sebastien.boeuf

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

Following the model from the Xen target, CloudHv is generated as a PVH
ELF binary to take advantage of the PVH specification, which requires
less emulation from the VMM.

The fdf include file CloudHvElfHeader.fdf.inc has been generated from
the following commands:

$ gcc -D PVH64 -o elf_gen OvmfPkg/OvmfXenElfHeaderGenerator.c
$ ./elf_gen 4194304 OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc | 54 ++++++++++++++++++++++++
 OvmfPkg/CloudHv/CloudHvX64.dsc           |  2 +-
 OvmfPkg/CloudHv/CloudHvX64.fdf           |  8 ++--
 OvmfPkg/VarStore.fdf.inc                 |  5 +++
 4 files changed, 65 insertions(+), 4 deletions(-)
 create mode 100644 OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc

diff --git a/OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc b/OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc
new file mode 100644
index 0000000000..8377e30bdc
--- /dev/null
+++ b/OvmfPkg/CloudHv/CloudHvElfHeader.fdf.inc
@@ -0,0 +1,54 @@
+## @file
+#  FDF include file that defines a PVH ELF header.
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+DATA = {
+  # ELF file header
+  0x7f, 0x45, 0x4c, 0x46, 0x02, 0x01, 0x01, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x03, 0x00, 0x01, 0x00, 0x00, 0x00,
+  0xd0, 0xff, 0x4f, 0x00, 0x00, 0x00, 0x00, 0x00, # hdr.e_entry
+  0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x38, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+
+  # ELF Program segment headers
+  # - Load segment
+  0x01, 0x00, 0x00, 0x00,
+  0x07, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x10, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x10, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x40, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x40, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x04, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  # - ELFNOTE segment
+  0x04, 0x00, 0x00, 0x00,
+  0x04, 0x00, 0x00, 0x00,
+  0xb0, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0xb0, 0x00, 0x10, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0xb0, 0x00, 0x10, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x14, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x14, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+  0x04, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00,
+
+  # XEN_ELFNOTE_PHYS32_ENTRY
+  0x04, 0x00, 0x00, 0x00,
+  0x04, 0x00, 0x00, 0x00,
+  0x12, 0x00, 0x00, 0x00,
+  0x58, 0x65, 0x6e, 0x00,
+  0xd0, 0xff, 0x4f, 0x00
+}
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index 3172100310..b4d855d80f 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -631,7 +631,7 @@
 #
 ################################################################################
 [Components]
-  OvmfPkg/ResetVector/ResetVector.inf
+  OvmfPkg/XenResetVector/XenResetVector.inf
 
   #
   # SEC Phase modules
diff --git a/OvmfPkg/CloudHv/CloudHvX64.fdf b/OvmfPkg/CloudHv/CloudHvX64.fdf
index ce3302c6d6..f638978730 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.fdf
+++ 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
 !include OvmfPkg/VarStore.fdf.inc
+DEFINE PVH_HEADER_CLOUDHV = FALSE
 
 $(VARS_SIZE)|$(FVMAIN_SIZE)
 FV = FVMAIN_COMPACT
@@ -142,7 +144,7 @@ READ_LOCK_STATUS   = TRUE
 #
 INF  OvmfPkg/Sec/SecMain.inf
 
-INF  RuleOverride=RESET_VECTOR OvmfPkg/ResetVector/ResetVector.inf
+INF  RuleOverride=RESET_VECTOR OvmfPkg/XenResetVector/XenResetVector.inf
 
 ################################################################################
 [FV.PEIFV]
diff --git a/OvmfPkg/VarStore.fdf.inc b/OvmfPkg/VarStore.fdf.inc
index a1e524e393..3774e1cca8 100644
--- a/OvmfPkg/VarStore.fdf.inc
+++ b/OvmfPkg/VarStore.fdf.inc
@@ -15,6 +15,10 @@
 0x00000000|0x00040000
 !endif
 #NV_VARIABLE_STORE
+
+!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
 
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
 0x0000e000|0x00001000
-- 
2.32.0

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 5/7] OvmfPkg: CloudHv: Retrieve RSDP address from PVH
  2022-02-25 10:45 [PATCH v4 0/7] CloudHv: Rely on PVH boot specification Boeuf, Sebastien
                   ` (3 preceding siblings ...)
  2022-02-25 10:45 ` [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary Boeuf, Sebastien
@ 2022-02-25 10:45 ` Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 6/7] OvmfPkg: CloudHv: Rely on PVH memmap instead of CMOS Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 7/7] OvmfPkg: CloudHv: Add README Boeuf, Sebastien
  6 siblings, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 10:45 UTC (permalink / raw)
  To: devel; +Cc: jiewen.yao, jordan.l.justen, kraxel, sebastien.boeuf

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

Instead of hardcoding the address of the RSDP in the firmware, let's
rely on the PVH structure hvm_start_info to retrieve this information.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf |  2 ++
 OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c       | 39 ++++++++++++++-------
 OvmfPkg/CloudHv/CloudHvX64.fdf              |  3 ++
 OvmfPkg/Include/IndustryStandard/CloudHv.h  |  5 ---
 4 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
index b36b8413e0..f22bd7cb6d 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -56,6 +56,8 @@
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
+  gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtr
+  gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtrSize
 
 [Depex]
   gEfiAcpiTableProtocolGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
index 44a6bb70fe..ff59600d3e 100644
--- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
@@ -7,9 +7,11 @@
 
 **/
 
-#include <IndustryStandard/CloudHv.h> // CLOUDHV_RSDP_ADDRESS
-#include <Library/BaseLib.h>          // CpuDeadLoop()
-#include <Library/DebugLib.h>         // DEBUG()
+#include <IndustryStandard/CloudHv.h>                     // CLOUDHV_RSDP_ADDRESS
+#include <IndustryStandard/Xen/arch-x86/hvm/start_info.h> // hvm_start_info
+#include <Library/BaseLib.h>                              // CpuDeadLoop()
+#include <Library/DebugLib.h>                             // DEBUG()
+#include <Library/PcdLib.h>                               // PcdGet32()
 
 #include "AcpiPlatform.h"
 
@@ -23,20 +25,33 @@ InstallCloudHvTables (
   EFI_STATUS  Status;
   UINTN       TableHandle;
 
-  EFI_ACPI_DESCRIPTION_HEADER                *Xsdt;
-  VOID                                       *CurrentTableEntry;
-  UINTN                                      CurrentTablePointer;
-  EFI_ACPI_DESCRIPTION_HEADER                *CurrentTable;
-  UINTN                                      Index;
-  UINTN                                      NumberOfTableEntries;
-  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt2Table;
-  EFI_ACPI_DESCRIPTION_HEADER                *DsdtTable;
+  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
+  VOID                                          *CurrentTableEntry;
+  UINTN                                         CurrentTablePointer;
+  EFI_ACPI_DESCRIPTION_HEADER                   *CurrentTable;
+  UINTN                                         Index;
+  UINTN                                         NumberOfTableEntries;
+  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt2Table;
+  EFI_ACPI_DESCRIPTION_HEADER                   *DsdtTable;
+  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *AcpiRsdpStructurePtr;
+  UINT32                                        *PVHResetVectorData;
+  struct hvm_start_info                         *pvh_start_info;
 
   Fadt2Table           = NULL;
   DsdtTable            = NULL;
   TableHandle          = 0;
   NumberOfTableEntries = 0;
-  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *AcpiRsdpStructurePtr = (VOID *)CLOUDHV_RSDP_ADDRESS;
+  AcpiRsdpStructurePtr = NULL;
+  PVHResetVectorData   = NULL;
+  pvh_start_info       = NULL;
+
+  PVHResetVectorData = (VOID *)(UINTN)PcdGet32 (PcdXenPvhStartOfDayStructPtr);
+  if (PVHResetVectorData == 0) {
+    return EFI_NOT_FOUND;
+  }
+
+  pvh_start_info       = (struct hvm_start_info *)(UINTN)PVHResetVectorData[0];
+  AcpiRsdpStructurePtr = (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)(UINTN)pvh_start_info->rsdp_paddr;
 
   // If XSDT table is found, just install its tables.
   // Otherwise, try to find and install the RSDT tables.
diff --git a/OvmfPkg/CloudHv/CloudHvX64.fdf b/OvmfPkg/CloudHv/CloudHvX64.fdf
index f638978730..7c480f1cbc 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.fdf
+++ b/OvmfPkg/CloudHv/CloudHvX64.fdf
@@ -96,6 +96,9 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgTokenSpaceGuid.PcdO
 0x00E000|0x001000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize
 
+0x00F000|0x001000
+gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtr|gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtrSize
+
 0x010000|0x010000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
 
diff --git a/OvmfPkg/Include/IndustryStandard/CloudHv.h b/OvmfPkg/Include/IndustryStandard/CloudHv.h
index 86404cc97e..d31ecc9eec 100644
--- a/OvmfPkg/Include/IndustryStandard/CloudHv.h
+++ b/OvmfPkg/Include/IndustryStandard/CloudHv.h
@@ -38,9 +38,4 @@
 //
 #define CLOUDHV_SMBIOS_ADDRESS  0xf0000
 
-//
-// RSDP address
-//
-#define CLOUDHV_RSDP_ADDRESS  0xa0000
-
 #endif // __CLOUDHV_H__
-- 
2.32.0

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 6/7] OvmfPkg: CloudHv: Rely on PVH memmap instead of CMOS
  2022-02-25 10:45 [PATCH v4 0/7] CloudHv: Rely on PVH boot specification Boeuf, Sebastien
                   ` (4 preceding siblings ...)
  2022-02-25 10:45 ` [PATCH v4 5/7] OvmfPkg: CloudHv: Retrieve RSDP address from PVH Boeuf, Sebastien
@ 2022-02-25 10:45 ` Boeuf, Sebastien
  2022-02-25 10:45 ` [PATCH v4 7/7] OvmfPkg: CloudHv: Add README Boeuf, Sebastien
  6 siblings, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 10:45 UTC (permalink / raw)
  To: devel; +Cc: jiewen.yao, jordan.l.justen, kraxel, sebastien.boeuf

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

Instead of using the CMOS, the CloudHv platform relies on the list of
memmap entries provided through the PVH boot protocol to determine the
last RAM address below 4G.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 OvmfPkg/PlatformPei/MemDetect.c     | 73 +++++++++++++++++++++++++++++
 OvmfPkg/PlatformPei/PlatformPei.inf |  2 +
 2 files changed, 75 insertions(+)

diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 1bcb5a08bc..8ecc8257f9 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -17,6 +17,7 @@ Module Name:
 #include <IndustryStandard/I440FxPiix4.h>
 #include <IndustryStandard/Q35MchIch9.h>
 #include <IndustryStandard/CloudHv.h>
+#include <IndustryStandard/Xen/arch-x86/hvm/start_info.h>
 #include <PiPei.h>
 #include <Register/Intel/SmramSaveStateMap.h>
 
@@ -315,6 +316,73 @@ ScanOrAdd64BitE820Ram (
   return EFI_SUCCESS;
 }
 
+/**
+  Returns PVH memmap
+
+  @param Entries      Pointer to PVH memmap
+  @param Count        Number of entries
+
+  @return EFI_STATUS
+**/
+EFI_STATUS
+GetPvhMemmapEntries (
+  struct hvm_memmap_table_entry  **Entries,
+  UINT32                         *Count
+  )
+{
+  UINT32                 *PVHResetVectorData;
+  struct hvm_start_info  *pvh_start_info;
+
+  PVHResetVectorData = (VOID *)(UINTN)PcdGet32 (PcdXenPvhStartOfDayStructPtr);
+  if (PVHResetVectorData == 0) {
+    return EFI_NOT_FOUND;
+  }
+
+  pvh_start_info = (struct hvm_start_info *)(UINTN)PVHResetVectorData[0];
+
+  *Entries = (struct hvm_memmap_table_entry *)(UINTN)pvh_start_info->memmap_paddr;
+  *Count   = pvh_start_info->memmap_entries;
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+UINT64
+GetHighestSystemMemoryAddressFromPvhMemmap (
+  BOOLEAN  Below4gb
+  )
+{
+  struct hvm_memmap_table_entry  *Memmap;
+  UINT32                         MemmapEntriesCount;
+  struct hvm_memmap_table_entry  *Entry;
+  EFI_STATUS                     Status;
+  UINT32                         Loop;
+  UINT64                         HighestAddress;
+  UINT64                         EntryEnd;
+
+  HighestAddress = 0;
+
+  Status = GetPvhMemmapEntries (&Memmap, &MemmapEntriesCount);
+  ASSERT_EFI_ERROR (Status);
+
+  for (Loop = 0; Loop < MemmapEntriesCount; Loop++) {
+    Entry    = Memmap + Loop;
+    EntryEnd = Entry->addr + Entry->size;
+
+    if ((Entry->type == XEN_HVM_MEMMAP_TYPE_RAM) &&
+        (EntryEnd > HighestAddress))
+    {
+      if (Below4gb && (EntryEnd <= BASE_4GB)) {
+        HighestAddress = EntryEnd;
+      } else if (!Below4gb && (EntryEnd >= BASE_4GB)) {
+        HighestAddress = EntryEnd;
+      }
+    }
+  }
+
+  return HighestAddress;
+}
+
 UINT32
 GetSystemMemorySizeBelow4gb (
   VOID
@@ -325,6 +393,11 @@ GetSystemMemorySizeBelow4gb (
   UINT8       Cmos0x34;
   UINT8       Cmos0x35;
 
+  if (mHostBridgeDevId == CLOUDHV_DEVICE_ID) {
+    // Get the information from PVH memmap
+    return (UINT32)GetHighestSystemMemoryAddressFromPvhMemmap (TRUE);
+  }
+
   Status = ScanOrAdd64BitE820Ram (FALSE, &LowerMemorySize, NULL);
   if ((Status == EFI_SUCCESS) && (LowerMemorySize > 0)) {
     return (UINT32)LowerMemorySize;
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 8ef404168c..212aa7b047 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -91,6 +91,8 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
+  gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtr
+  gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtrSize
   gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
-- 
2.32.0

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 7/7] OvmfPkg: CloudHv: Add README
  2022-02-25 10:45 [PATCH v4 0/7] CloudHv: Rely on PVH boot specification Boeuf, Sebastien
                   ` (5 preceding siblings ...)
  2022-02-25 10:45 ` [PATCH v4 6/7] OvmfPkg: CloudHv: Rely on PVH memmap instead of CMOS Boeuf, Sebastien
@ 2022-02-25 10:45 ` Boeuf, Sebastien
  6 siblings, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 10:45 UTC (permalink / raw)
  To: devel; +Cc: jiewen.yao, jordan.l.justen, kraxel, sebastien.boeuf

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

Add some documentation to the CloudHv target in order to clarify how to
use it and what to expect from it.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 OvmfPkg/CloudHv/README | 67 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 OvmfPkg/CloudHv/README

diff --git a/OvmfPkg/CloudHv/README b/OvmfPkg/CloudHv/README
new file mode 100644
index 0000000000..63e28860e0
--- /dev/null
+++ b/OvmfPkg/CloudHv/README
@@ -0,0 +1,67 @@
+
+CloudHv is a port of OVMF for the Cloud Hypervisor project.
+
+The Cloud Hypervisor project
+----------------------------
+
+Cloud Hypervisor is a Virtual Machine Monitor that runs on top of KVM. The
+project focuses on exclusively running modern, cloud workloads, on top of a
+limited set of hardware architectures and platforms. Cloud workloads refers to
+those that are usually run by customers inside a cloud provider. This means
+modern operating systems with most I/O handled by paravirtualised devices
+(i.e. virtio), no requirement for legacy devices, and 64-bit CPUs.
+
+https://github.com/cloud-hypervisor/cloud-hypervisor
+
+Design
+------
+
+Based on Cloud Hypervisor's motto to reduce the emulation as much as possible,
+the project logically decided to support the PVH boot specification as the only
+way of booting virtual machines. That includes both direct kernel boot and OVMF
+firmware which must be generated as PVH ELF binaries.
+PVH allows information like location of ACPI tables and location of guest RAM
+ranges to be shared without the need of an extra emulated device like a CMOS.
+
+Features
+--------
+
+* Serial console
+* EFI shell
+* virtio-pci
+
+Build
+-----
+
+The way to build the CloudHv target is as follows:
+
+OvmfPkg/build.sh -p OvmfPkg/CloudHv/CloudHvX64.dsc -a X64 -b DEBUG
+
+Usage
+-----
+
+Assuming Cloud Hypervisor is already built, one can start a virtual machine as
+follows:
+
+./cloud-hypervisor \
+    --cpus boot=1 \
+    --memory size=1G \
+    --kernel Build/CloudHvX64/DEBUG_GCC5/FV/CLOUDHV.fd \
+    --disk path=/path/to/disk.raw
+
+Releases
+--------
+
+In edk2-stable202202, CloudHv is generated as data-only binary.
+Starting with edk2-stable202205, CloudHv is generated as a PVH ELF binary to
+reduce the amount of emulation needed from Cloud Hypervisor.
+For TDX, things are handled differently and PVH is not used, which is why the
+firmware is always generated as a data-only binary.
+
++-------------------+----------------+
+|                   |    CloudHv     |
++-------------------+----------------+
+| edk2-stable202202 | Data binary    |
++-------------------+----------------+
+| edk2-stable202205 | PVH ELF binary |
++-------------------+----------------+
-- 
2.32.0

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-02-25 10:45 ` [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary Boeuf, Sebastien
@ 2022-02-25 13:00   ` Gerd Hoffmann
  2022-02-25 14:00     ` [edk2-devel] " Boeuf, Sebastien
  0 siblings, 1 reply; 18+ messages in thread
From: Gerd Hoffmann @ 2022-02-25 13:00 UTC (permalink / raw)
  To: sebastien.boeuf; +Cc: devel, jiewen.yao, jordan.l.justen

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-02-25 13:00   ` Gerd Hoffmann
@ 2022-02-25 14:00     ` Boeuf, Sebastien
  2022-02-28  7:08       ` Gerd Hoffmann
  0 siblings, 1 reply; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-25 14:00 UTC (permalink / raw)
  To: kraxel@redhat.com, devel@edk2.groups.io; +Cc: Yao, Jiewen, Justen, Jordan L

On Fri, 2022-02-25 at 14:00 +0100, Gerd Hoffmann wrote:
> > +++ 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.

I wanted to make sure it would only be enabled for this very specific
case.

> 
> >  !include OvmfPkg/VarStore.fdf.inc
> > +DEFINE PVH_HEADER_CLOUDHV = FALSE
> 
> No need to flip that back to FALSE.

Well if I don't flip it back, the [FD.CLOUDHV_VARS] section is build as
a PVH ELF binary as well.
 
> 
> > +!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).

Sorry it's not entirely clear to me what should be done here. Should I
remove the VARS section since we're not using it anyway?
I'm lacking some knowledge here to understand what this is used for,
and how I actually broke it. I mean with the define and the include, I
ended up modifying exclusively the [FD.CLOUDHV] which is exactly what
was done for Xen. Do you think the Xen VARS section is broken as well?

> 
> take care,
>   Gerd
> 
> 
> 
> 
> 
> 

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-02-25 14:00     ` [edk2-devel] " Boeuf, Sebastien
@ 2022-02-28  7:08       ` Gerd Hoffmann
  2022-02-28  8:15         ` Boeuf, Sebastien
       [not found]         ` <16D7E5267B34FC87.32375@groups.io>
  0 siblings, 2 replies; 18+ messages in thread
From: Gerd Hoffmann @ 2022-02-28  7:08 UTC (permalink / raw)
  To: Boeuf, Sebastien; +Cc: devel@edk2.groups.io, Yao, Jiewen, Justen, Jordan L

  Hi,

> > 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).
> 
> Sorry it's not entirely clear to me what should be done here. Should I
> remove the VARS section since we're not using it anyway?
> I'm lacking some knowledge here to understand what this is used for,
> and how I actually broke it. I mean with the define and the include, I
> ended up modifying exclusively the [FD.CLOUDHV] which is exactly what
> was done for Xen. Do you think the Xen VARS section is broken as well?

Probably.  Not sure how xen uses the generated firmware images though.

OVMF has three images.  The OVMF_VARS.fd is the variable store.
OVMF_CODE.fd is the firmware code.  Both combined is OVMF.fd.  The split
into VARS and CODE exists to simplify firmware updates:  On the host
machine you'll have a per-guest VARS file with the persistent uefi
variables, and the shared CODE file.  Firmware upgrades work by simply
updating the shared CODE file.

The address space layout is the same in both cases (combined/splitted).
The firmware takes 2M or 4M below 4G.  The upper part is covered by the
CODE, the lower part is covered by VARS.  OVMF expects the varstore at
a fixes address, but as mentioned can cope with the varstore not being
there or not being backed by flash.

So, do you use the CODE or VARS file for cloudhv?
Does cloudhv emulate flash?

take care,
  Gerd


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-02-28  7:08       ` Gerd Hoffmann
@ 2022-02-28  8:15         ` Boeuf, Sebastien
       [not found]         ` <16D7E5267B34FC87.32375@groups.io>
  1 sibling, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-28  8:15 UTC (permalink / raw)
  To: kraxel@redhat.com, devel@edk2.groups.io; +Cc: Yao, Jiewen, Justen, Jordan L

On Mon, 2022-02-28 at 08:08 +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > 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).
> > 
> > Sorry it's not entirely clear to me what should be done here.
> > Should I
> > remove the VARS section since we're not using it anyway?
> > I'm lacking some knowledge here to understand what this is used
> > for,
> > and how I actually broke it. I mean with the define and the
> > include, I
> > ended up modifying exclusively the [FD.CLOUDHV] which is exactly
> > what
> > was done for Xen. Do you think the Xen VARS section is broken as
> > well?
> 
> Probably.  Not sure how xen uses the generated firmware images
> though.
> 
> OVMF has three images.  The OVMF_VARS.fd is the variable store.
> OVMF_CODE.fd is the firmware code.  Both combined is OVMF.fd.  The
> split
> into VARS and CODE exists to simplify firmware updates:  On the host
> machine you'll have a per-guest VARS file with the persistent uefi
> variables, and the shared CODE file.  Firmware upgrades work by
> simply
> updating the shared CODE file.
> 
> The address space layout is the same in both cases
> (combined/splitted).
> The firmware takes 2M or 4M below 4G.  The upper part is covered by
> the
> CODE, the lower part is covered by VARS.  OVMF expects the varstore
> at
> a fixes address, but as mentioned can cope with the varstore not
> being
> there or not being backed by flash.

Thanks for the explanation, it helps :)

> 
> So, do you use the CODE or VARS file for cloudhv?

No we don't.

> Does cloudhv emulate flash?

No it doesn't.

> 
> take care,
>   Gerd
> 
> 
> 
> 
> 
> 

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
       [not found]         ` <16D7E5267B34FC87.32375@groups.io>
@ 2022-02-28 15:12           ` Boeuf, Sebastien
  2022-03-01  7:16             ` Gerd Hoffmann
  0 siblings, 1 reply; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-02-28 15:12 UTC (permalink / raw)
  To: kraxel@redhat.com, devel@edk2.groups.io; +Cc: Yao, Jiewen, Justen, Jordan L

So what do you think I should do with this patch?

Thanks,
Sebastien

On Mon, 2022-02-28 at 08:15 +0000, Boeuf, Sebastien wrote:
> On Mon, 2022-02-28 at 08:08 +0100, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > > 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).
> > > 
> > > Sorry it's not entirely clear to me what should be done here.
> > > Should I
> > > remove the VARS section since we're not using it anyway?
> > > I'm lacking some knowledge here to understand what this is used
> > > for,
> > > and how I actually broke it. I mean with the define and the
> > > include, I
> > > ended up modifying exclusively the [FD.CLOUDHV] which is exactly
> > > what
> > > was done for Xen. Do you think the Xen VARS section is broken as
> > > well?
> > 
> > Probably.  Not sure how xen uses the generated firmware images
> > though.
> > 
> > OVMF has three images.  The OVMF_VARS.fd is the variable store.
> > OVMF_CODE.fd is the firmware code.  Both combined is OVMF.fd.  The
> > split
> > into VARS and CODE exists to simplify firmware updates:  On the
> > host
> > machine you'll have a per-guest VARS file with the persistent uefi
> > variables, and the shared CODE file.  Firmware upgrades work by
> > simply
> > updating the shared CODE file.
> > 
> > The address space layout is the same in both cases
> > (combined/splitted).
> > The firmware takes 2M or 4M below 4G.  The upper part is covered by
> > the
> > CODE, the lower part is covered by VARS.  OVMF expects the varstore
> > at
> > a fixes address, but as mentioned can cope with the varstore not
> > being
> > there or not being backed by flash.
> 
> Thanks for the explanation, it helps :)
> 
> > 
> > So, do you use the CODE or VARS file for cloudhv?
> 
> No we don't.
> 
> > Does cloudhv emulate flash?
> 
> No it doesn't.
> 
> > 
> > take care,
> >   Gerd
> > 
> > 
> > 
> > 
> > 
> > 
> 
> ---------------------------------------------------------------------
> Intel Corporation SAS (French simplified joint stock company)
> Registered headquarters: "Les Montalets"- 2, rue de Paris, 
> 92196 Meudon Cedex, France
> Registration Number:  302 456 199 R.C.S. NANTERRE
> Capital: 4,572,000 Euros
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> 
> 
> 
> 
> 

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-02-28 15:12           ` Boeuf, Sebastien
@ 2022-03-01  7:16             ` Gerd Hoffmann
  2022-03-01  8:53               ` Boeuf, Sebastien
       [not found]               ` <16D835C7D9FE3DA1.466@groups.io>
  0 siblings, 2 replies; 18+ messages in thread
From: Gerd Hoffmann @ 2022-03-01  7:16 UTC (permalink / raw)
  To: devel, sebastien.boeuf; +Cc: Yao, Jiewen, Justen, Jordan L

On Mon, Feb 28, 2022 at 03:12:53PM +0000, Boeuf, Sebastien wrote:
> So what do you think I should do with this patch?

I think you can:

  (1) drop FD.CLOUDHV_VARS
  (2) drop FD.CLOUDHV_CODE
  (3) make VARS_SIZE smaller (just enough for the pvh header),
  (4) make FVMAIN_SIZE larger (so the total size remains the same).
  (5) possibly rename VARS_SIZE.  It's not about VARS any more.  Not
      sure this is worth the churn and how many places you need to touch
      for this, maybe a big'n'fat comment instead will do.

HTH,
  Gerd


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-03-01  7:16             ` Gerd Hoffmann
@ 2022-03-01  8:53               ` Boeuf, Sebastien
  2022-03-01 11:28                 ` Gerd Hoffmann
       [not found]               ` <16D835C7D9FE3DA1.466@groups.io>
  1 sibling, 1 reply; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-03-01  8:53 UTC (permalink / raw)
  To: kraxel@redhat.com, devel@edk2.groups.io; +Cc: Yao, Jiewen, Justen, Jordan L

On Tue, 2022-03-01 at 08:16 +0100, Gerd Hoffmann wrote:
> On Mon, Feb 28, 2022 at 03:12:53PM +0000, Boeuf, Sebastien wrote:
> > So what do you think I should do with this patch?
> 
> I think you can:
> 
>   (1) drop FD.CLOUDHV_VARS
Ok
>   (2) drop FD.CLOUDHV_CODE
Ok
>   (3) make VARS_SIZE smaller (just enough for the pvh header),

Ok. Do you want me to keep the factorization in VarStore.fdf.inc?

>   (4) make FVMAIN_SIZE larger (so the total size remains the same).
Ok
>   (5) possibly rename VARS_SIZE.  It's not about VARS any more.  Not
>       sure this is worth the churn and how many places you need to
> touch
>       for this, maybe a big'n'fat comment instead will do.

Ok I'll check how feasible this is.
> 
> HTH,
>   Gerd
> 
> 
> 
> 
> 
> 

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
       [not found]               ` <16D835C7D9FE3DA1.466@groups.io>
@ 2022-03-01  9:17                 ` Boeuf, Sebastien
  0 siblings, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-03-01  9:17 UTC (permalink / raw)
  To: kraxel@redhat.com, devel@edk2.groups.io; +Cc: Yao, Jiewen, Justen, Jordan L

On Tue, 2022-03-01 at 08:53 +0000, Boeuf, Sebastien wrote:
> On Tue, 2022-03-01 at 08:16 +0100, Gerd Hoffmann wrote:
> > On Mon, Feb 28, 2022 at 03:12:53PM +0000, Boeuf, Sebastien wrote:
> > > So what do you think I should do with this patch?
> > 
> > I think you can:
> > 
> >   (1) drop FD.CLOUDHV_VARS
> Ok
> >   (2) drop FD.CLOUDHV_CODE
> Ok
> >   (3) make VARS_SIZE smaller (just enough for the pvh header),
> 
> Ok. Do you want me to keep the factorization in VarStore.fdf.inc?
> 
> >   (4) make FVMAIN_SIZE larger (so the total size remains the same).
> Ok
> >   (5) possibly rename VARS_SIZE.  It's not about VARS any more. 
> > Not
> >       sure this is worth the churn and how many places you need to
> > touch
> >       for this, maybe a big'n'fat comment instead will do.
> 
> Ok I'll check how feasible this is.

Or I can simply do 1 and 2, but leave 3,4,5 so that I don't make
everything more complex. I mean I don't really need the FVMAIN_SIZE to
be larger, and by keeping it the way it is (both VARS_SIZE and
FVMAIN_SIZE), I can rely on VarStore.fdf.inc.

> > 
> > HTH,
> >   Gerd
> > 
> > 
> > 
> > 
> > 
> > 
> 
> ---------------------------------------------------------------------
> Intel Corporation SAS (French simplified joint stock company)
> Registered headquarters: "Les Montalets"- 2, rue de Paris, 
> 92196 Meudon Cedex, France
> Registration Number:  302 456 199 R.C.S. NANTERRE
> Capital: 4,572,000 Euros
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> 
> 
> 
> 
> 

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-03-01  8:53               ` Boeuf, Sebastien
@ 2022-03-01 11:28                 ` Gerd Hoffmann
  2022-03-01 13:30                   ` Boeuf, Sebastien
  0 siblings, 1 reply; 18+ messages in thread
From: Gerd Hoffmann @ 2022-03-01 11:28 UTC (permalink / raw)
  To: Boeuf, Sebastien; +Cc: devel@edk2.groups.io, Yao, Jiewen, Justen, Jordan L

On Tue, Mar 01, 2022 at 08:53:10AM +0000, Boeuf, Sebastien wrote:
> On Tue, 2022-03-01 at 08:16 +0100, Gerd Hoffmann wrote:
> > On Mon, Feb 28, 2022 at 03:12:53PM +0000, Boeuf, Sebastien wrote:
> > > So what do you think I should do with this patch?
> > 
> > I think you can:
> > 
> >   (1) drop FD.CLOUDHV_VARS
> Ok
> >   (2) drop FD.CLOUDHV_CODE
> Ok
> >   (3) make VARS_SIZE smaller (just enough for the pvh header),
> 
> Ok. Do you want me to keep the factorization in VarStore.fdf.inc?

I think you don't need VarStore.fdf.inc? at all if you don't have a
varstore anyway.

take care,
  Gerd


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [edk2-devel] [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary
  2022-03-01 11:28                 ` Gerd Hoffmann
@ 2022-03-01 13:30                   ` Boeuf, Sebastien
  0 siblings, 0 replies; 18+ messages in thread
From: Boeuf, Sebastien @ 2022-03-01 13:30 UTC (permalink / raw)
  To: kraxel@redhat.com; +Cc: Yao, Jiewen, Justen, Jordan L, devel@edk2.groups.io

On Tue, 2022-03-01 at 12:28 +0100, kraxel@redhat.com wrote:
> On Tue, Mar 01, 2022 at 08:53:10AM +0000, Boeuf, Sebastien wrote:
> > On Tue, 2022-03-01 at 08:16 +0100, Gerd Hoffmann wrote:
> > > On Mon, Feb 28, 2022 at 03:12:53PM +0000, Boeuf, Sebastien wrote:
> > > > So what do you think I should do with this patch?
> > > 
> > > I think you can:
> > > 
> > >   (1) drop FD.CLOUDHV_VARS
> > Ok
> > >   (2) drop FD.CLOUDHV_CODE
> > Ok
> > >   (3) make VARS_SIZE smaller (just enough for the pvh header),
> > 
> > Ok. Do you want me to keep the factorization in VarStore.fdf.inc?
> 
> I think you don't need VarStore.fdf.inc? at all if you don't have a
> varstore anyway.

Ok I took care of it in the v5.

> 
> take care,
>   Gerd
> 

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-03-01 13:30 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-25 10:45 [PATCH v4 0/7] CloudHv: Rely on PVH boot specification Boeuf, Sebastien
2022-02-25 10:45 ` [PATCH v4 1/7] OvmfPkg: Make the Xen ELF header generator more flexible Boeuf, Sebastien
2022-02-25 10:45 ` [PATCH v4 2/7] OvmfPkg: Xen: Use a new fdf include for the PVH ELF header Boeuf, Sebastien
2022-02-25 10:45 ` [PATCH v4 3/7] OvmfPkg: Xen: Generate fdf include file from ELF header generator Boeuf, Sebastien
2022-02-25 10:45 ` [PATCH v4 4/7] OvmfPkg: Generate CloudHv as a PVH ELF binary Boeuf, Sebastien
2022-02-25 13:00   ` Gerd Hoffmann
2022-02-25 14:00     ` [edk2-devel] " Boeuf, Sebastien
2022-02-28  7:08       ` Gerd Hoffmann
2022-02-28  8:15         ` Boeuf, Sebastien
     [not found]         ` <16D7E5267B34FC87.32375@groups.io>
2022-02-28 15:12           ` Boeuf, Sebastien
2022-03-01  7:16             ` Gerd Hoffmann
2022-03-01  8:53               ` Boeuf, Sebastien
2022-03-01 11:28                 ` Gerd Hoffmann
2022-03-01 13:30                   ` Boeuf, Sebastien
     [not found]               ` <16D835C7D9FE3DA1.466@groups.io>
2022-03-01  9:17                 ` Boeuf, Sebastien
2022-02-25 10:45 ` [PATCH v4 5/7] OvmfPkg: CloudHv: Retrieve RSDP address from PVH Boeuf, Sebastien
2022-02-25 10:45 ` [PATCH v4 6/7] OvmfPkg: CloudHv: Rely on PVH memmap instead of CMOS Boeuf, Sebastien
2022-02-25 10:45 ` [PATCH v4 7/7] OvmfPkg: CloudHv: Add README Boeuf, Sebastien

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox