diff mbox series

[2/3] xen/x86: bzImage parse kernel_alignment

Message ID 20240306185032.103216-3-jason.andryuk@amd.com (mailing list archive)
State Superseded
Headers show
Series x86/pvh: Support relocating dom0 kernel | expand

Commit Message

Jason Andryuk March 6, 2024, 6:50 p.m. UTC
Expand bzimage_parse() to return kernel_alignment from the setup_header.
This will be needed if loading a PVH kernel at a physical offset to
compensate for a reserved E820 region.

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 | 3 +--
 xen/arch/x86/pv/dom0_build.c       | 2 +-
 4 files changed, 8 insertions(+), 5 deletions(-)

Comments

Stefano Stabellini March 7, 2024, 2:09 a.m. UTC | #1
On Wed, 6 Mar 2024, Jason Andryuk wrote:
> Expand bzimage_parse() to return kernel_alignment from the setup_header.
> This will be needed if loading a PVH kernel at a physical offset to
> compensate for a reserved E820 region.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/x86/bzimage.c             | 4 +++-
>  xen/arch/x86/hvm/dom0_build.c      | 4 +++-
>  xen/arch/x86/include/asm/bzimage.h | 3 +--
>  xen/arch/x86/pv/dom0_build.c       | 2 +-
>  4 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/bzimage.c b/xen/arch/x86/bzimage.c
> index ac4fd428be..0f4cfc49f7 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 long *image_len, unsigned int *align)
>  {
>      struct setup_header *hdr = (struct setup_header *)(*image_start);
>      int err = bzimage_check(hdr, *image_len);
> @@ -118,6 +118,8 @@ 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 0ceda4140b..bbae8a5645 100644
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -548,12 +548,14 @@ 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;
>  
> -    if ( (rc = bzimage_parse(image_base, &image_start, &image_len)) != 0 )
> +    rc = bzimage_parse(image_base, &image_start, &image_len, &align);
> +    if ( rc != 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 7ed69d3910..de4e9a446f 100644
> --- a/xen/arch/x86/include/asm/bzimage.h
> +++ b/xen/arch/x86/include/asm/bzimage.h
> @@ -4,8 +4,7 @@
>  #include <xen/init.h>
>  
>  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 long *image_len, unsigned int *align);
>  
>  #endif /* __X86_BZIMAGE_H__ */
> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> index d8043fa58a..e9fa8a9a82 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)) != 0 )
> +    if ( (rc = bzimage_parse(image_base, &image_start, &image_len, NULL)) != 0 )
>          return rc;
>  
>      if ( (rc = elf_init(&elf, image_start, image_len)) != 0 )
> -- 
> 2.44.0
> 
>
Jan Beulich March 7, 2024, 8:26 a.m. UTC | #2
On 07.03.2024 03:09, Stefano Stabellini wrote:
> On Wed, 6 Mar 2024, Jason Andryuk wrote:
>> Expand bzimage_parse() to return kernel_alignment from the setup_header.
>> This will be needed if loading a PVH kernel at a physical offset to
>> compensate for a reserved E820 region.
>>
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Acked-by: Jan Beulich <jbeulich@suse.com>
with two remarks:

>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> @@ -548,12 +548,14 @@ 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;

Strictly speaking this isn't needed here, yet, and would suffice when added
in the next patch. But I'm okay with keeping it.

>> --- a/xen/arch/x86/include/asm/bzimage.h
>> +++ b/xen/arch/x86/include/asm/bzimage.h
>> @@ -4,8 +4,7 @@
>>  #include <xen/init.h>
>>  
>>  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 long *image_len, unsigned int *align);

Any particular reason for dropping the blank line? I'd prefer if it was kept,
and I may take the liberty to respectively adjust the patch while committing.

Jan
Jason Andryuk March 7, 2024, 3:06 p.m. UTC | #3
On 2024-03-07 03:26, Jan Beulich wrote:
> On 07.03.2024 03:09, Stefano Stabellini wrote:
>> On Wed, 6 Mar 2024, Jason Andryuk wrote:
>>> Expand bzimage_parse() to return kernel_alignment from the setup_header.
>>> This will be needed if loading a PVH kernel at a physical offset to
>>> compensate for a reserved E820 region.
>>>
>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>
>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with two remarks:
> 
>>> --- a/xen/arch/x86/hvm/dom0_build.c
>>> +++ b/xen/arch/x86/hvm/dom0_build.c
>>> @@ -548,12 +548,14 @@ 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;
> 
> Strictly speaking this isn't needed here, yet, and would suffice when added
> in the next patch. But I'm okay with keeping it.
> 
>>> --- a/xen/arch/x86/include/asm/bzimage.h
>>> +++ b/xen/arch/x86/include/asm/bzimage.h
>>> @@ -4,8 +4,7 @@
>>>   #include <xen/init.h>
>>>   
>>>   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 long *image_len, unsigned int *align);
> 
> Any particular reason for dropping the blank line? I'd prefer if it was kept,
> and I may take the liberty to respectively adjust the patch while committing.

No, no particular reason.  The blank line can be retained.

Thanks,
Jason
diff mbox series

Patch

diff --git a/xen/arch/x86/bzimage.c b/xen/arch/x86/bzimage.c
index ac4fd428be..0f4cfc49f7 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 long *image_len, unsigned int *align)
 {
     struct setup_header *hdr = (struct setup_header *)(*image_start);
     int err = bzimage_check(hdr, *image_len);
@@ -118,6 +118,8 @@  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 0ceda4140b..bbae8a5645 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -548,12 +548,14 @@  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;
 
-    if ( (rc = bzimage_parse(image_base, &image_start, &image_len)) != 0 )
+    rc = bzimage_parse(image_base, &image_start, &image_len, &align);
+    if ( rc != 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 7ed69d3910..de4e9a446f 100644
--- a/xen/arch/x86/include/asm/bzimage.h
+++ b/xen/arch/x86/include/asm/bzimage.h
@@ -4,8 +4,7 @@ 
 #include <xen/init.h>
 
 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 long *image_len, unsigned int *align);
 
 #endif /* __X86_BZIMAGE_H__ */
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index d8043fa58a..e9fa8a9a82 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)) != 0 )
+    if ( (rc = bzimage_parse(image_base, &image_start, &image_len, NULL)) != 0 )
         return rc;
 
     if ( (rc = elf_init(&elf, image_start, image_len)) != 0 )