Re: [PATCH kvmtool] arm64: Determine kernel offset even on non-seekable file descriptors

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

 



On 20/10/2020 13:30, Andre Przywara wrote:

Hi,

> Commit c9acdae1d2e7 ("arm64: Use default kernel offset when the image
> file can't be seeked") "guessed" the arm64 kernel offset to be the old
> default of 512K if the file descriptor for the kernel image could not
> be seeked. This mostly works today because most modern kernels are
> somewhat forgiving when loaded at the wrong offset, but emit a warning:
> 
> [Firmware Bug]: Kernel image misaligned at boot, please fix your bootloader!
> 
> To fix this properly, let's drop the seek operation altogether, instead
> give the kernel header parsing function a memory buffer, containing the
> first 64 bytes of the kernel file. We read the rest of the file into the
> right location after this function has decoded the proper kernel offset.
> 
> This brings back proper loading even for kernels loaded via pipes.

are there any opinions on this?
Look, Marc, it's a negative diffstat ;-)

Cheers,
Andre

> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
>  arm/aarch64/include/kvm/kvm-arch.h |  3 ++-
>  arm/aarch64/kvm.c                  | 26 ++++++++------------------
>  arm/kvm.c                          | 13 ++++++++++---
>  3 files changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
> index 55ef8ed1..7e6cffb6 100644
> --- a/arm/aarch64/include/kvm/kvm-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-arch.h
> @@ -2,7 +2,8 @@
>  #define KVM__KVM_ARCH_H
>  
>  struct kvm;
> -unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
> +unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, void *header,
> +					     unsigned int bufsize);
>  
>  #define ARM_MAX_MEMORY(kvm)	((kvm)->cfg.arch.aarch32_guest	?	\
>  				ARM_LOMAP_MAX_MEMORY		:	\
> diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
> index 49e1dd31..9a6460ac 100644
> --- a/arm/aarch64/kvm.c
> +++ b/arm/aarch64/kvm.c
> @@ -10,39 +10,29 @@
>   * instead of Little-Endian. BE kernels of this vintage may fail to
>   * boot. See Documentation/arm64/booting.rst in your local kernel tree.
>   */
> -unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd)
> +unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm,
> +					     void *buffer, unsigned int bufsize)
>  {
> -	struct arm64_image_header header;
> -	off_t cur_offset;
> -	ssize_t size;
> +	struct arm64_image_header *header = buffer;
>  	const char *warn_str;
>  
>  	/* the 32bit kernel offset is a well known value */
>  	if (kvm->cfg.arch.aarch32_guest)
>  		return 0x8000;
>  
> -	cur_offset = lseek(fd, 0, SEEK_CUR);
> -	if (cur_offset == (off_t)-1 ||
> -	    lseek(fd, 0, SEEK_SET) == (off_t)-1) {
> -		warn_str = "Failed to seek in kernel image file";
> +	if (bufsize < sizeof(*header)) {
> +		warn_str = "Provided kernel header too small";
>  		goto fail;
>  	}
>  
> -	size = xread(fd, &header, sizeof(header));
> -	if (size < 0 || (size_t)size < sizeof(header))
> -		die("Failed to read kernel image header");
> -
> -	lseek(fd, cur_offset, SEEK_SET);
> -
> -	if (memcmp(&header.magic, ARM64_IMAGE_MAGIC, sizeof(header.magic)))
> +	if (memcmp(&header->magic, ARM64_IMAGE_MAGIC, sizeof(header->magic)))
>  		pr_warning("Kernel image magic not matching");
>  
> -	if (le64_to_cpu(header.image_size))
> -		return le64_to_cpu(header.text_offset);
> +	if (le64_to_cpu(header->image_size))
> +		return le64_to_cpu(header->text_offset);
>  
>  	warn_str = "Image size is 0";
>  fail:
>  	pr_warning("%s, assuming TEXT_OFFSET to be 0x80000", warn_str);
>  	return 0x80000;
>  }
> -
> diff --git a/arm/kvm.c b/arm/kvm.c
> index 5aea18fe..685fabb1 100644
> --- a/arm/kvm.c
> +++ b/arm/kvm.c
> @@ -90,12 +90,14 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
>  
>  #define FDT_ALIGN	SZ_2M
>  #define INITRD_ALIGN	4
> +#define MAX_KERNEL_HEADER_SIZE	64
>  bool kvm__arch_load_kernel_image(struct kvm *kvm, int fd_kernel, int fd_initrd,
>  				 const char *kernel_cmdline)
>  {
>  	void *pos, *kernel_end, *limit;
>  	unsigned long guest_addr;
>  	ssize_t file_size;
> +	char header[MAX_KERNEL_HEADER_SIZE];
>  
>  	/*
>  	 * Linux requires the initrd and dtb to be mapped inside lowmem,
> @@ -103,16 +105,21 @@ bool kvm__arch_load_kernel_image(struct kvm *kvm, int fd_kernel, int fd_initrd,
>  	 */
>  	limit = kvm->ram_start + min(kvm->ram_size, (u64)SZ_256M) - 1;
>  
> -	pos = kvm->ram_start + kvm__arch_get_kern_offset(kvm, fd_kernel);
> +	if (xread(fd_kernel, header, sizeof(header)) != sizeof(header))
> +		die_perror("kernel header read");
> +	pos = kvm->ram_start + kvm__arch_get_kern_offset(kvm, header,
> +							 sizeof(header));
>  	kvm->arch.kern_guest_start = host_to_guest_flat(kvm, pos);
> -	file_size = read_file(fd_kernel, pos, limit - pos);
> +	memcpy(pos, header, sizeof(header));
> +	file_size = read_file(fd_kernel, pos + sizeof(header),
> +			      limit - pos - sizeof(header));
>  	if (file_size < 0) {
>  		if (errno == ENOMEM)
>  			die("kernel image too big to contain in guest memory.");
>  
>  		die_perror("kernel read");
>  	}
> -	kernel_end = pos + file_size;
> +	kernel_end = pos + file_size + sizeof(header);
>  	pr_debug("Loaded kernel to 0x%llx (%zd bytes)",
>  		 kvm->arch.kern_guest_start, file_size);
>  
> 




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux