Re: [PATCH] s390x/ipl: support extended kernel command line size

[Thomas Huth <thuth@redhat.com>]

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;
>                   }