[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/2] x86: Add support for building a multiboot2 PE binary


  • To: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 8 Apr 2024 12:25:37 +0200
  • Autocrypt: addr=jbeulich@xxxxxxxx; keydata= xsDiBFk3nEQRBADAEaSw6zC/EJkiwGPXbWtPxl2xCdSoeepS07jW8UgcHNurfHvUzogEq5xk hu507c3BarVjyWCJOylMNR98Yd8VqD9UfmX0Hb8/BrA+Hl6/DB/eqGptrf4BSRwcZQM32aZK 7Pj2XbGWIUrZrd70x1eAP9QE3P79Y2oLrsCgbZJfEwCgvz9JjGmQqQkRiTVzlZVCJYcyGGsD /0tbFCzD2h20ahe8rC1gbb3K3qk+LpBtvjBu1RY9drYk0NymiGbJWZgab6t1jM7sk2vuf0Py O9Hf9XBmK0uE9IgMaiCpc32XV9oASz6UJebwkX+zF2jG5I1BfnO9g7KlotcA/v5ClMjgo6Gl MDY4HxoSRu3i1cqqSDtVlt+AOVBJBACrZcnHAUSuCXBPy0jOlBhxPqRWv6ND4c9PH1xjQ3NP nxJuMBS8rnNg22uyfAgmBKNLpLgAGVRMZGaGoJObGf72s6TeIqKJo/LtggAS9qAUiuKVnygo 3wjfkS9A3DRO+SpU7JqWdsveeIQyeyEJ/8PTowmSQLakF+3fote9ybzd880fSmFuIEJldWxp Y2ggPGpiZXVsaWNoQHN1c2UuY29tPsJgBBMRAgAgBQJZN5xEAhsDBgsJCAcDAgQVAggDBBYC AwECHgECF4AACgkQoDSui/t3IH4J+wCfQ5jHdEjCRHj23O/5ttg9r9OIruwAn3103WUITZee e7Sbg12UgcQ5lv7SzsFNBFk3nEQQCACCuTjCjFOUdi5Nm244F+78kLghRcin/awv+IrTcIWF hUpSs1Y91iQQ7KItirz5uwCPlwejSJDQJLIS+QtJHaXDXeV6NI0Uef1hP20+y8qydDiVkv6l IreXjTb7DvksRgJNvCkWtYnlS3mYvQ9NzS9PhyALWbXnH6sIJd2O9lKS1Mrfq+y0IXCP10eS FFGg+Av3IQeFatkJAyju0PPthyTqxSI4lZYuJVPknzgaeuJv/2NccrPvmeDg6Coe7ZIeQ8Yj t0ARxu2xytAkkLCel1Lz1WLmwLstV30g80nkgZf/wr+/BXJW/oIvRlonUkxv+IbBM3dX2OV8 AmRv1ySWPTP7AAMFB/9PQK/VtlNUJvg8GXj9ootzrteGfVZVVT4XBJkfwBcpC/XcPzldjv+3 HYudvpdNK3lLujXeA5fLOH+Z/G9WBc5pFVSMocI71I8bT8lIAzreg0WvkWg5V2WZsUMlnDL9 mpwIGFhlbM3gfDMs7MPMu8YQRFVdUvtSpaAs8OFfGQ0ia3LGZcjA6Ik2+xcqscEJzNH+qh8V m5jjp28yZgaqTaRbg3M/+MTbMpicpZuqF4rnB0AQD12/3BNWDR6bmh+EkYSMcEIpQmBM51qM EKYTQGybRCjpnKHGOxG0rfFY1085mBDZCH5Kx0cl0HVJuQKC+dV2ZY5AqjcKwAxpE75MLFkr wkkEGBECAAkFAlk3nEQCGwwACgkQoDSui/t3IH7nnwCfcJWUDUFKdCsBH/E5d+0ZnMQi+G0A nAuWpQkjM1ASeQwSHEeAWPgskBQL
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 08 Apr 2024 10:25:41 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 28.03.2024 16:11, Ross Lagerwall wrote:
> In addition to building xen.efi and xen.gz, build xen-mbi.exe. The
> latter is a PE binary that can be used with a multiboot2 loader that
> supports loading PE binaries.

I have to admit I find .exe a strange extension outside of the Windows
world. Would it be an option to have no extension at all (xen-mbi), or
use xen.mbi?

> Using this option allows the binary to be signed and verified by Shim.
> This means the same xen-mbi.exe binary can then be used for BIOS boot,
> UEFI Boot and UEFI boot with Secure Boot verification (all with the
> convenience of GRUB2 as a bootloader).

With which "UEFI boot" really means "chainloader" from grub? That isn't
required though, is it? I.e. "UEFI boot" ought to work also without
involving grub?

