| Message ID | 20240313193021.241764-2-jason.andryuk@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/pvh: Support relocating dom0 kernel | expand |
On 13.03.2024 20:30, Jason Andryuk wrote: > A new ELF note will specify the alignment for a relocatable PVH kernel. > ELF notes are suitable for vmlinux and other ELF files, so this > Linux-specific bzImage parsing in unnecessary. Hmm, shouldn't the order of attempts to figure the alignment be ELF note, ELF header, and then 2Mb? Jan
On 2024-03-14 03:11, Jan Beulich wrote: > On 13.03.2024 20:30, Jason Andryuk wrote: >> A new ELF note will specify the alignment for a relocatable PVH kernel. >> ELF notes are suitable for vmlinux and other ELF files, so this >> Linux-specific bzImage parsing in unnecessary. > > Hmm, shouldn't the order of attempts to figure the alignment be ELF note, > ELF header, and then 2Mb? This patch series makes Xen only relocate when the ELF Note is present. Linux PVH entry points today are not relocatable, so that needs to be specified some way. This new note also includes the alignment, so there is no need to try other means of obtaining the alignment. Also, the ELF header values didn't change with CONFIG_PHYSICAL_ALIGN, so that didn't seem reliable. Thanks, Jason
On 14.03.2024 14:01, Jason Andryuk wrote: > On 2024-03-14 03:11, Jan Beulich wrote: >> On 13.03.2024 20:30, Jason Andryuk wrote: >>> A new ELF note will specify the alignment for a relocatable PVH kernel. >>> ELF notes are suitable for vmlinux and other ELF files, so this >>> Linux-specific bzImage parsing in unnecessary. >> >> Hmm, shouldn't the order of attempts to figure the alignment be ELF note, >> ELF header, and then 2Mb? > > This patch series makes Xen only relocate when the ELF Note is present. > Linux PVH entry points today are not relocatable, so that needs to be > specified some way. This new note also includes the alignment, so there > is no need to try other means of obtaining the alignment. Going yet beyond what I said in reply to the patch: The mere presence of the note, with no data at all, could be sufficient to indicate the kernel is relocatable, when no other restrictions (min, max, align) apply. > Also, the ELF header values didn't change with CONFIG_PHYSICAL_ALIGN, so > that didn't seem reliable. Well, that's an observation for Linux. But the interface here ought to be OS-agnostic. Jan
diff --git a/xen/arch/x86/bzimage.c b/xen/arch/x86/bzimage.c index 0f4cfc49f7..ac4fd428be 100644 --- a/xen/arch/x86/bzimage.c +++ b/xen/arch/x86/bzimage.c @@ -105,7 +105,7 @@ unsigned long __init bzimage_headroom(void *image_start, } int __init bzimage_parse(void *image_base, void **image_start, - unsigned long *image_len, unsigned int *align) + unsigned long *image_len) { struct setup_header *hdr = (struct setup_header *)(*image_start); int err = bzimage_check(hdr, *image_len); @@ -118,8 +118,6 @@ int __init bzimage_parse(void *image_base, void **image_start, { *image_start += (hdr->setup_sects + 1) * 512 + hdr->payload_offset; *image_len = hdr->payload_length; - if ( align ) - *align = hdr->kernel_alignment; } if ( elf_is_elfbinary(*image_start, *image_len) ) diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c index bbae8a5645..0ceda4140b 100644 --- a/xen/arch/x86/hvm/dom0_build.c +++ b/xen/arch/x86/hvm/dom0_build.c @@ -548,14 +548,12 @@ static int __init pvh_load_kernel(struct domain *d, const module_t *image, struct elf_binary elf; struct elf_dom_parms parms; paddr_t last_addr; - unsigned int align = 0; struct hvm_start_info start_info = { 0 }; struct hvm_modlist_entry mod = { 0 }; struct vcpu *v = d->vcpu[0]; int rc; - rc = bzimage_parse(image_base, &image_start, &image_len, &align); - if ( rc != 0 ) + if ( (rc = bzimage_parse(image_base, &image_start, &image_len)) != 0 ) { printk("Error trying to detect bz compressed kernel\n"); return rc; diff --git a/xen/arch/x86/include/asm/bzimage.h b/xen/arch/x86/include/asm/bzimage.h index 2e04f5cc7b..7ed69d3910 100644 --- a/xen/arch/x86/include/asm/bzimage.h +++ b/xen/arch/x86/include/asm/bzimage.h @@ -6,6 +6,6 @@ unsigned long bzimage_headroom(void *image_start, unsigned long image_length); int bzimage_parse(void *image_base, void **image_start, - unsigned long *image_len, unsigned int *align); + unsigned long *image_len); #endif /* __X86_BZIMAGE_H__ */ diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c index e9fa8a9a82..d8043fa58a 100644 --- a/xen/arch/x86/pv/dom0_build.c +++ b/xen/arch/x86/pv/dom0_build.c @@ -416,7 +416,7 @@ int __init dom0_construct_pv(struct domain *d, d->max_pages = ~0U; - if ( (rc = bzimage_parse(image_base, &image_start, &image_len, NULL)) != 0 ) + if ( (rc = bzimage_parse(image_base, &image_start, &image_len)) != 0 ) return rc; if ( (rc = elf_init(&elf, image_start, image_len)) != 0 )
A new ELF note will specify the alignment for a relocatable PVH kernel. ELF notes are suitable for vmlinux and other ELF files, so this Linux-specific bzImage parsing in unnecessary. This reverts commit c44cac229067faeec8f49247d1cf281723ac2d40. Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> --- xen/arch/x86/bzimage.c | 4 +--- xen/arch/x86/hvm/dom0_build.c | 4 +--- xen/arch/x86/include/asm/bzimage.h | 2 +- xen/arch/x86/pv/dom0_build.c | 2 +- 4 files changed, 4 insertions(+), 8 deletions(-)