All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] s390x/ipl: support extended kernel command line size
@ 2021-11-22 11:29 Marc Hartmayer
  2021-11-22 11:54 ` David Hildenbrand
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Marc Hartmayer @ 2021-11-22 11:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: Halil Pasic, Christian Borntraeger, qemu-s390x, Thomas Huth,
	David Hildenbrand

In the past s390 used a fixed command line length of 896 bytes. This has changed
with the Linux commit 5ecb2da660ab ("s390: support command lines longer than 896
bytes"). There is now a parm area indicating the maximum command line size. This
parm area has always been initialized to zero, so with older kernels this field
would read zero and we must then assume that only 896 bytes are available.

Acked-by: Viktor Mihajlovski <mihajlov@de.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
---
 hw/s390x/ipl.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 7ddca0127fc2..092c66b3f9f1 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -37,8 +37,9 @@
 
 #define KERN_IMAGE_START                0x010000UL
 #define LINUX_MAGIC_ADDR                0x010008UL
+#define KERN_PARM_AREA_SIZE_ADDR        0x010430UL
 #define KERN_PARM_AREA                  0x010480UL
-#define KERN_PARM_AREA_SIZE             0x000380UL
+#define LEGACY_KERN_PARM_AREA_SIZE      0x000380UL
 #define INITRD_START                    0x800000UL
 #define INITRD_PARM_START               0x010408UL
 #define PARMFILE_START                  0x001000UL
@@ -110,6 +111,21 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
     return srcaddr + dstaddr;
 }
 
+static uint64_t get_max_kernel_cmdline_size(void)
+{
+    uint64_t *size_ptr = rom_ptr(KERN_PARM_AREA_SIZE_ADDR, sizeof(*size_ptr));
+
+    if (size_ptr) {
+        uint64_t size;
+
+        size = be64_to_cpu(*size_ptr);
+        if (size != 0) {
+            return size;
+        }
+    }
+    return LEGACY_KERN_PARM_AREA_SIZE;
+}
+
 static void s390_ipl_realize(DeviceState *dev, Error **errp)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
@@ -197,10 +213,11 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
             ipl->start_addr = KERN_IMAGE_START;
             /* Overwrite parameters in the kernel image, which are "rom" */
             if (parm_area) {
-                if (cmdline_size > KERN_PARM_AREA_SIZE) {
+                uint64_t max_cmdline_size = get_max_kernel_cmdline_size();
+                if (cmdline_size > max_cmdline_size) {
                     error_setg(errp,
                                "kernel command line exceeds maximum size: %zu > %lu",
-                               cmdline_size, KERN_PARM_AREA_SIZE);
+                               cmdline_size, max_cmdline_size);
                     return;
                 }
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] s390x/ipl: support extended kernel command line size
  2021-11-22 11:29 [PATCH] s390x/ipl: support extended kernel command line size Marc Hartmayer
@ 2021-11-22 11:54 ` David Hildenbrand
  2021-11-22 15:07   ` Marc Hartmayer
  2021-11-29 17:28 ` Christian Borntraeger
  2021-12-08  9:04 ` Thomas Huth
  2 siblings, 1 reply; 5+ messages in thread
From: David Hildenbrand @ 2021-11-22 11:54 UTC (permalink / raw)
  To: Marc Hartmayer, qemu-devel
  Cc: Halil Pasic, Christian Borntraeger, qemu-s390x, Thomas Huth

On 22.11.21 12:29, Marc Hartmayer wrote:
> In the past s390 used a fixed command line length of 896 bytes. This has changed
> with the Linux commit 5ecb2da660ab ("s390: support command lines longer than 896
> bytes"). There is now a parm area indicating the maximum command line size. This
> parm area has always been initialized to zero, so with older kernels this field
> would read zero and we must then assume that only 896 bytes are available.
> 
> Acked-by: Viktor Mihajlovski <mihajlov@de.ibm.com>
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
>  hw/s390x/ipl.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 7ddca0127fc2..092c66b3f9f1 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -37,8 +37,9 @@
>  
>  #define KERN_IMAGE_START                0x010000UL
>  #define LINUX_MAGIC_ADDR                0x010008UL
> +#define KERN_PARM_AREA_SIZE_ADDR        0x010430UL
>  #define KERN_PARM_AREA                  0x010480UL
> -#define KERN_PARM_AREA_SIZE             0x000380UL
> +#define LEGACY_KERN_PARM_AREA_SIZE      0x000380UL
>  #define INITRD_START                    0x800000UL
>  #define INITRD_PARM_START               0x010408UL
>  #define PARMFILE_START                  0x001000UL
> @@ -110,6 +111,21 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>      return srcaddr + dstaddr;
>  }
>  
> +static uint64_t get_max_kernel_cmdline_size(void)
> +{
> +    uint64_t *size_ptr = rom_ptr(KERN_PARM_AREA_SIZE_ADDR, sizeof(*size_ptr));
> +
> +    if (size_ptr) {
> +        uint64_t size;
> +
> +        size = be64_to_cpu(*size_ptr);
> +        if (size != 0) {

Could do "if (size) {"

> +            return size;
> +        }
> +    }
> +    return LEGACY_KERN_PARM_AREA_SIZE;
> +}
> +
>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>  {
>      MachineState *ms = MACHINE(qdev_get_machine());
> @@ -197,10 +213,11 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>              ipl->start_addr = KERN_IMAGE_START;
>              /* Overwrite parameters in the kernel image, which are "rom" */
>              if (parm_area) {
> -                if (cmdline_size > KERN_PARM_AREA_SIZE) {
> +                uint64_t max_cmdline_size = get_max_kernel_cmdline_size();

We might want an empty line here.

> +                if (cmdline_size > max_cmdline_size) {
>                      error_setg(errp,
>                                 "kernel command line exceeds maximum size: %zu > %lu",
> -                               cmdline_size, KERN_PARM_AREA_SIZE);
> +                               cmdline_size, max_cmdline_size);
>                      return;
>                  }
>  
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] s390x/ipl: support extended kernel command line size
  2021-11-22 11:54 ` David Hildenbrand
