On 2020-10-20 13:30, Andre Przywara wrote:
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.
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
Isn't that arm64 specific? AFAICR, 32bit doesn't need any of this.
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");
Same thing: 32bit doesn't require any preliminary 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);
I'd prefer the whole thing to be kept in the arm64-specific code, TBH.
Or the 32bit support to be purged from kvmtool, which would simplify
tons of things.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
