| Message ID | 20190924124433.96810-2-slp@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce the microvm machine type | expand |
Hi Sergio, On 9/24/19 2:44 PM, Sergio Lopez wrote: > Extract PVH related functions from pc.c, and put them in pvh.c, so > they can be shared with other components. > > Signed-off-by: Sergio Lopez <slp@redhat.com> > --- > hw/i386/Makefile.objs | 1 + > hw/i386/pc.c | 120 +++++------------------------------------- > hw/i386/pvh.c | 113 +++++++++++++++++++++++++++++++++++++++ > hw/i386/pvh.h | 10 ++++ > 4 files changed, 136 insertions(+), 108 deletions(-) > create mode 100644 hw/i386/pvh.c > create mode 100644 hw/i386/pvh.h > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > index 5d9c9efd5f..c5f20bbd72 100644 > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -1,5 +1,6 @@ > obj-$(CONFIG_KVM) += kvm/ > obj-y += multiboot.o > +obj-y += pvh.o > obj-y += pc.o > obj-$(CONFIG_I440FX) += pc_piix.o > obj-$(CONFIG_Q35) += pc_q35.o > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index bad866fe44..10e4ced0c6 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -42,6 +42,7 @@ > #include "elf.h" > #include "migration/vmstate.h" > #include "multiboot.h" > +#include "pvh.h" > #include "hw/timer/mc146818rtc.h" > #include "hw/dma/i8257.h" > #include "hw/timer/i8254.h" > @@ -116,9 +117,6 @@ static struct e820_entry *e820_table; > static unsigned e820_entries; > struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX}; > > -/* Physical Address of PVH entry point read from kernel ELF NOTE */ > -static size_t pvh_start_addr; > - > GlobalProperty pc_compat_4_1[] = {}; > const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1); > > @@ -1076,109 +1074,6 @@ struct setup_data { > uint8_t data[0]; > } __attribute__((packed)); > > - > -/* > - * The entry point into the kernel for PVH boot is different from > - * the native entry point. The PVH entry is defined by the x86/HVM > - * direct boot ABI and is available in an ELFNOTE in the kernel binary. > - * > - * This function is passed to load_elf() when it is called from > - * load_elfboot() which then additionally checks for an ELF Note of > - * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to > - * parse the PVH entry address from the ELF Note. > - * > - * Due to trickery in elf_opts.h, load_elf() is actually available as > - * load_elf32() or load_elf64() and this routine needs to be able > - * to deal with being called as 32 or 64 bit. > - * > - * The address of the PVH entry point is saved to the 'pvh_start_addr' > - * global variable. (although the entry point is 32-bit, the kernel > - * binary can be either 32-bit or 64-bit). > - */ > -static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) > -{ > - size_t *elf_note_data_addr; > - > - /* Check if ELF Note header passed in is valid */ > - if (arg1 == NULL) { > - return 0; > - } > - > - if (is64) { > - struct elf64_note *nhdr64 = (struct elf64_note *)arg1; > - uint64_t nhdr_size64 = sizeof(struct elf64_note); > - uint64_t phdr_align = *(uint64_t *)arg2; > - uint64_t nhdr_namesz = nhdr64->n_namesz; > - > - elf_note_data_addr = > - ((void *)nhdr64) + nhdr_size64 + > - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > - } else { > - struct elf32_note *nhdr32 = (struct elf32_note *)arg1; > - uint32_t nhdr_size32 = sizeof(struct elf32_note); > - uint32_t phdr_align = *(uint32_t *)arg2; > - uint32_t nhdr_namesz = nhdr32->n_namesz; > - > - elf_note_data_addr = > - ((void *)nhdr32) + nhdr_size32 + > - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > - } > - > - pvh_start_addr = *elf_note_data_addr; > - > - return pvh_start_addr; > -} > - > -static bool load_elfboot(const char *kernel_filename, > - int kernel_file_size, > - uint8_t *header, > - size_t pvh_xen_start_addr, > - FWCfgState *fw_cfg) > -{ > - uint32_t flags = 0; > - uint32_t mh_load_addr = 0; > - uint32_t elf_kernel_size = 0; > - uint64_t elf_entry; > - uint64_t elf_low, elf_high; > - int kernel_size; > - > - if (ldl_p(header) != 0x464c457f) { > - return false; /* no elfboot */ > - } > - > - bool elf_is64 = header[EI_CLASS] == ELFCLASS64; > - flags = elf_is64 ? > - ((Elf64_Ehdr *)header)->e_flags : ((Elf32_Ehdr *)header)->e_flags; > - > - if (flags & 0x00010004) { /* LOAD_ELF_HEADER_HAS_ADDR */ > - error_report("elfboot unsupported flags = %x", flags); > - exit(1); > - } > - > - uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; > - kernel_size = load_elf(kernel_filename, read_pvh_start_addr, > - NULL, &elf_note_type, &elf_entry, > - &elf_low, &elf_high, 0, I386_ELF_MACHINE, > - 0, 0); > - > - if (kernel_size < 0) { > - error_report("Error while loading elf kernel"); > - exit(1); > - } > - mh_load_addr = elf_low; > - elf_kernel_size = elf_high - elf_low; > - > - if (pvh_start_addr == 0) { > - error_report("Error loading uncompressed kernel without PVH ELF Note"); > - exit(1); > - } > - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); > - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); > - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); > - > - return true; > -} > - > static void load_linux(PCMachineState *pcms, > FWCfgState *fw_cfg) > { > @@ -1218,6 +1113,9 @@ static void load_linux(PCMachineState *pcms, > if (ldl_p(header+0x202) == 0x53726448) { > protocol = lduw_p(header+0x206); > } else { > + size_t pvh_start_addr; > + uint32_t mh_load_addr = 0; > + uint32_t elf_kernel_size = 0; > /* > * This could be a multiboot kernel. If it is, let's stop treating it > * like a Linux kernel. > @@ -1235,10 +1133,16 @@ static void load_linux(PCMachineState *pcms, > * If load_elfboot() is successful, populate the fw_cfg info. > */ > if (pcmc->pvh_enabled && > - load_elfboot(kernel_filename, kernel_size, > - header, pvh_start_addr, fw_cfg)) { > + pvh_load_elfboot(kernel_filename, > + &mh_load_addr, &elf_kernel_size)) { > fclose(f); > > + pvh_start_addr = pvh_get_start_addr(); > + > + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); > + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); > + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); > + > fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, > strlen(kernel_cmdline) + 1); > fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline); > diff --git a/hw/i386/pvh.c b/hw/i386/pvh.c > new file mode 100644 > index 0000000000..1c81727811 > --- /dev/null > +++ b/hw/i386/pvh.c > @@ -0,0 +1,113 @@ > +/* > + * PVH Boot Helper > + * > + * Copyright (C) 2019 Oracle > + * Copyright (C) 2019 Red Hat, Inc > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/units.h" > +#include "qemu/error-report.h" > +#include "hw/loader.h" > +#include "cpu.h" > +#include "elf.h" > +#include "pvh.h" > + > +static size_t pvh_start_addr; > + > +size_t pvh_get_start_addr(void) > +{ > + return pvh_start_addr; > +} > + > +/* > + * The entry point into the kernel for PVH boot is different from > + * the native entry point. The PVH entry is defined by the x86/HVM > + * direct boot ABI and is available in an ELFNOTE in the kernel binary. > + * > + * This function is passed to load_elf() when it is called from > + * load_elfboot() which then additionally checks for an ELF Note of > + * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to > + * parse the PVH entry address from the ELF Note. > + * > + * Due to trickery in elf_opts.h, load_elf() is actually available as > + * load_elf32() or load_elf64() and this routine needs to be able > + * to deal with being called as 32 or 64 bit. > + * > + * The address of the PVH entry point is saved to the 'pvh_start_addr' > + * global variable. (although the entry point is 32-bit, the kernel > + * binary can be either 32-bit or 64-bit). > + */ > + > +static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) > +{ > + size_t *elf_note_data_addr; > + > + /* Check if ELF Note header passed in is valid */ > + if (arg1 == NULL) { > + return 0; > + } > + > + if (is64) { > + struct elf64_note *nhdr64 = (struct elf64_note *)arg1; > + uint64_t nhdr_size64 = sizeof(struct elf64_note); > + uint64_t phdr_align = *(uint64_t *)arg2; > + uint64_t nhdr_namesz = nhdr64->n_namesz; > + > + elf_note_data_addr = > + ((void *)nhdr64) + nhdr_size64 + > + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > + } else { > + struct elf32_note *nhdr32 = (struct elf32_note *)arg1; > + uint32_t nhdr_size32 = sizeof(struct elf32_note); > + uint32_t phdr_align = *(uint32_t *)arg2; > + uint32_t nhdr_namesz = nhdr32->n_namesz; > + > + elf_note_data_addr = > + ((void *)nhdr32) + nhdr_size32 + > + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > + } > + > + pvh_start_addr = *elf_note_data_addr; > + > + return pvh_start_addr; > +} > + > +bool pvh_load_elfboot(const char *kernel_filename, > + uint32_t *mh_load_addr, > + uint32_t *elf_kernel_size) > +{ > + uint64_t elf_entry; > + uint64_t elf_low, elf_high; > + int kernel_size; > + uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; > + > + kernel_size = load_elf(kernel_filename, read_pvh_start_addr, > + NULL, &elf_note_type, &elf_entry, > + &elf_low, &elf_high, 0, I386_ELF_MACHINE, > + 0, 0); > + > + if (kernel_size < 0) { > + error_report("Error while loading elf kernel"); > + return false; > + } > + > + if (pvh_start_addr == 0) { > + error_report("Error loading uncompressed kernel without PVH ELF Note"); > + return false; > + } > + > + if (mh_load_addr) { > + *mh_load_addr = elf_low; > + } > + > + if (elf_kernel_size) { > + *elf_kernel_size = elf_high - elf_low; > + } > + > + return true; > +} > diff --git a/hw/i386/pvh.h b/hw/i386/pvh.h > new file mode 100644 > index 0000000000..ada67ff6e8 > --- /dev/null > +++ b/hw/i386/pvh.h > @@ -0,0 +1,10 @@ License missing. > +#ifndef HW_I386_PVH_H > +#define HW_I386_PVH_H > + > +size_t pvh_get_start_addr(void); > + > +bool pvh_load_elfboot(const char *kernel_filename, > + uint32_t *mh_load_addr, > + uint32_t *elf_kernel_size); Can you document these functions? Thanks, Phil. > + > +#endif >
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > Hi Sergio, > > On 9/24/19 2:44 PM, Sergio Lopez wrote: >> Extract PVH related functions from pc.c, and put them in pvh.c, so >> they can be shared with other components. >> >> Signed-off-by: Sergio Lopez <slp@redhat.com> >> --- >> hw/i386/Makefile.objs | 1 + >> hw/i386/pc.c | 120 +++++------------------------------------- >> hw/i386/pvh.c | 113 +++++++++++++++++++++++++++++++++++++++ >> hw/i386/pvh.h | 10 ++++ >> 4 files changed, 136 insertions(+), 108 deletions(-) >> create mode 100644 hw/i386/pvh.c >> create mode 100644 hw/i386/pvh.h >> >> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs >> index 5d9c9efd5f..c5f20bbd72 100644 >> --- a/hw/i386/Makefile.objs >> +++ b/hw/i386/Makefile.objs >> @@ -1,5 +1,6 @@ >> obj-$(CONFIG_KVM) += kvm/ >> obj-y += multiboot.o >> +obj-y += pvh.o >> obj-y += pc.o >> obj-$(CONFIG_I440FX) += pc_piix.o >> obj-$(CONFIG_Q35) += pc_q35.o >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index bad866fe44..10e4ced0c6 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -42,6 +42,7 @@ >> #include "elf.h" >> #include "migration/vmstate.h" >> #include "multiboot.h" >> +#include "pvh.h" >> #include "hw/timer/mc146818rtc.h" >> #include "hw/dma/i8257.h" >> #include "hw/timer/i8254.h" >> @@ -116,9 +117,6 @@ static struct e820_entry *e820_table; >> static unsigned e820_entries; >> struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX}; >> >> -/* Physical Address of PVH entry point read from kernel ELF NOTE */ >> -static size_t pvh_start_addr; >> - >> GlobalProperty pc_compat_4_1[] = {}; >> const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1); >> >> @@ -1076,109 +1074,6 @@ struct setup_data { >> uint8_t data[0]; >> } __attribute__((packed)); >> >> - >> -/* >> - * The entry point into the kernel for PVH boot is different from >> - * the native entry point. The PVH entry is defined by the x86/HVM >> - * direct boot ABI and is available in an ELFNOTE in the kernel binary. >> - * >> - * This function is passed to load_elf() when it is called from >> - * load_elfboot() which then additionally checks for an ELF Note of >> - * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to >> - * parse the PVH entry address from the ELF Note. >> - * >> - * Due to trickery in elf_opts.h, load_elf() is actually available as >> - * load_elf32() or load_elf64() and this routine needs to be able >> - * to deal with being called as 32 or 64 bit. >> - * >> - * The address of the PVH entry point is saved to the 'pvh_start_addr' >> - * global variable. (although the entry point is 32-bit, the kernel >> - * binary can be either 32-bit or 64-bit). >> - */ >> -static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) >> -{ >> - size_t *elf_note_data_addr; >> - >> - /* Check if ELF Note header passed in is valid */ >> - if (arg1 == NULL) { >> - return 0; >> - } >> - >> - if (is64) { >> - struct elf64_note *nhdr64 = (struct elf64_note *)arg1; >> - uint64_t nhdr_size64 = sizeof(struct elf64_note); >> - uint64_t phdr_align = *(uint64_t *)arg2; >> - uint64_t nhdr_namesz = nhdr64->n_namesz; >> - >> - elf_note_data_addr = >> - ((void *)nhdr64) + nhdr_size64 + >> - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); >> - } else { >> - struct elf32_note *nhdr32 = (struct elf32_note *)arg1; >> - uint32_t nhdr_size32 = sizeof(struct elf32_note); >> - uint32_t phdr_align = *(uint32_t *)arg2; >> - uint32_t nhdr_namesz = nhdr32->n_namesz; >> - >> - elf_note_data_addr = >> - ((void *)nhdr32) + nhdr_size32 + >> - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); >> - } >> - >> - pvh_start_addr = *elf_note_data_addr; >> - >> - return pvh_start_addr; >> -} >> - >> -static bool load_elfboot(const char *kernel_filename, >> - int kernel_file_size, >> - uint8_t *header, >> - size_t pvh_xen_start_addr, >> - FWCfgState *fw_cfg) >> -{ >> - uint32_t flags = 0; >> - uint32_t mh_load_addr = 0; >> - uint32_t elf_kernel_size = 0; >> - uint64_t elf_entry; >> - uint64_t elf_low, elf_high; >> - int kernel_size; >> - >> - if (ldl_p(header) != 0x464c457f) { >> - return false; /* no elfboot */ >> - } >> - >> - bool elf_is64 = header[EI_CLASS] == ELFCLASS64; >> - flags = elf_is64 ? >> - ((Elf64_Ehdr *)header)->e_flags : ((Elf32_Ehdr *)header)->e_flags; >> - >> - if (flags & 0x00010004) { /* LOAD_ELF_HEADER_HAS_ADDR */ >> - error_report("elfboot unsupported flags = %x", flags); >> - exit(1); >> - } >> - >> - uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; >> - kernel_size = load_elf(kernel_filename, read_pvh_start_addr, >> - NULL, &elf_note_type, &elf_entry, >> - &elf_low, &elf_high, 0, I386_ELF_MACHINE, >> - 0, 0); >> - >> - if (kernel_size < 0) { >> - error_report("Error while loading elf kernel"); >> - exit(1); >> - } >> - mh_load_addr = elf_low; >> - elf_kernel_size = elf_high - elf_low; >> - >> - if (pvh_start_addr == 0) { >> - error_report("Error loading uncompressed kernel without PVH ELF Note"); >> - exit(1); >> - } >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); >> - >> - return true; >> -} >> - >> static void load_linux(PCMachineState *pcms, >> FWCfgState *fw_cfg) >> { >> @@ -1218,6 +1113,9 @@ static void load_linux(PCMachineState *pcms, >> if (ldl_p(header+0x202) == 0x53726448) { >> protocol = lduw_p(header+0x206); >> } else { >> + size_t pvh_start_addr; >> + uint32_t mh_load_addr = 0; >> + uint32_t elf_kernel_size = 0; >> /* >> * This could be a multiboot kernel. If it is, let's stop treating it >> * like a Linux kernel. >> @@ -1235,10 +1133,16 @@ static void load_linux(PCMachineState *pcms, >> * If load_elfboot() is successful, populate the fw_cfg info. >> */ >> if (pcmc->pvh_enabled && >> - load_elfboot(kernel_filename, kernel_size, >> - header, pvh_start_addr, fw_cfg)) { >> + pvh_load_elfboot(kernel_filename, >> + &mh_load_addr, &elf_kernel_size)) { >> fclose(f); >> >> + pvh_start_addr = pvh_get_start_addr(); >> + >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); >> + >> fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, >> strlen(kernel_cmdline) + 1); >> fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline); >> diff --git a/hw/i386/pvh.c b/hw/i386/pvh.c >> new file mode 100644 >> index 0000000000..1c81727811 >> --- /dev/null >> +++ b/hw/i386/pvh.c >> @@ -0,0 +1,113 @@ >> +/* >> + * PVH Boot Helper >> + * >> + * Copyright (C) 2019 Oracle >> + * Copyright (C) 2019 Red Hat, Inc >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + * >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "qemu/units.h" >> +#include "qemu/error-report.h" >> +#include "hw/loader.h" >> +#include "cpu.h" >> +#include "elf.h" >> +#include "pvh.h" >> + >> +static size_t pvh_start_addr; >> + >> +size_t pvh_get_start_addr(void) >> +{ >> + return pvh_start_addr; >> +} >> + >> +/* >> + * The entry point into the kernel for PVH boot is different from >> + * the native entry point. The PVH entry is defined by the x86/HVM >> + * direct boot ABI and is available in an ELFNOTE in the kernel binary. >> + * >> + * This function is passed to load_elf() when it is called from >> + * load_elfboot() which then additionally checks for an ELF Note of >> + * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to >> + * parse the PVH entry address from the ELF Note. >> + * >> + * Due to trickery in elf_opts.h, load_elf() is actually available as >> + * load_elf32() or load_elf64() and this routine needs to be able >> + * to deal with being called as 32 or 64 bit. >> + * >> + * The address of the PVH entry point is saved to the 'pvh_start_addr' >> + * global variable. (although the entry point is 32-bit, the kernel >> + * binary can be either 32-bit or 64-bit). >> + */ >> + >> +static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) >> +{ >> + size_t *elf_note_data_addr; >> + >> + /* Check if ELF Note header passed in is valid */ >> + if (arg1 == NULL) { >> + return 0; >> + } >> + >> + if (is64) { >> + struct elf64_note *nhdr64 = (struct elf64_note *)arg1; >> + uint64_t nhdr_size64 = sizeof(struct elf64_note); >> + uint64_t phdr_align = *(uint64_t *)arg2; >> + uint64_t nhdr_namesz = nhdr64->n_namesz; >> + >> + elf_note_data_addr = >> + ((void *)nhdr64) + nhdr_size64 + >> + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); >> + } else { >> + struct elf32_note *nhdr32 = (struct elf32_note *)arg1; >> + uint32_t nhdr_size32 = sizeof(struct elf32_note); >> + uint32_t phdr_align = *(uint32_t *)arg2; >> + uint32_t nhdr_namesz = nhdr32->n_namesz; >> + >> + elf_note_data_addr = >> + ((void *)nhdr32) + nhdr_size32 + >> + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); >> + } >> + >> + pvh_start_addr = *elf_note_data_addr; >> + >> + return pvh_start_addr; >> +} >> + >> +bool pvh_load_elfboot(const char *kernel_filename, >> + uint32_t *mh_load_addr, >> + uint32_t *elf_kernel_size) >> +{ >> + uint64_t elf_entry; >> + uint64_t elf_low, elf_high; >> + int kernel_size; >> + uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; >> + >> + kernel_size = load_elf(kernel_filename, read_pvh_start_addr, >> + NULL, &elf_note_type, &elf_entry, >> + &elf_low, &elf_high, 0, I386_ELF_MACHINE, >> + 0, 0); >> + >> + if (kernel_size < 0) { >> + error_report("Error while loading elf kernel"); >> + return false; >> + } >> + >> + if (pvh_start_addr == 0) { >> + error_report("Error loading uncompressed kernel without PVH ELF Note"); >> + return false; >> + } >> + >> + if (mh_load_addr) { >> + *mh_load_addr = elf_low; >> + } >> + >> + if (elf_kernel_size) { >> + *elf_kernel_size = elf_high - elf_low; >> + } >> + >> + return true; >> +} >> diff --git a/hw/i386/pvh.h b/hw/i386/pvh.h >> new file mode 100644 >> index 0000000000..ada67ff6e8 >> --- /dev/null >> +++ b/hw/i386/pvh.h >> @@ -0,0 +1,10 @@ > > License missing. I'm a bit confused about the policy for license blocks in headers, as some do have it, while others don't (i.e. multiboot.h and acpi-build.h). >> +#ifndef HW_I386_PVH_H >> +#define HW_I386_PVH_H >> + >> +size_t pvh_get_start_addr(void); >> + >> +bool pvh_load_elfboot(const char *kernel_filename, >> + uint32_t *mh_load_addr, >> + uint32_t *elf_kernel_size); > > Can you document these functions? Sure. Thanks, Sergio. >> + >> +#endif >>
Hi Sergio, On Tue, Sep 24, 2019 at 02:44:26PM +0200, Sergio Lopez wrote: > Extract PVH related functions from pc.c, and put them in pvh.c, so > they can be shared with other components. > > Signed-off-by: Sergio Lopez <slp@redhat.com> > --- > hw/i386/Makefile.objs | 1 + > hw/i386/pc.c | 120 +++++------------------------------------- > hw/i386/pvh.c | 113 +++++++++++++++++++++++++++++++++++++++ > hw/i386/pvh.h | 10 ++++ > 4 files changed, 136 insertions(+), 108 deletions(-) > create mode 100644 hw/i386/pvh.c > create mode 100644 hw/i386/pvh.h > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > index 5d9c9efd5f..c5f20bbd72 100644 > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -1,5 +1,6 @@ > obj-$(CONFIG_KVM) += kvm/ > obj-y += multiboot.o > +obj-y += pvh.o > obj-y += pc.o > obj-$(CONFIG_I440FX) += pc_piix.o > obj-$(CONFIG_Q35) += pc_q35.o > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index bad866fe44..10e4ced0c6 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -42,6 +42,7 @@ > #include "elf.h" > #include "migration/vmstate.h" > #include "multiboot.h" > +#include "pvh.h" > #include "hw/timer/mc146818rtc.h" > #include "hw/dma/i8257.h" > #include "hw/timer/i8254.h" > @@ -116,9 +117,6 @@ static struct e820_entry *e820_table; > static unsigned e820_entries; > struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX}; > > -/* Physical Address of PVH entry point read from kernel ELF NOTE */ > -static size_t pvh_start_addr; > - > GlobalProperty pc_compat_4_1[] = {}; > const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1); > > @@ -1076,109 +1074,6 @@ struct setup_data { > uint8_t data[0]; > } __attribute__((packed)); > > - > -/* > - * The entry point into the kernel for PVH boot is different from > - * the native entry point. The PVH entry is defined by the x86/HVM > - * direct boot ABI and is available in an ELFNOTE in the kernel binary. > - * > - * This function is passed to load_elf() when it is called from > - * load_elfboot() which then additionally checks for an ELF Note of > - * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to > - * parse the PVH entry address from the ELF Note. > - * > - * Due to trickery in elf_opts.h, load_elf() is actually available as > - * load_elf32() or load_elf64() and this routine needs to be able > - * to deal with being called as 32 or 64 bit. > - * > - * The address of the PVH entry point is saved to the 'pvh_start_addr' > - * global variable. (although the entry point is 32-bit, the kernel > - * binary can be either 32-bit or 64-bit). > - */ > -static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) > -{ > - size_t *elf_note_data_addr; > - > - /* Check if ELF Note header passed in is valid */ > - if (arg1 == NULL) { > - return 0; > - } > - > - if (is64) { > - struct elf64_note *nhdr64 = (struct elf64_note *)arg1; > - uint64_t nhdr_size64 = sizeof(struct elf64_note); > - uint64_t phdr_align = *(uint64_t *)arg2; > - uint64_t nhdr_namesz = nhdr64->n_namesz; > - > - elf_note_data_addr = > - ((void *)nhdr64) + nhdr_size64 + > - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > - } else { > - struct elf32_note *nhdr32 = (struct elf32_note *)arg1; > - uint32_t nhdr_size32 = sizeof(struct elf32_note); > - uint32_t phdr_align = *(uint32_t *)arg2; > - uint32_t nhdr_namesz = nhdr32->n_namesz; > - > - elf_note_data_addr = > - ((void *)nhdr32) + nhdr_size32 + > - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > - } > - > - pvh_start_addr = *elf_note_data_addr; > - > - return pvh_start_addr; > -} > - > -static bool load_elfboot(const char *kernel_filename, > - int kernel_file_size, > - uint8_t *header, > - size_t pvh_xen_start_addr, > - FWCfgState *fw_cfg) > -{ > - uint32_t flags = 0; > - uint32_t mh_load_addr = 0; > - uint32_t elf_kernel_size = 0; > - uint64_t elf_entry; > - uint64_t elf_low, elf_high; > - int kernel_size; > - Are we removing the following checks (ELF magic, flags) because they are superfluous? Should we mention this in the commit message? > - if (ldl_p(header) != 0x464c457f) { > - return false; /* no elfboot */ > - } > - > - bool elf_is64 = header[EI_CLASS] == ELFCLASS64; > - flags = elf_is64 ? > - ((Elf64_Ehdr *)header)->e_flags : ((Elf32_Ehdr *)header)->e_flags; > - > - if (flags & 0x00010004) { /* LOAD_ELF_HEADER_HAS_ADDR */ > - error_report("elfboot unsupported flags = %x", flags); > - exit(1); > - } > - > - uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; > - kernel_size = load_elf(kernel_filename, read_pvh_start_addr, > - NULL, &elf_note_type, &elf_entry, > - &elf_low, &elf_high, 0, I386_ELF_MACHINE, > - 0, 0); > - > - if (kernel_size < 0) { > - error_report("Error while loading elf kernel"); > - exit(1); > - } > - mh_load_addr = elf_low; > - elf_kernel_size = elf_high - elf_low; > - > - if (pvh_start_addr == 0) { > - error_report("Error loading uncompressed kernel without PVH ELF Note"); > - exit(1); > - } > - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); > - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); > - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); > - > - return true; > -} > - > static void load_linux(PCMachineState *pcms, > FWCfgState *fw_cfg) > { > @@ -1218,6 +1113,9 @@ static void load_linux(PCMachineState *pcms, > if (ldl_p(header+0x202) == 0x53726448) { > protocol = lduw_p(header+0x206); > } else { > + size_t pvh_start_addr; > + uint32_t mh_load_addr = 0; > + uint32_t elf_kernel_size = 0; > /* > * This could be a multiboot kernel. If it is, let's stop treating it > * like a Linux kernel. > @@ -1235,10 +1133,16 @@ static void load_linux(PCMachineState *pcms, > * If load_elfboot() is successful, populate the fw_cfg info. > */ > if (pcmc->pvh_enabled && > - load_elfboot(kernel_filename, kernel_size, > - header, pvh_start_addr, fw_cfg)) { > + pvh_load_elfboot(kernel_filename, > + &mh_load_addr, &elf_kernel_size)) { > fclose(f); > > + pvh_start_addr = pvh_get_start_addr(); > + > + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); > + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); > + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); > + > fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, > strlen(kernel_cmdline) + 1); > fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline); > diff --git a/hw/i386/pvh.c b/hw/i386/pvh.c > new file mode 100644 > index 0000000000..1c81727811 > --- /dev/null > +++ b/hw/i386/pvh.c > @@ -0,0 +1,113 @@ > +/* > + * PVH Boot Helper > + * > + * Copyright (C) 2019 Oracle > + * Copyright (C) 2019 Red Hat, Inc > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/units.h" > +#include "qemu/error-report.h" > +#include "hw/loader.h" > +#include "cpu.h" > +#include "elf.h" > +#include "pvh.h" > + > +static size_t pvh_start_addr; > + > +size_t pvh_get_start_addr(void) > +{ > + return pvh_start_addr; > +} > + > +/* > + * The entry point into the kernel for PVH boot is different from > + * the native entry point. The PVH entry is defined by the x86/HVM > + * direct boot ABI and is available in an ELFNOTE in the kernel binary. > + * > + * This function is passed to load_elf() when it is called from > + * load_elfboot() which then additionally checks for an ELF Note of > + * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to > + * parse the PVH entry address from the ELF Note. > + * > + * Due to trickery in elf_opts.h, load_elf() is actually available as > + * load_elf32() or load_elf64() and this routine needs to be able > + * to deal with being called as 32 or 64 bit. > + * > + * The address of the PVH entry point is saved to the 'pvh_start_addr' > + * global variable. (although the entry point is 32-bit, the kernel > + * binary can be either 32-bit or 64-bit). > + */ > + > +static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) > +{ > + size_t *elf_note_data_addr; > + > + /* Check if ELF Note header passed in is valid */ > + if (arg1 == NULL) { > + return 0; > + } > + > + if (is64) { > + struct elf64_note *nhdr64 = (struct elf64_note *)arg1; > + uint64_t nhdr_size64 = sizeof(struct elf64_note); > + uint64_t phdr_align = *(uint64_t *)arg2; > + uint64_t nhdr_namesz = nhdr64->n_namesz; > + > + elf_note_data_addr = > + ((void *)nhdr64) + nhdr_size64 + > + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > + } else { > + struct elf32_note *nhdr32 = (struct elf32_note *)arg1; > + uint32_t nhdr_size32 = sizeof(struct elf32_note); > + uint32_t phdr_align = *(uint32_t *)arg2; > + uint32_t nhdr_namesz = nhdr32->n_namesz; > + > + elf_note_data_addr = > + ((void *)nhdr32) + nhdr_size32 + > + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > + } > + > + pvh_start_addr = *elf_note_data_addr; > + > + return pvh_start_addr; > +} > + > +bool pvh_load_elfboot(const char *kernel_filename, > + uint32_t *mh_load_addr, > + uint32_t *elf_kernel_size) > +{ > + uint64_t elf_entry; > + uint64_t elf_low, elf_high; > + int kernel_size; > + uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; > + > + kernel_size = load_elf(kernel_filename, read_pvh_start_addr, > + NULL, &elf_note_type, &elf_entry, > + &elf_low, &elf_high, 0, I386_ELF_MACHINE, > + 0, 0); > + > + if (kernel_size < 0) { > + error_report("Error while loading elf kernel"); > + return false; > + } > + > + if (pvh_start_addr == 0) { > + error_report("Error loading uncompressed kernel without PVH ELF Note"); > + return false; > + } > + > + if (mh_load_addr) { > + *mh_load_addr = elf_low; > + } > + > + if (elf_kernel_size) { > + *elf_kernel_size = elf_high - elf_low; > + } > + > + return true; > +} > diff --git a/hw/i386/pvh.h b/hw/i386/pvh.h > new file mode 100644 > index 0000000000..ada67ff6e8 > --- /dev/null > +++ b/hw/i386/pvh.h > @@ -0,0 +1,10 @@ > +#ifndef HW_I386_PVH_H > +#define HW_I386_PVH_H > + > +size_t pvh_get_start_addr(void); What about adding "size_t *pvh_start_addr" to the pvh_load_elfboot()? Just an idea, I'm not sure if it is better... > + > +bool pvh_load_elfboot(const char *kernel_filename, > + uint32_t *mh_load_addr, > + uint32_t *elf_kernel_size); > + > +#endif Thanks, Stefano
Stefano Garzarella <sgarzare@redhat.com> writes: > Hi Sergio, > > On Tue, Sep 24, 2019 at 02:44:26PM +0200, Sergio Lopez wrote: >> Extract PVH related functions from pc.c, and put them in pvh.c, so >> they can be shared with other components. >> >> Signed-off-by: Sergio Lopez <slp@redhat.com> >> --- >> hw/i386/Makefile.objs | 1 + >> hw/i386/pc.c | 120 +++++------------------------------------- >> hw/i386/pvh.c | 113 +++++++++++++++++++++++++++++++++++++++ >> hw/i386/pvh.h | 10 ++++ >> 4 files changed, 136 insertions(+), 108 deletions(-) >> create mode 100644 hw/i386/pvh.c >> create mode 100644 hw/i386/pvh.h >> >> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs >> index 5d9c9efd5f..c5f20bbd72 100644 >> --- a/hw/i386/Makefile.objs >> +++ b/hw/i386/Makefile.objs >> @@ -1,5 +1,6 @@ >> obj-$(CONFIG_KVM) += kvm/ >> obj-y += multiboot.o >> +obj-y += pvh.o >> obj-y += pc.o >> obj-$(CONFIG_I440FX) += pc_piix.o >> obj-$(CONFIG_Q35) += pc_q35.o >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index bad866fe44..10e4ced0c6 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -42,6 +42,7 @@ >> #include "elf.h" >> #include "migration/vmstate.h" >> #include "multiboot.h" >> +#include "pvh.h" >> #include "hw/timer/mc146818rtc.h" >> #include "hw/dma/i8257.h" >> #include "hw/timer/i8254.h" >> @@ -116,9 +117,6 @@ static struct e820_entry *e820_table; >> static unsigned e820_entries; >> struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX}; >> >> -/* Physical Address of PVH entry point read from kernel ELF NOTE */ >> -static size_t pvh_start_addr; >> - >> GlobalProperty pc_compat_4_1[] = {}; >> const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1); >> >> @@ -1076,109 +1074,6 @@ struct setup_data { >> uint8_t data[0]; >> } __attribute__((packed)); >> >> - >> -/* >> - * The entry point into the kernel for PVH boot is different from >> - * the native entry point. The PVH entry is defined by the x86/HVM >> - * direct boot ABI and is available in an ELFNOTE in the kernel binary. >> - * >> - * This function is passed to load_elf() when it is called from >> - * load_elfboot() which then additionally checks for an ELF Note of >> - * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to >> - * parse the PVH entry address from the ELF Note. >> - * >> - * Due to trickery in elf_opts.h, load_elf() is actually available as >> - * load_elf32() or load_elf64() and this routine needs to be able >> - * to deal with being called as 32 or 64 bit. >> - * >> - * The address of the PVH entry point is saved to the 'pvh_start_addr' >> - * global variable. (although the entry point is 32-bit, the kernel >> - * binary can be either 32-bit or 64-bit). >> - */ >> -static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) >> -{ >> - size_t *elf_note_data_addr; >> - >> - /* Check if ELF Note header passed in is valid */ >> - if (arg1 == NULL) { >> - return 0; >> - } >> - >> - if (is64) { >> - struct elf64_note *nhdr64 = (struct elf64_note *)arg1; >> - uint64_t nhdr_size64 = sizeof(struct elf64_note); >> - uint64_t phdr_align = *(uint64_t *)arg2; >> - uint64_t nhdr_namesz = nhdr64->n_namesz; >> - >> - elf_note_data_addr = >> - ((void *)nhdr64) + nhdr_size64 + >> - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); >> - } else { >> - struct elf32_note *nhdr32 = (struct elf32_note *)arg1; >> - uint32_t nhdr_size32 = sizeof(struct elf32_note); >> - uint32_t phdr_align = *(uint32_t *)arg2; >> - uint32_t nhdr_namesz = nhdr32->n_namesz; >> - >> - elf_note_data_addr = >> - ((void *)nhdr32) + nhdr_size32 + >> - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); >> - } >> - >> - pvh_start_addr = *elf_note_data_addr; >> - >> - return pvh_start_addr; >> -} >> - >> -static bool load_elfboot(const char *kernel_filename, >> - int kernel_file_size, >> - uint8_t *header, >> - size_t pvh_xen_start_addr, >> - FWCfgState *fw_cfg) >> -{ >> - uint32_t flags = 0; >> - uint32_t mh_load_addr = 0; >> - uint32_t elf_kernel_size = 0; >> - uint64_t elf_entry; >> - uint64_t elf_low, elf_high; >> - int kernel_size; >> - > > Are we removing the following checks (ELF magic, flags) because they > are superfluous? > > Should we mention this in the commit message? Damn, good catch, that's wrong. The only patches coming from previous iterations are the one factorizing the e820 functions and this one, and both are wrong. I'm going to ditch them and write whatever it's needed from scratch. >> - if (ldl_p(header) != 0x464c457f) { >> - return false; /* no elfboot */ >> - } >> - >> - bool elf_is64 = header[EI_CLASS] == ELFCLASS64; >> - flags = elf_is64 ? >> - ((Elf64_Ehdr *)header)->e_flags : ((Elf32_Ehdr *)header)->e_flags; >> - >> - if (flags & 0x00010004) { /* LOAD_ELF_HEADER_HAS_ADDR */ >> - error_report("elfboot unsupported flags = %x", flags); >> - exit(1); >> - } >> - >> - uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; >> - kernel_size = load_elf(kernel_filename, read_pvh_start_addr, >> - NULL, &elf_note_type, &elf_entry, >> - &elf_low, &elf_high, 0, I386_ELF_MACHINE, >> - 0, 0); >> - >> - if (kernel_size < 0) { >> - error_report("Error while loading elf kernel"); >> - exit(1); >> - } >> - mh_load_addr = elf_low; >> - elf_kernel_size = elf_high - elf_low; >> - >> - if (pvh_start_addr == 0) { >> - error_report("Error loading uncompressed kernel without PVH ELF Note"); >> - exit(1); >> - } >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); >> - >> - return true; >> -} >> - >> static void load_linux(PCMachineState *pcms, >> FWCfgState *fw_cfg) >> { >> @@ -1218,6 +1113,9 @@ static void load_linux(PCMachineState *pcms, >> if (ldl_p(header+0x202) == 0x53726448) { >> protocol = lduw_p(header+0x206); >> } else { >> + size_t pvh_start_addr; >> + uint32_t mh_load_addr = 0; >> + uint32_t elf_kernel_size = 0; >> /* >> * This could be a multiboot kernel. If it is, let's stop treating it >> * like a Linux kernel. >> @@ -1235,10 +1133,16 @@ static void load_linux(PCMachineState *pcms, >> * If load_elfboot() is successful, populate the fw_cfg info. >> */ >> if (pcmc->pvh_enabled && >> - load_elfboot(kernel_filename, kernel_size, >> - header, pvh_start_addr, fw_cfg)) { >> + pvh_load_elfboot(kernel_filename, >> + &mh_load_addr, &elf_kernel_size)) { >> fclose(f); >> >> + pvh_start_addr = pvh_get_start_addr(); >> + >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); >> + >> fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, >> strlen(kernel_cmdline) + 1); >> fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline); >> diff --git a/hw/i386/pvh.c b/hw/i386/pvh.c >> new file mode 100644 >> index 0000000000..1c81727811 >> --- /dev/null >> +++ b/hw/i386/pvh.c >> @@ -0,0 +1,113 @@ >> +/* >> + * PVH Boot Helper >> + * >> + * Copyright (C) 2019 Oracle >> + * Copyright (C) 2019 Red Hat, Inc >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + * >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "qemu/units.h" >> +#include "qemu/error-report.h" >> +#include "hw/loader.h" >> +#include "cpu.h" >> +#include "elf.h" >> +#include "pvh.h" >> + >> +static size_t pvh_start_addr; >> + >> +size_t pvh_get_start_addr(void) >> +{ >> + return pvh_start_addr; >> +} >> + >> +/* >> + * The entry point into the kernel for PVH boot is different from >> + * the native entry point. The PVH entry is defined by the x86/HVM >> + * direct boot ABI and is available in an ELFNOTE in the kernel binary. >> + * >> + * This function is passed to load_elf() when it is called from >> + * load_elfboot() which then additionally checks for an ELF Note of >> + * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to >> + * parse the PVH entry address from the ELF Note. >> + * >> + * Due to trickery in elf_opts.h, load_elf() is actually available as >> + * load_elf32() or load_elf64() and this routine needs to be able >> + * to deal with being called as 32 or 64 bit. >> + * >> + * The address of the PVH entry point is saved to the 'pvh_start_addr' >> + * global variable. (although the entry point is 32-bit, the kernel >> + * binary can be either 32-bit or 64-bit). >> + */ >> + >> +static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) >> +{ >> + size_t *elf_note_data_addr; >> + >> + /* Check if ELF Note header passed in is valid */ >> + if (arg1 == NULL) { >> + return 0; >> + } >> + >> + if (is64) { >> + struct elf64_note *nhdr64 = (struct elf64_note *)arg1; >> + uint64_t nhdr_size64 = sizeof(struct elf64_note); >> + uint64_t phdr_align = *(uint64_t *)arg2; >> + uint64_t nhdr_namesz = nhdr64->n_namesz; >> + >> + elf_note_data_addr = >> + ((void *)nhdr64) + nhdr_size64 + >> + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); >> + } else { >> + struct elf32_note *nhdr32 = (struct elf32_note *)arg1; >> + uint32_t nhdr_size32 = sizeof(struct elf32_note); >> + uint32_t phdr_align = *(uint32_t *)arg2; >> + uint32_t nhdr_namesz = nhdr32->n_namesz; >> + >> + elf_note_data_addr = >> + ((void *)nhdr32) + nhdr_size32 + >> + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); >> + } >> + >> + pvh_start_addr = *elf_note_data_addr; >> + >> + return pvh_start_addr; >> +} >> + >> +bool pvh_load_elfboot(const char *kernel_filename, >> + uint32_t *mh_load_addr, >> + uint32_t *elf_kernel_size) >> +{ >> + uint64_t elf_entry; >> + uint64_t elf_low, elf_high; >> + int kernel_size; >> + uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; >> + >> + kernel_size = load_elf(kernel_filename, read_pvh_start_addr, >> + NULL, &elf_note_type, &elf_entry, >> + &elf_low, &elf_high, 0, I386_ELF_MACHINE, >> + 0, 0); >> + >> + if (kernel_size < 0) { >> + error_report("Error while loading elf kernel"); >> + return false; >> + } >> + >> + if (pvh_start_addr == 0) { >> + error_report("Error loading uncompressed kernel without PVH ELF Note"); >> + return false; >> + } >> + >> + if (mh_load_addr) { >> + *mh_load_addr = elf_low; >> + } >> + >> + if (elf_kernel_size) { >> + *elf_kernel_size = elf_high - elf_low; >> + } >> + >> + return true; >> +} >> diff --git a/hw/i386/pvh.h b/hw/i386/pvh.h >> new file mode 100644 >> index 0000000000..ada67ff6e8 >> --- /dev/null >> +++ b/hw/i386/pvh.h >> @@ -0,0 +1,10 @@ >> +#ifndef HW_I386_PVH_H >> +#define HW_I386_PVH_H >> + >> +size_t pvh_get_start_addr(void); > > What about adding "size_t *pvh_start_addr" to the pvh_load_elfboot()? > Just an idea, I'm not sure if it is better... I agree. In fact, given that patch 4/8 extracts some common functions from pc.c into x86.c, and load_linux is among these functions, perhaps we can avoid creating an independent file and just put the PVH code there. What do you think? Thanks a lot, Sergio. >> + >> +bool pvh_load_elfboot(const char *kernel_filename, >> + uint32_t *mh_load_addr, >> + uint32_t *elf_kernel_size); >> + >> +#endif > > Thanks, > Stefano
On Wed, Sep 25, 2019 at 11:00:30AM +0200, Sergio Lopez wrote: > Stefano Garzarella <sgarzare@redhat.com> writes: > > Hi Sergio, > > > > On Tue, Sep 24, 2019 at 02:44:26PM +0200, Sergio Lopez wrote: > >> Extract PVH related functions from pc.c, and put them in pvh.c, so > >> they can be shared with other components. > >> > >> Signed-off-by: Sergio Lopez <slp@redhat.com> > >> --- > >> hw/i386/Makefile.objs | 1 + > >> hw/i386/pc.c | 120 +++++------------------------------------- > >> hw/i386/pvh.c | 113 +++++++++++++++++++++++++++++++++++++++ > >> hw/i386/pvh.h | 10 ++++ > >> 4 files changed, 136 insertions(+), 108 deletions(-) > >> create mode 100644 hw/i386/pvh.c > >> create mode 100644 hw/i386/pvh.h > >> > >> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > >> index 5d9c9efd5f..c5f20bbd72 100644 > >> --- a/hw/i386/Makefile.objs > >> +++ b/hw/i386/Makefile.objs > >> @@ -1,5 +1,6 @@ > >> obj-$(CONFIG_KVM) += kvm/ > >> obj-y += multiboot.o > >> +obj-y += pvh.o > >> obj-y += pc.o > >> obj-$(CONFIG_I440FX) += pc_piix.o > >> obj-$(CONFIG_Q35) += pc_q35.o > >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c > >> index bad866fe44..10e4ced0c6 100644 > >> --- a/hw/i386/pc.c > >> +++ b/hw/i386/pc.c > >> @@ -42,6 +42,7 @@ > >> #include "elf.h" > >> #include "migration/vmstate.h" > >> #include "multiboot.h" > >> +#include "pvh.h" > >> #include "hw/timer/mc146818rtc.h" > >> #include "hw/dma/i8257.h" > >> #include "hw/timer/i8254.h" > >> @@ -116,9 +117,6 @@ static struct e820_entry *e820_table; > >> static unsigned e820_entries; > >> struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX}; > >> > >> -/* Physical Address of PVH entry point read from kernel ELF NOTE */ > >> -static size_t pvh_start_addr; > >> - > >> GlobalProperty pc_compat_4_1[] = {}; > >> const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1); > >> > >> @@ -1076,109 +1074,6 @@ struct setup_data { > >> uint8_t data[0]; > >> } __attribute__((packed)); > >> > >> - > >> -/* > >> - * The entry point into the kernel for PVH boot is different from > >> - * the native entry point. The PVH entry is defined by the x86/HVM > >> - * direct boot ABI and is available in an ELFNOTE in the kernel binary. > >> - * > >> - * This function is passed to load_elf() when it is called from > >> - * load_elfboot() which then additionally checks for an ELF Note of > >> - * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to > >> - * parse the PVH entry address from the ELF Note. > >> - * > >> - * Due to trickery in elf_opts.h, load_elf() is actually available as > >> - * load_elf32() or load_elf64() and this routine needs to be able > >> - * to deal with being called as 32 or 64 bit. > >> - * > >> - * The address of the PVH entry point is saved to the 'pvh_start_addr' > >> - * global variable. (although the entry point is 32-bit, the kernel > >> - * binary can be either 32-bit or 64-bit). > >> - */ > >> -static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) > >> -{ > >> - size_t *elf_note_data_addr; > >> - > >> - /* Check if ELF Note header passed in is valid */ > >> - if (arg1 == NULL) { > >> - return 0; > >> - } > >> - > >> - if (is64) { > >> - struct elf64_note *nhdr64 = (struct elf64_note *)arg1; > >> - uint64_t nhdr_size64 = sizeof(struct elf64_note); > >> - uint64_t phdr_align = *(uint64_t *)arg2; > >> - uint64_t nhdr_namesz = nhdr64->n_namesz; > >> - > >> - elf_note_data_addr = > >> - ((void *)nhdr64) + nhdr_size64 + > >> - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > >> - } else { > >> - struct elf32_note *nhdr32 = (struct elf32_note *)arg1; > >> - uint32_t nhdr_size32 = sizeof(struct elf32_note); > >> - uint32_t phdr_align = *(uint32_t *)arg2; > >> - uint32_t nhdr_namesz = nhdr32->n_namesz; > >> - > >> - elf_note_data_addr = > >> - ((void *)nhdr32) + nhdr_size32 + > >> - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > >> - } > >> - > >> - pvh_start_addr = *elf_note_data_addr; > >> - > >> - return pvh_start_addr; > >> -} > >> - > >> -static bool load_elfboot(const char *kernel_filename, > >> - int kernel_file_size, > >> - uint8_t *header, > >> - size_t pvh_xen_start_addr, > >> - FWCfgState *fw_cfg) > >> -{ > >> - uint32_t flags = 0; > >> - uint32_t mh_load_addr = 0; > >> - uint32_t elf_kernel_size = 0; > >> - uint64_t elf_entry; > >> - uint64_t elf_low, elf_high; > >> - int kernel_size; > >> - > > > > Are we removing the following checks (ELF magic, flags) because they > > are superfluous? > > > > Should we mention this in the commit message? > > Damn, good catch, that's wrong. > > The only patches coming from previous iterations are the one factorizing > the e820 functions and this one, and both are wrong. I'm going to ditch > them and write whatever it's needed from scratch. > > >> - if (ldl_p(header) != 0x464c457f) { > >> - return false; /* no elfboot */ > >> - } > >> - > >> - bool elf_is64 = header[EI_CLASS] == ELFCLASS64; > >> - flags = elf_is64 ? > >> - ((Elf64_Ehdr *)header)->e_flags : ((Elf32_Ehdr *)header)->e_flags; > >> - > >> - if (flags & 0x00010004) { /* LOAD_ELF_HEADER_HAS_ADDR */ > >> - error_report("elfboot unsupported flags = %x", flags); > >> - exit(1); > >> - } > >> - > >> - uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; > >> - kernel_size = load_elf(kernel_filename, read_pvh_start_addr, > >> - NULL, &elf_note_type, &elf_entry, > >> - &elf_low, &elf_high, 0, I386_ELF_MACHINE, > >> - 0, 0); > >> - > >> - if (kernel_size < 0) { > >> - error_report("Error while loading elf kernel"); > >> - exit(1); > >> - } > >> - mh_load_addr = elf_low; > >> - elf_kernel_size = elf_high - elf_low; > >> - > >> - if (pvh_start_addr == 0) { > >> - error_report("Error loading uncompressed kernel without PVH ELF Note"); > >> - exit(1); > >> - } > >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); > >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); > >> - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); > >> - > >> - return true; > >> -} > >> - > >> static void load_linux(PCMachineState *pcms, > >> FWCfgState *fw_cfg) > >> { > >> @@ -1218,6 +1113,9 @@ static void load_linux(PCMachineState *pcms, > >> if (ldl_p(header+0x202) == 0x53726448) { > >> protocol = lduw_p(header+0x206); > >> } else { > >> + size_t pvh_start_addr; > >> + uint32_t mh_load_addr = 0; > >> + uint32_t elf_kernel_size = 0; > >> /* > >> * This could be a multiboot kernel. If it is, let's stop treating it > >> * like a Linux kernel. > >> @@ -1235,10 +1133,16 @@ static void load_linux(PCMachineState *pcms, > >> * If load_elfboot() is successful, populate the fw_cfg info. > >> */ > >> if (pcmc->pvh_enabled && > >> - load_elfboot(kernel_filename, kernel_size, > >> - header, pvh_start_addr, fw_cfg)) { > >> + pvh_load_elfboot(kernel_filename, > >> + &mh_load_addr, &elf_kernel_size)) { > >> fclose(f); > >> > >> + pvh_start_addr = pvh_get_start_addr(); > >> + > >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); > >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); > >> + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); > >> + > >> fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, > >> strlen(kernel_cmdline) + 1); > >> fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline); > >> diff --git a/hw/i386/pvh.c b/hw/i386/pvh.c > >> new file mode 100644 > >> index 0000000000..1c81727811 > >> --- /dev/null > >> +++ b/hw/i386/pvh.c > >> @@ -0,0 +1,113 @@ > >> +/* > >> + * PVH Boot Helper > >> + * > >> + * Copyright (C) 2019 Oracle > >> + * Copyright (C) 2019 Red Hat, Inc > >> + * > >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. > >> + * See the COPYING file in the top-level directory. > >> + * > >> + */ > >> + > >> +#include "qemu/osdep.h" > >> +#include "qemu/units.h" > >> +#include "qemu/error-report.h" > >> +#include "hw/loader.h" > >> +#include "cpu.h" > >> +#include "elf.h" > >> +#include "pvh.h" > >> + > >> +static size_t pvh_start_addr; > >> + > >> +size_t pvh_get_start_addr(void) > >> +{ > >> + return pvh_start_addr; > >> +} > >> + > >> +/* > >> + * The entry point into the kernel for PVH boot is different from > >> + * the native entry point. The PVH entry is defined by the x86/HVM > >> + * direct boot ABI and is available in an ELFNOTE in the kernel binary. > >> + * > >> + * This function is passed to load_elf() when it is called from > >> + * load_elfboot() which then additionally checks for an ELF Note of > >> + * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to > >> + * parse the PVH entry address from the ELF Note. > >> + * > >> + * Due to trickery in elf_opts.h, load_elf() is actually available as > >> + * load_elf32() or load_elf64() and this routine needs to be able > >> + * to deal with being called as 32 or 64 bit. > >> + * > >> + * The address of the PVH entry point is saved to the 'pvh_start_addr' > >> + * global variable. (although the entry point is 32-bit, the kernel > >> + * binary can be either 32-bit or 64-bit). > >> + */ > >> + > >> +static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) > >> +{ > >> + size_t *elf_note_data_addr; > >> + > >> + /* Check if ELF Note header passed in is valid */ > >> + if (arg1 == NULL) { > >> + return 0; > >> + } > >> + > >> + if (is64) { > >> + struct elf64_note *nhdr64 = (struct elf64_note *)arg1; > >> + uint64_t nhdr_size64 = sizeof(struct elf64_note); > >> + uint64_t phdr_align = *(uint64_t *)arg2; > >> + uint64_t nhdr_namesz = nhdr64->n_namesz; > >> + > >> + elf_note_data_addr = > >> + ((void *)nhdr64) + nhdr_size64 + > >> + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > >> + } else { > >> + struct elf32_note *nhdr32 = (struct elf32_note *)arg1; > >> + uint32_t nhdr_size32 = sizeof(struct elf32_note); > >> + uint32_t phdr_align = *(uint32_t *)arg2; > >> + uint32_t nhdr_namesz = nhdr32->n_namesz; > >> + > >> + elf_note_data_addr = > >> + ((void *)nhdr32) + nhdr_size32 + > >> + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); > >> + } > >> + > >> + pvh_start_addr = *elf_note_data_addr; > >> + > >> + return pvh_start_addr; > >> +} > >> + > >> +bool pvh_load_elfboot(const char *kernel_filename, > >> + uint32_t *mh_load_addr, > >> + uint32_t *elf_kernel_size) > >> +{ > >> + uint64_t elf_entry; > >> + uint64_t elf_low, elf_high; > >> + int kernel_size; > >> + uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; > >> + > >> + kernel_size = load_elf(kernel_filename, read_pvh_start_addr, > >> + NULL, &elf_note_type, &elf_entry, > >> + &elf_low, &elf_high, 0, I386_ELF_MACHINE, > >> + 0, 0); > >> + > >> + if (kernel_size < 0) { > >> + error_report("Error while loading elf kernel"); > >> + return false; > >> + } > >> + > >> + if (pvh_start_addr == 0) { > >> + error_report("Error loading uncompressed kernel without PVH ELF Note"); > >> + return false; > >> + } > >> + > >> + if (mh_load_addr) { > >> + *mh_load_addr = elf_low; > >> + } > >> + > >> + if (elf_kernel_size) { > >> + *elf_kernel_size = elf_high - elf_low; > >> + } > >> + > >> + return true; > >> +} > >> diff --git a/hw/i386/pvh.h b/hw/i386/pvh.h > >> new file mode 100644 > >> index 0000000000..ada67ff6e8 > >> --- /dev/null > >> +++ b/hw/i386/pvh.h > >> @@ -0,0 +1,10 @@ > >> +#ifndef HW_I386_PVH_H > >> +#define HW_I386_PVH_H > >> + > >> +size_t pvh_get_start_addr(void); > > > > What about adding "size_t *pvh_start_addr" to the pvh_load_elfboot()? > > Just an idea, I'm not sure if it is better... > > I agree. In fact, given that patch 4/8 extracts some common functions > from pc.c into x86.c, and load_linux is among these functions, perhaps > we can avoid creating an independent file and just put the PVH code > there. > > What do you think? Make sense to me, since it's going to be used by pc and microvm. Thanks, Stefano
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs index 5d9c9efd5f..c5f20bbd72 100644 --- a/hw/i386/Makefile.objs +++ b/hw/i386/Makefile.objs @@ -1,5 +1,6 @@ obj-$(CONFIG_KVM) += kvm/ obj-y += multiboot.o +obj-y += pvh.o obj-y += pc.o obj-$(CONFIG_I440FX) += pc_piix.o obj-$(CONFIG_Q35) += pc_q35.o diff --git a/hw/i386/pc.c b/hw/i386/pc.c index bad866fe44..10e4ced0c6 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -42,6 +42,7 @@ #include "elf.h" #include "migration/vmstate.h" #include "multiboot.h" +#include "pvh.h" #include "hw/timer/mc146818rtc.h" #include "hw/dma/i8257.h" #include "hw/timer/i8254.h" @@ -116,9 +117,6 @@ static struct e820_entry *e820_table; static unsigned e820_entries; struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX}; -/* Physical Address of PVH entry point read from kernel ELF NOTE */ -static size_t pvh_start_addr; - GlobalProperty pc_compat_4_1[] = {}; const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1); @@ -1076,109 +1074,6 @@ struct setup_data { uint8_t data[0]; } __attribute__((packed)); - -/* - * The entry point into the kernel for PVH boot is different from - * the native entry point. The PVH entry is defined by the x86/HVM - * direct boot ABI and is available in an ELFNOTE in the kernel binary. - * - * This function is passed to load_elf() when it is called from - * load_elfboot() which then additionally checks for an ELF Note of - * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to - * parse the PVH entry address from the ELF Note. - * - * Due to trickery in elf_opts.h, load_elf() is actually available as - * load_elf32() or load_elf64() and this routine needs to be able - * to deal with being called as 32 or 64 bit. - * - * The address of the PVH entry point is saved to the 'pvh_start_addr' - * global variable. (although the entry point is 32-bit, the kernel - * binary can be either 32-bit or 64-bit). - */ -static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) -{ - size_t *elf_note_data_addr; - - /* Check if ELF Note header passed in is valid */ - if (arg1 == NULL) { - return 0; - } - - if (is64) { - struct elf64_note *nhdr64 = (struct elf64_note *)arg1; - uint64_t nhdr_size64 = sizeof(struct elf64_note); - uint64_t phdr_align = *(uint64_t *)arg2; - uint64_t nhdr_namesz = nhdr64->n_namesz; - - elf_note_data_addr = - ((void *)nhdr64) + nhdr_size64 + - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); - } else { - struct elf32_note *nhdr32 = (struct elf32_note *)arg1; - uint32_t nhdr_size32 = sizeof(struct elf32_note); - uint32_t phdr_align = *(uint32_t *)arg2; - uint32_t nhdr_namesz = nhdr32->n_namesz; - - elf_note_data_addr = - ((void *)nhdr32) + nhdr_size32 + - QEMU_ALIGN_UP(nhdr_namesz, phdr_align); - } - - pvh_start_addr = *elf_note_data_addr; - - return pvh_start_addr; -} - -static bool load_elfboot(const char *kernel_filename, - int kernel_file_size, - uint8_t *header, - size_t pvh_xen_start_addr, - FWCfgState *fw_cfg) -{ - uint32_t flags = 0; - uint32_t mh_load_addr = 0; - uint32_t elf_kernel_size = 0; - uint64_t elf_entry; - uint64_t elf_low, elf_high; - int kernel_size; - - if (ldl_p(header) != 0x464c457f) { - return false; /* no elfboot */ - } - - bool elf_is64 = header[EI_CLASS] == ELFCLASS64; - flags = elf_is64 ? - ((Elf64_Ehdr *)header)->e_flags : ((Elf32_Ehdr *)header)->e_flags; - - if (flags & 0x00010004) { /* LOAD_ELF_HEADER_HAS_ADDR */ - error_report("elfboot unsupported flags = %x", flags); - exit(1); - } - - uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; - kernel_size = load_elf(kernel_filename, read_pvh_start_addr, - NULL, &elf_note_type, &elf_entry, - &elf_low, &elf_high, 0, I386_ELF_MACHINE, - 0, 0); - - if (kernel_size < 0) { - error_report("Error while loading elf kernel"); - exit(1); - } - mh_load_addr = elf_low; - elf_kernel_size = elf_high - elf_low; - - if (pvh_start_addr == 0) { - error_report("Error loading uncompressed kernel without PVH ELF Note"); - exit(1); - } - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); - fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); - - return true; -} - static void load_linux(PCMachineState *pcms, FWCfgState *fw_cfg) { @@ -1218,6 +1113,9 @@ static void load_linux(PCMachineState *pcms, if (ldl_p(header+0x202) == 0x53726448) { protocol = lduw_p(header+0x206); } else { + size_t pvh_start_addr; + uint32_t mh_load_addr = 0; + uint32_t elf_kernel_size = 0; /* * This could be a multiboot kernel. If it is, let's stop treating it * like a Linux kernel. @@ -1235,10 +1133,16 @@ static void load_linux(PCMachineState *pcms, * If load_elfboot() is successful, populate the fw_cfg info. */ if (pcmc->pvh_enabled && - load_elfboot(kernel_filename, kernel_size, - header, pvh_start_addr, fw_cfg)) { + pvh_load_elfboot(kernel_filename, + &mh_load_addr, &elf_kernel_size)) { fclose(f); + pvh_start_addr = pvh_get_start_addr(); + + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr); + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr); + fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, elf_kernel_size); + fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline) + 1); fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline); diff --git a/hw/i386/pvh.c b/hw/i386/pvh.c new file mode 100644 index 0000000000..1c81727811 --- /dev/null +++ b/hw/i386/pvh.c @@ -0,0 +1,113 @@ +/* + * PVH Boot Helper + * + * Copyright (C) 2019 Oracle + * Copyright (C) 2019 Red Hat, Inc + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "qemu/units.h" +#include "qemu/error-report.h" +#include "hw/loader.h" +#include "cpu.h" +#include "elf.h" +#include "pvh.h" + +static size_t pvh_start_addr; + +size_t pvh_get_start_addr(void) +{ + return pvh_start_addr; +} + +/* + * The entry point into the kernel for PVH boot is different from + * the native entry point. The PVH entry is defined by the x86/HVM + * direct boot ABI and is available in an ELFNOTE in the kernel binary. + * + * This function is passed to load_elf() when it is called from + * load_elfboot() which then additionally checks for an ELF Note of + * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to + * parse the PVH entry address from the ELF Note. + * + * Due to trickery in elf_opts.h, load_elf() is actually available as + * load_elf32() or load_elf64() and this routine needs to be able + * to deal with being called as 32 or 64 bit. + * + * The address of the PVH entry point is saved to the 'pvh_start_addr' + * global variable. (although the entry point is 32-bit, the kernel + * binary can be either 32-bit or 64-bit). + */ + +static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64) +{ + size_t *elf_note_data_addr; + + /* Check if ELF Note header passed in is valid */ + if (arg1 == NULL) { + return 0; + } + + if (is64) { + struct elf64_note *nhdr64 = (struct elf64_note *)arg1; + uint64_t nhdr_size64 = sizeof(struct elf64_note); + uint64_t phdr_align = *(uint64_t *)arg2; + uint64_t nhdr_namesz = nhdr64->n_namesz; + + elf_note_data_addr = + ((void *)nhdr64) + nhdr_size64 + + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); + } else { + struct elf32_note *nhdr32 = (struct elf32_note *)arg1; + uint32_t nhdr_size32 = sizeof(struct elf32_note); + uint32_t phdr_align = *(uint32_t *)arg2; + uint32_t nhdr_namesz = nhdr32->n_namesz; + + elf_note_data_addr = + ((void *)nhdr32) + nhdr_size32 + + QEMU_ALIGN_UP(nhdr_namesz, phdr_align); + } + + pvh_start_addr = *elf_note_data_addr; + + return pvh_start_addr; +} + +bool pvh_load_elfboot(const char *kernel_filename, + uint32_t *mh_load_addr, + uint32_t *elf_kernel_size) +{ + uint64_t elf_entry; + uint64_t elf_low, elf_high; + int kernel_size; + uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY; + + kernel_size = load_elf(kernel_filename, read_pvh_start_addr, + NULL, &elf_note_type, &elf_entry, + &elf_low, &elf_high, 0, I386_ELF_MACHINE, + 0, 0); + + if (kernel_size < 0) { + error_report("Error while loading elf kernel"); + return false; + } + + if (pvh_start_addr == 0) { + error_report("Error loading uncompressed kernel without PVH ELF Note"); + return false; + } + + if (mh_load_addr) { + *mh_load_addr = elf_low; + } + + if (elf_kernel_size) { + *elf_kernel_size = elf_high - elf_low; + } + + return true; +} diff --git a/hw/i386/pvh.h b/hw/i386/pvh.h new file mode 100644 index 0000000000..ada67ff6e8 --- /dev/null +++ b/hw/i386/pvh.h @@ -0,0 +1,10 @@ +#ifndef HW_I386_PVH_H +#define HW_I386_PVH_H + +size_t pvh_get_start_addr(void); + +bool pvh_load_elfboot(const char *kernel_filename, + uint32_t *mh_load_addr, + uint32_t *elf_kernel_size); + +#endif
Extract PVH related functions from pc.c, and put them in pvh.c, so they can be shared with other components. Signed-off-by: Sergio Lopez <slp@redhat.com> --- hw/i386/Makefile.objs | 1 + hw/i386/pc.c | 120 +++++------------------------------------- hw/i386/pvh.c | 113 +++++++++++++++++++++++++++++++++++++++ hw/i386/pvh.h | 10 ++++ 4 files changed, 136 insertions(+), 108 deletions(-) create mode 100644 hw/i386/pvh.c create mode 100644 hw/i386/pvh.h