@ 2021-11-22 15:07   ` Marc Hartmayer
  0 siblings, 0 replies; 5+ messages in thread
From: Marc Hartmayer @ 2021-11-22 15:07 UTC (permalink / raw)
  To: David Hildenbrand, qemu-devel
  Cc: Halil Pasic, Christian Borntraeger, qemu-s390x, Thomas Huth

David Hildenbrand <david@redhat.com> writes:

> On 22.11.21 12:29, Marc Hartmayer wrote:
>> In the past s390 used a fixed command line length of 896 bytes. This has changed
>> with the Linux commit 5ecb2da660ab ("s390: support command lines longer than 896
>> bytes"). There is now a parm area indicating the maximum command line size. This
>> parm area has always been initialized to zero, so with older kernels this field
>> would read zero and we must then assume that only 896 bytes are available.
>> 
>> Acked-by: Viktor Mihajlovski <mihajlov@de.ibm.com>
>> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>> ---
>>  hw/s390x/ipl.c | 23 ++++++++++++++++++++---
>>  1 file changed, 20 insertions(+), 3 deletions(-)
>> 
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 7ddca0127fc2..092c66b3f9f1 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -37,8 +37,9 @@
>>  
>>  #define KERN_IMAGE_START                0x010000UL
>>  #define LINUX_MAGIC_ADDR                0x010008UL
>> +#define KERN_PARM_AREA_SIZE_ADDR        0x010430UL
>>  #define KERN_PARM_AREA                  0x010480UL
>> -#define KERN_PARM_AREA_SIZE             0x000380UL
>> +#define LEGACY_KERN_PARM_AREA_SIZE      0x000380UL
>>  #define INITRD_START                    0x800000UL
>>  #define INITRD_PARM_START               0x010408UL
>>  #define PARMFILE_START                  0x001000UL
>> @@ -110,6 +111,21 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>>      return srcaddr + dstaddr;
>>  }
>>  
>> +static uint64_t get_max_kernel_cmdline_size(void)
>> +{
>> +    uint64_t *size_ptr = rom_ptr(KERN_PARM_AREA_SIZE_ADDR, sizeof(*size_ptr));
>> +
>> +    if (size_ptr) {
>> +        uint64_t size;
>> +
>> +        size = be64_to_cpu(*size_ptr);
>> +        if (size != 0) {
>
> Could do "if (size) {"

Ok.

>
>> +            return size;
>> +        }
>> +    }
>> +    return LEGACY_KERN_PARM_AREA_SIZE;
>> +}
>> +
>>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>  {
>>      MachineState *ms = MACHINE(qdev_get_machine());
>> @@ -197,10 +213,11 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>              ipl->start_addr = KERN_IMAGE_START;
>>              /* Overwrite parameters in the kernel image, which are "rom" */
>>              if (parm_area) {
>> -                if (cmdline_size > KERN_PARM_AREA_SIZE) {
>> +                uint64_t max_cmdline_size = get_max_kernel_cmdline_size();
>
> We might want an empty line here.

Yep.

>
>> +                if (cmdline_size > max_cmdline_size) {
>>                      error_setg(errp,
>>                                 "kernel command line exceeds maximum size: %zu > %lu",
>> -                               cmdline_size, KERN_PARM_AREA_SIZE);
>> +                               cmdline_size, max_cmdline_size);
>>                      return;
>>                  }
>>  
>> 
>
> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks for the review!

>
> -- 
> Thanks,
>
> David / dhildenb
>
-- 
Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] s390x/ipl: support extended kernel command line size
  2021-11-22 11:29 [PATCH] s390x/ipl: support extended kernel command line size Marc Hartmayer
  2021-11-22 11:54 ` David Hildenbrand
