[PATCH 0/2] improve loading of 32 bit PVH kernels

[PATCH 0/2] improve loading of 32 bit PVH kernels

[David Edmondson]

While testing a non-Linux 32 bit PVH "kernel" (it's really just a
bunch of test code), I ran into some problems with the current PVH
loader in QEMU, which seems to work somewhat by accident for 32 bit
kernels today, having been fortunate in the layout of the object files
it typically encounters.

Non-PAE 32 bit Linux kernels still don't appear to work even with this
change, but I think that problem is somewhere else (the kernel is
loaded and starts, but the VM resets when it attempts to enable PG/PE)
and I haven't tracked it down yet.

David Edmondson (2):
  elf_ops: correct loading of 32 bit PVH kernel
  x86/pvh: extract only 4 bytes of start address for 32 bit kernels

 hw/i386/x86.c        | 6 ++++--
 include/hw/elf_ops.h | 4 +---
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.30.0

[PATCH 1/2] elf_ops: correct loading of 32 bit PVH kernel

[David Edmondson]

Because sizeof(struct elf64_note) == sizeof(struct elf32_note),
attempting to use the size of the currently defined struct elf_note as
a discriminator for whether the object being loaded is 64 bit in
load_elf() fails.

Instead, take advantage of the existing glue parameter SZ, which is
defined as 32 or 64 in the respective variants of load_elf().

Fixes: 696aa04c84c6 ("elf-ops.h: Add get_elf_note_type()")
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 include/hw/elf_ops.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 8e8436831d..78409ab34a 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -598,9 +598,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,
             nhdr = glue(get_elf_note_type, SZ)(nhdr, file_size, ph->p_align,
                                                *(uint64_t *)translate_opaque);
             if (nhdr != NULL) {
-                bool is64 =
-                    sizeof(struct elf_note) == sizeof(struct elf64_note);
-                elf_note_fn((void *)nhdr, (void *)&ph->p_align, is64);
+                elf_note_fn((void *)nhdr, (void *)&ph->p_align, SZ == 64);
             }
             data = NULL;
         }
-- 
2.30.0

Re: [PATCH 1/2] elf_ops: correct loading of 32 bit PVH kernel

[Philippe Mathieu-Daudé]

On 3/2/21 10:03 AM, David Edmondson wrote:
> Because sizeof(struct elf64_note) == sizeof(struct elf32_note),
> attempting to use the size of the currently defined struct elf_note as
> a discriminator for whether the object being loaded is 64 bit in
> load_elf() fails.
> 
> Instead, take advantage of the existing glue parameter SZ, which is
> defined as 32 or 64 in the respective variants of load_elf().
> 
> Fixes: 696aa04c84c6 ("elf-ops.h: Add get_elf_note_type()")
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
>  include/hw/elf_ops.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 1/2] elf_ops: correct loading of 32 bit PVH kernel

[Stefano Garzarella]

On Tue, Mar 02, 2021 at 09:03:14AM +0000, David Edmondson wrote:
>Because sizeof(struct elf64_note) == sizeof(struct elf32_note),
>attempting to use the size of the currently defined struct elf_note as
>a discriminator for whether the object being loaded is 64 bit in
>load_elf() fails.
>
>Instead, take advantage of the existing glue parameter SZ, which is
>defined as 32 or 64 in the respective variants of load_elf().
>
>Fixes: 696aa04c84c6 ("elf-ops.h: Add get_elf_note_type()")
>Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>---
> include/hw/elf_ops.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
>index 8e8436831d..78409ab34a 100644
>--- a/include/hw/elf_ops.h
>+++ b/include/hw/elf_ops.h
>@@ -598,9 +598,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>             nhdr = glue(get_elf_note_type, SZ)(nhdr, file_size, ph->p_align,
>                                                *(uint64_t *)translate_opaque);
>             if (nhdr != NULL) {
>-                bool is64 =
>-                    sizeof(struct elf_note) == sizeof(struct elf64_note);
>-                elf_note_fn((void *)nhdr, (void *)&ph->p_align, is64);
>+                elf_note_fn((void *)nhdr, (void *)&ph->p_align, SZ == 64);
>             }
>             data = NULL;
>         }
>-- 
>2.30.0
>
>

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [PATCH 1/2] elf_ops: correct loading of 32 bit PVH kernel

[Alex Bennée]

David Edmondson <david.edmondson@oracle.com> writes:

> Because sizeof(struct elf64_note) == sizeof(struct elf32_note),
> attempting to use the size of the currently defined struct elf_note as
> a discriminator for whether the object being loaded is 64 bit in
> load_elf() fails.
>
> Instead, take advantage of the existing glue parameter SZ, which is
> defined as 32 or 64 in the respective variants of load_elf().
>
> Fixes: 696aa04c84c6 ("elf-ops.h: Add get_elf_note_type()")
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

