Re: [PATCH 6/8] xenpvh: support building a standalone image

[Daniel Kiper]

On Wed, Nov 29, 2017 at 02:46:48PM +0100, Juergen Gross wrote:
> Suppor mkimage for xenpvh.
>
> Signed-off-by: Juergen Gross <address@hidden>
> ---
>  include/grub/util/mkimage.h |  3 ++-
>  util/grub-mkimage32.c       |  1 +
>  util/grub-mkimage64.c       |  1 +
>  util/grub-mkimagexx.c       | 44 ++++++++++++++++++++++++++++++++++++++++----
>  util/mkimage.c              | 23 ++++++++++++++++++++++-
>  5 files changed, 66 insertions(+), 6 deletions(-)
>
> diff --git a/include/grub/util/mkimage.h b/include/grub/util/mkimage.h
> index b3a5ca132..3f5bc2e00 100644
> --- a/include/grub/util/mkimage.h
> +++ b/include/grub/util/mkimage.h
> @@ -71,7 +71,8 @@ struct grub_install_image_target_desc
>      IMAGE_I386_IEEE1275,
>      IMAGE_LOONGSON_ELF, IMAGE_QEMU, IMAGE_PPC, IMAGE_YEELOONG_FLASH,
>      IMAGE_FULOONG2F_FLASH, IMAGE_I386_PC_PXE, IMAGE_MIPS_ARC,
> -    IMAGE_QEMU_MIPS_FLASH, IMAGE_UBOOT, IMAGE_XEN, IMAGE_I386_PC_ELTORITO
> +    IMAGE_QEMU_MIPS_FLASH, IMAGE_UBOOT, IMAGE_XEN, IMAGE_I386_PC_ELTORITO,
> +    IMAGE_XENPVH
>    } id;
>    enum
>      {
> diff --git a/util/grub-mkimage32.c b/util/grub-mkimage32.c
> index 9b31397bc..4253c4897 100644
> --- a/util/grub-mkimage32.c
> +++ b/util/grub-mkimage32.c
> @@ -18,5 +18,6 @@
>  # define ELF_R_TYPE(val)             ELF32_R_TYPE(val)
>  # define ELF_ST_TYPE(val)            ELF32_ST_TYPE(val)

Please add empty line here...

>  #define XEN_NOTE_SIZE 132
> +#define XENPVH_NOTE_SIZE 20

...and align the numbers for XEN_NOTE_SIZE and XENPVH_NOTE_SIZE.

>
>  #include "grub-mkimagexx.c"
> diff --git a/util/grub-mkimage64.c b/util/grub-mkimage64.c
> index d83345924..c862be8c0 100644
> --- a/util/grub-mkimage64.c
> +++ b/util/grub-mkimage64.c
> @@ -18,5 +18,6 @@
>  # define ELF_R_TYPE(val)             ELF64_R_TYPE(val)
>  # define ELF_ST_TYPE(val)            ELF64_ST_TYPE(val)
>  #define XEN_NOTE_SIZE 120
> +#define XENPVH_NOTE_SIZE 24

Ditto.

>
>  #include "grub-mkimagexx.c"
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index a2bb05439..a024e57b6 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -207,12 +207,12 @@ SUFFIX (grub_mkimage_generate_elf) (const struct 
> grub_install_image_target_desc
>        phnum++;
>        footer_size += sizeof (struct grub_ieee1275_note);
>      }
> -  if (image_target->id == IMAGE_XEN)
> +  if (image_target->id == IMAGE_XEN || image_target->id == IMAGE_XENPVH)
>      {
>        phnum++;
>        shnum++;
>        string_size += sizeof (".xen");
> -      footer_size += XEN_NOTE_SIZE;
> +      footer_size += (image_target->id == IMAGE_XEN) ? XEN_NOTE_SIZE : 
> XENPVH_NOTE_SIZE;
>      }
>    header_size = ALIGN_UP (sizeof (*ehdr) + phnum * sizeof (*phdr)
>                         + shnum * sizeof (*shdr) + string_size, 
> layout->align);
> @@ -399,6 +399,39 @@ SUFFIX (grub_mkimage_generate_elf) (const struct 
> grub_install_image_target_desc
>        phdr->p_offset = grub_host_to_target32 (header_size + program_size);
>      }
>
> +  if (image_target->id == IMAGE_XENPVH)
> +    {
> +      char *note_start = (elf_img + program_size + header_size);
> +      Elf_Nhdr *note_ptr;
> +      char *ptr = (char *) note_start;
> +
> +      grub_util_info ("adding XEN NOTE segment");
> +
> +      /* Phys32 Entry.  */
> +      note_ptr = (Elf_Nhdr *) ptr;
> +      note_ptr->n_namesz = grub_host_to_target32 (sizeof 
> (GRUB_XEN_NOTE_NAME));
> +      note_ptr->n_descsz = grub_host_to_target32 
> (image_target->voidp_sizeof);
> +      note_ptr->n_type = grub_host_to_target32 (18);

What 18 means? Could you use an existing constant or if not define one?

> +      ptr += sizeof (Elf_Nhdr);
> +      memcpy (ptr, GRUB_XEN_NOTE_NAME, sizeof (GRUB_XEN_NOTE_NAME));
> +      ptr += ALIGN_UP (sizeof (GRUB_XEN_NOTE_NAME), 4);
> +      memset (ptr, 0, image_target->voidp_sizeof);
> +      *(grub_uint32_t *) ptr = GRUB_KERNEL_I386_XENPVH_LINK_ADDR;
> +      ptr += image_target->voidp_sizeof;
> +
> +      assert (XENPVH_NOTE_SIZE == (ptr - note_start));
> +
> +      phdr++;
> +      phdr->p_type = grub_host_to_target32 (PT_NOTE);
> +      phdr->p_flags = grub_host_to_target32 (PF_R);
> +      phdr->p_align = grub_host_to_target32 (image_target->voidp_sizeof);
> +      phdr->p_vaddr = 0;
> +      phdr->p_paddr = 0;
> +      phdr->p_filesz = grub_host_to_target32 (XENPVH_NOTE_SIZE);
> +      phdr->p_memsz = 0;
> +      phdr->p_offset = grub_host_to_target32 (header_size + program_size);
> +    }
> +
>    if (note)
>      {
>        int note_size = sizeof (struct grub_ieee1275_note);
> @@ -474,7 +507,7 @@ SUFFIX (grub_mkimage_generate_elf) (const struct 
> grub_install_image_target_desc
>      shdr->sh_entsize = grub_host_to_target32 (0);
>      shdr++;
>
> -    if (image_target->id == IMAGE_XEN)
> +    if (image_target->id == IMAGE_XEN || image_target->id == IMAGE_XENPVH)
>        {
>       memcpy (ptr, ".xen", sizeof (".xen"));
>       shdr->sh_name = grub_host_to_target32 (ptr - str_start);
> @@ -482,7 +515,10 @@ SUFFIX (grub_mkimage_generate_elf) (const struct 
> grub_install_image_target_desc
>       shdr->sh_type = grub_host_to_target32 (SHT_PROGBITS);
>       shdr->sh_addr = grub_host_to_target_addr (target_addr + 
> layout->kernel_size);
>       shdr->sh_offset = grub_host_to_target_addr (program_size + header_size);
> -     shdr->sh_size = grub_host_to_target32 (XEN_NOTE_SIZE);
> +     if (image_target->id == IMAGE_XEN)
> +       shdr->sh_size = grub_host_to_target32 (XEN_NOTE_SIZE);
> +     else
> +       shdr->sh_size = grub_host_to_target32 (XENPVH_NOTE_SIZE);
>       shdr->sh_link = grub_host_to_target32 (0);
>       shdr->sh_info = grub_host_to_target32 (0);
>       shdr->sh_addralign = grub_host_to_target32 (image_target->voidp_sizeof);
> diff --git a/util/mkimage.c b/util/mkimage.c
> index e22d82afa..5e071c074 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -133,6 +133,24 @@ static const struct grub_install_image_target_desc 
> image_targets[] =
>        .default_compression = GRUB_COMPRESSION_LZMA
>      },
>      {
> +      .dirname = "i386-xenpvh",
> +      .names = { "i386-xenpvh", NULL },
> +      .voidp_sizeof = 4,
> +      .bigendian = 0,
> +      .id = IMAGE_XENPVH,
> +      .flags = PLATFORM_FLAGS_NONE,
> +      .total_module_size = TARGET_NO_FIELD,
> +      .decompressor_compressed_size = TARGET_NO_FIELD,
> +      .decompressor_uncompressed_size = TARGET_NO_FIELD,
> +      .decompressor_uncompressed_addr = TARGET_NO_FIELD,
> +      .elf_target = EM_386,
> +      .section_align = 1,
> +      .vaddr_offset = 0,
> +      .link_addr = GRUB_KERNEL_I386_XENPVH_LINK_ADDR,
> +      .mod_align = GRUB_KERNEL_I386_XENPVH_MOD_ALIGN,
> +      .link_align = 4
> +    },
> +    {
>        .dirname = "i386-pc",
>        .names = { "i386-pc-pxe", NULL },
>        .voidp_sizeof = 4,
> @@ -860,7 +878,8 @@ grub_install_generate_image (const char *dir, const char 
> *prefix,
>    else
>      kernel_img = grub_mkimage_load_image64 (kernel_path, total_module_size,
>                                           &layout, image_target);
> -  if (image_target->id == IMAGE_XEN && layout.align < 4096)
> +  if ((image_target->id == IMAGE_XEN || image_target->id == IMAGE_XENPVH) &&
> +      layout.align < 4096)
>      layout.align = 4096;

Could you use some kind of PAGE_SIZE instead of 4096 here?

Daniel