@ 2021-11-29 17:28 ` Christian Borntraeger
  2021-12-08  9:04 ` Thomas Huth
  2 siblings, 0 replies; 5+ messages in thread
From: Christian Borntraeger @ 2021-11-29 17:28 UTC (permalink / raw)
  To: Marc Hartmayer, qemu-devel
  Cc: Halil Pasic, qemu-s390x, Thomas Huth, David Hildenbrand



Am 22.11.21 um 12:29 schrieb Marc Hartmayer:
> In the past s390 used a fixed command line length of 896 bytes. This has changed
> with the Linux commit 5ecb2da660ab ("s390: support command lines longer than 896
> bytes"). There is now a parm area indicating the maximum command line size. This
> parm area has always been initialized to zero, so with older kernels this field
> would read zero and we must then assume that only 896 bytes are available.
> 
> Acked-by: Viktor Mihajlovski <mihajlov@de.ibm.com>
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>

Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

> ---
>   hw/s390x/ipl.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 7ddca0127fc2..092c66b3f9f1 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -37,8 +37,9 @@
>   
>   #define KERN_IMAGE_START                0x010000UL
>   #define LINUX_MAGIC_ADDR                0x010008UL
> +#define KERN_PARM_AREA_SIZE_ADDR        0x010430UL
>   #define KERN_PARM_AREA                  0x010480UL
> -#define KERN_PARM_AREA_SIZE             0x000380UL
> +#define LEGACY_KERN_PARM_AREA_SIZE      0x000380UL
>   #define INITRD_START                    0x800000UL
>   #define INITRD_PARM_START               0x010408UL
>   #define PARMFILE_START                  0x001000UL
> @@ -110,6 +111,21 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>       return srcaddr + dstaddr;
>   }
>   
> +static uint64_t get_max_kernel_cmdline_size(void)
> +{
> +    uint64_t *size_ptr = rom_ptr(KERN_PARM_AREA_SIZE_ADDR, sizeof(*size_ptr));
> +
> +    if (size_ptr) {
> +        uint64_t size;
> +
> +        size = be64_to_cpu(*size_ptr);
> +        if (size != 0) {
> +            return size;
> +        }
> +    }
> +    return LEGACY_KERN_PARM_AREA_SIZE;
> +}
> +
>   static void s390_ipl_realize(DeviceState *dev, Error **errp)
>   {
>       MachineState *ms = MACHINE(qdev_get_machine());
> @@ -197,10 +213,11 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>               ipl->start_addr = KERN_IMAGE_START;
>               /* Overwrite parameters in the kernel image, which are "rom" */
>               if (parm_area) {
> -                if (cmdline_size > KERN_PARM_AREA_SIZE) {
> +                uint64_t max_cmdline_size = get_max_kernel_cmdline_size();
> +                if (cmdline_size > max_cmdline_size) {
>                       error_setg(errp,
>                                  "kernel command line exceeds maximum size: %zu > %lu",
> -                               cmdline_size, KERN_PARM_AREA_SIZE);
> +                               cmdline_size, max_cmdline_size);
>                       return;
>                   }
>   
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] s390x/ipl: support extended kernel command line size
  2021-11-22 11:29 [PATCH] s390x/ipl: support extended kernel command line size Marc Hartmayer
  2021-11-22 11:54 ` David Hildenbrand
  2021-11-29 17:28 ` Christian Borntraeger
@ 2021-12-08  9:04 ` Thomas Huth
  2 siblings, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2021-12-08  9:04 UTC (permalink / raw)
  To: Marc Hartmayer, qemu-devel
  Cc: Halil Pasic, Christian Borntraeger, qemu-s390x, David Hildenbrand

On 22/11/2021 12.29, Marc Hartmayer wrote:
> In the past s390 used a fixed command line length of 896 bytes. This has changed
> with the Linux commit 5ecb2da660ab ("s390: support command lines longer than 896
> bytes"). There is now a parm area indicating the maximum command line size. This
> parm area has always been initialized to zero, so with older kernels this field
> would read zero and we must then assume that only 896 bytes are available.
> 
> Acked-by: Viktor Mihajlovski <mihajlov@de.ibm.com>
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
>   hw/s390x/ipl.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 7ddca0127fc2..092c66b3f9f1 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -37,8 +37,9 @@
>   
>   #define KERN_IMAGE_START                0x010000UL
>   #define LINUX_MAGIC_ADDR                0x010008UL
> +#define KERN_PARM_AREA_SIZE_ADDR        0x010430UL
>   #define KERN_PARM_AREA                  0x010480UL
> -#define KERN_PARM_AREA_SIZE             0x000380UL
> +#define LEGACY_KERN_PARM_AREA_SIZE      0x000380UL
>   #define INITRD_START                    0x800000UL
>   #define INITRD_PARM_START               0x010408UL
>   #define PARMFILE_START                  0x001000UL
> @@ -110,6 +111,21 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>       return srcaddr + dstaddr;
>   }
>   
> +static uint64_t get_max_kernel_cmdline_size(void)
> +{
> +    uint64_t *size_ptr = rom_ptr(KERN_PARM_AREA_SIZE_ADDR, sizeof(*size_ptr));
> +
> +    if (size_ptr) {
> +        uint64_t size;
> +
> +        size = be64_to_cpu(*size_ptr);
> +        if (size != 0) {
> +            return size;
> +        }
> +    }
> +    return LEGACY_KERN_PARM_AREA_SIZE;
> +}
> +
>   static void s390_ipl_realize(DeviceState *dev, Error **errp)
>   {
>       MachineState *ms = MACHINE(qdev_get_machine());
> @@ -197,10 +213,11 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>               ipl->start_addr = KERN_IMAGE_START;
>               /* Overwrite parameters in the kernel image, which are "rom" */
>               if (parm_area) {
> -                if (cmdline_size > KERN_PARM_AREA_SIZE) {
> +                uint64_t max_cmdline_size = get_max_kernel_cmdline_size();
> +                if (cmdline_size > max_cmdline_size) {
>                       error_setg(errp,
>                                  "kernel command line exceeds maximum size: %zu > %lu",

I think the %lu likely needs to be turned into a PRIu64 now ... could you 
please try compiling on a 32-bit host env (e.g. via gitlab-ci), fix the 
patch accordingly (also with the cosmetics mentioned by David) and send a 
v2? Thanks!

  Thomas

> -                               cmdline_size, KERN_PARM_AREA_SIZE);
> +                               cmdline_size, max_cmdline_size);
>                       return;
>                   }



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-08  9:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22 11:29 [PATCH] s390x/ipl: support extended kernel command line size Marc Hartmayer
2021-11-22 11:54 ` David Hildenbrand
2021-11-22 15:07   ` Marc Hartmayer
2021-11-29 17:28 ` Christian Borntraeger
2021-12-08  9:04 ` Thomas Huth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.