[PATCH 2/2] x86/pvh: extract only 4 bytes of start address for 32 bit kernels

[David Edmondson]

When loading the PVH start address from a 32 bit ELF note, extract
only the appropriate number of bytes.

Fixes: ab969087da65 ("pvh: Boot uncompressed kernel using direct boot ABI")
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 hw/i386/x86.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6329f90ef9..7865660e2c 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -690,6 +690,8 @@ static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64)
         elf_note_data_addr =
             ((void *)nhdr64) + nhdr_size64 +
             QEMU_ALIGN_UP(nhdr_namesz, phdr_align);
+
+        pvh_start_addr = *elf_note_data_addr;
     } else {
         struct elf32_note *nhdr32 = (struct elf32_note *)arg1;
         uint32_t nhdr_size32 = sizeof(struct elf32_note);
@@ -699,9 +701,9 @@ static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64)
         elf_note_data_addr =
             ((void *)nhdr32) + nhdr_size32 +
             QEMU_ALIGN_UP(nhdr_namesz, phdr_align);
-    }
 
-    pvh_start_addr = *elf_note_data_addr;
+        pvh_start_addr = *(uint32_t *)elf_note_data_addr;
+    }
 
     return pvh_start_addr;
 }
-- 
2.30.0

Re: [PATCH 2/2] x86/pvh: extract only 4 bytes of start address for 32 bit kernels

[Stefano Garzarella]

On Tue, Mar 02, 2021 at 09:03:15AM +0000, David Edmondson wrote:
>When loading the PVH start address from a 32 bit ELF note, extract
>only the appropriate number of bytes.
>
>Fixes: ab969087da65 ("pvh: Boot uncompressed kernel using direct boot ABI")
>Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>---
> hw/i386/x86.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>index 6329f90ef9..7865660e2c 100644
>--- a/hw/i386/x86.c
>+++ b/hw/i386/x86.c
>@@ -690,6 +690,8 @@ static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64)
>         elf_note_data_addr =
>             ((void *)nhdr64) + nhdr_size64 +
>             QEMU_ALIGN_UP(nhdr_namesz, phdr_align);
>+
>+        pvh_start_addr = *elf_note_data_addr;
>     } else {
>         struct elf32_note *nhdr32 = (struct elf32_note *)arg1;
>         uint32_t nhdr_size32 = sizeof(struct elf32_note);
>@@ -699,9 +701,9 @@ static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64)
>         elf_note_data_addr =
>             ((void *)nhdr32) + nhdr_size32 +
>             QEMU_ALIGN_UP(nhdr_namesz, phdr_align);
>-    }
>
>-    pvh_start_addr = *elf_note_data_addr;
>+        pvh_start_addr = *(uint32_t *)elf_note_data_addr;
>+    }
>
>     return pvh_start_addr;
> }
>-- 
>2.30.0
>
>

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [PATCH 0/2] improve loading of 32 bit PVH kernels

[Philippe Mathieu-Daudé]

Cc'ing Stefano

On 3/2/21 10:03 AM, David Edmondson wrote:
> While testing a non-Linux 32 bit PVH "kernel" (it's really just a
> bunch of test code), I ran into some problems with the current PVH
> loader in QEMU, which seems to work somewhat by accident for 32 bit
> kernels today, having been fortunate in the layout of the object files
> it typically encounters.
> 
> Non-PAE 32 bit Linux kernels still don't appear to work even with this
> change, but I think that problem is somewhere else (the kernel is
> loaded and starts, but the VM resets when it attempts to enable PG/PE)
> and I haven't tracked it down yet.
> 
> David Edmondson (2):
>   elf_ops: correct loading of 32 bit PVH kernel
>   x86/pvh: extract only 4 bytes of start address for 32 bit kernels
> 
>  hw/i386/x86.c        | 6 ++++--
>  include/hw/elf_ops.h | 4 +---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 

Re: [PATCH 0/2] improve loading of 32 bit PVH kernels

[Paolo Bonzini]

On 02/03/21 10:03, David Edmondson wrote:
> While testing a non-Linux 32 bit PVH "kernel" (it's really just a
> bunch of test code), I ran into some problems with the current PVH
> loader in QEMU, which seems to work somewhat by accident for 32 bit
> kernels today, having been fortunate in the layout of the object files
> it typically encounters.
> 
> Non-PAE 32 bit Linux kernels still don't appear to work even with this
> change, but I think that problem is somewhere else (the kernel is
> loaded and starts, but the VM resets when it attempts to enable PG/PE)
> and I haven't tracked it down yet.
> 
> David Edmondson (2):
>    elf_ops: correct loading of 32 bit PVH kernel
>    x86/pvh: extract only 4 bytes of start address for 32 bit kernels
> 
>   hw/i386/x86.c        | 6 ++++--
>   include/hw/elf_ops.h | 4 +---
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 

Queued, thanks.

Paolo