> The new binary is created by modifying xen.efi:
> * Relocations are stripped since they are not needed.

Because of the changed entry point, aiui. What hasn't become clear to
me is what difference in functionality there's going to be between
booting this new image in "UEFI boot" mode vs using xen.efi. Clearly
xen.efi's own (EFI-level) command line options won't be available. But
it feels like there might be more.

> * The image base address is set to 0 since it must necessarily be below
>   4 GiB and the loader will relocate it anyway.

While technically okay, what is the reason for this adjustment? 

> * The PE entry point is set to the multiboot2 entry point rather than
>   the normal EFI entry point. This is only relevant for BIOS boot since
>   for EFI boot the entry point is specified via a multiboot2 tag.

Hmm, I may then be confused about the terminology further up: What is
"BIOS boot" then? So far I've taken that as the equivalent of xen.gz's
way of being booted (i.e. grub without EFI underneath).

> @@ -123,6 +124,19 @@ syms-warn-dup-$(CONFIG_ENFORCE_UNIQUE_SYMBOLS) := 
> --error-dup
>  
>  orphan-handling-$(call ld-option,--orphan-handling=warn) += 
> --orphan-handling=warn
>  
> +ifeq ($(XEN_BUILD_PE),y)
> +$(TARGET)-mbi.exe: $(TARGET).efi $(obj)/efi/modify-mbi-exe
> +     $(OBJCOPY) --remove-section=.reloc $< $@.tmp
> +     $(obj)/efi/modify-mbi-exe $@.tmp
> +     $(OBJCOPY) --set-start=0x$$($(NM) -pa $@.tmp | awk '/T start$$/{print 
> $$1}') $@.tmp $@.tmp2

I understand section removal is better done by objcopy. Changing the entry
point could be done by the new tool, though (by passing it the designated
address)?

As to stripping .reloc - generally this needs accompanying by setting the
"relocations stripped" flag in the COFF(?) header. Here, however, I take
it this is not only not needed, but actually not wanted. This imo wants
saying somewhere; the individual steps done here could do with brief
comments anyway, imo.

> --- /dev/null
> +++ b/xen/arch/x86/efi/modify-mbi-exe.c
> @@ -0,0 +1,77 @@
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +
> +struct mz_hdr {
> +    uint16_t signature;
> +#define MZ_SIGNATURE 0x5a4d
> +    uint16_t last_page_size;
> +    uint16_t page_count;
> +    uint16_t relocation_count;
> +    uint16_t header_paras;
> +    uint16_t min_paras;
> +    uint16_t max_paras;
> +    uint16_t entry_ss;
> +    uint16_t entry_sp;
> +    uint16_t checksum;
> +    uint16_t entry_ip;
> +    uint16_t entry_cs;
> +    uint16_t relocations;
> +    uint16_t overlay;
> +    uint8_t reserved[32];
> +    uint32_t extended_header_base;
> +};
> +
> +struct coff_hdr {
> +    uint32_t signature;
> +    uint16_t cpu;
> +    uint16_t section_count;
> +    int32_t timestamp;
> +    uint32_t symbols_file_offset;
> +    uint32_t symbol_count;
> +    uint16_t opt_hdr_size;
> +    uint16_t flags;
> +};

I can't spot any use of this struct.

Jan

> +#define IMAGE_BASE_OFFSET 48
> +#define NEW_IMAGE_BASE 0x0
> +
> +int main(int argc, char **argv)
> +{
> +    int fd;
> +    struct mz_hdr mz_hdr;
> +    const uint64_t base_addr = NEW_IMAGE_BASE;
> +
> +    if ( argc != 2 )
> +    {
> +        fprintf(stderr, "usage: %s <image>\n", argv[0]);
> +        return 1;
> +    }
> +
> +    fd = open(argv[1], O_RDWR);
> +    if ( fd < 0 ||
> +         read(fd, &mz_hdr, sizeof(mz_hdr)) != sizeof(mz_hdr) )
> +    {
> +        perror(argv[1]);
> +        return 2;
> +    }
> +
> +    if ( mz_hdr.signature != MZ_SIGNATURE ||
> +         !mz_hdr.extended_header_base )
> +    {
> +        fprintf(stderr, "%s: Wrong DOS file format\n", argv[1]);
> +        return 2;
> +    }
> +
> +    if ( lseek(fd, mz_hdr.extended_header_base + IMAGE_BASE_OFFSET, 
> SEEK_SET) < 0 ||
> +         write(fd, &base_addr, sizeof(base_addr)) != sizeof(base_addr) )
> +    {
> +        perror(argv[1]);
> +        return 3;
> +    }
> +
> +    close(fd);
> +
> +    return 0;
> +}




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.