Re: [PATCH/RFC] vl/s390: fixup ram sizes for compat machines

[David Hildenbrand]

On 31.03.20 14:02, Christian Borntraeger wrote:
> compat machines did fixup the ram size to match what can be reported via
> sclp. We need to mimic those for machines 4.2 and older to not fail on
> inbound migration.
> 
> Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
> Reported-by: Lukáš Doktor <address@hidden>
> Cc: Igor Mammedov <address@hidden>
> Cc: Dr. David Alan Gilbert <address@hidden>
> Signed-off-by: David Hildenbrand <address@hidden>
> Signed-off-by: Christian Borntraeger <address@hidden>
> ---
>  hw/s390x/s390-virtio-ccw.c | 12 ++++++++++++
>  hw/s390x/sclp.c            | 10 ----------
>  include/hw/boards.h        |  1 +
>  softmmu/vl.c               |  3 +++
>  4 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 3cf19c99f3..748c08b157 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -579,6 +579,17 @@ static void s390_nmi(NMIState *n, int cpu_index, Error 
> **errp)
>      s390_cpu_restart(S390_CPU(cs));
>  }
>  
> +#define MAX_STORAGE_INCREMENTS                  1020
> +static ram_addr_t s390_align_ram(ram_addr_t sz)
> +{
> +    /* same logic as in sclp.c */
> +    int increment_size = 20;
> +    while ((sz >> increment_size) > MAX_STORAGE_INCREMENTS) {
> +        increment_size++;
> +    }
> +    return sz >> increment_size << increment_size;
> +}
> +
>  static void ccw_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -808,6 +819,7 @@ static void ccw_machine_4_2_instance_options(MachineState 
> *machine)
>  static void ccw_machine_4_2_class_options(MachineClass *mc)
>  {
>      ccw_machine_5_0_class_options(mc);
> +    mc->machine_align_ram = s390_align_ram;
>      compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);
>  }
>  DEFINE_CCW_MACHINE(4_2, "4.2", false);
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index d8ae207731..0a6ff2be82 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -372,16 +372,6 @@ static void sclp_memory_init(SCLPDevice *sclp)
>          increment_size++;
>      }
>      sclp->increment_size = increment_size;
> -
> -    /* The core memory area needs to be aligned with the increment size.
> -     * In effect, this can cause the user-specified memory size to be rounded
> -     * down to align with the nearest increment boundary. */
> -    initial_mem = initial_mem >> increment_size << increment_size;
> -
> -    machine->ram_size = initial_mem;
> -    machine->maxram_size = initial_mem;
> -    /* let's propagate the changed ram size into the global variable. */
> -    ram_size = initial_mem;
>  }
>  
>  static void sclp_init(Object *obj)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 236d239c19..e3574f4b5f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -218,6 +218,7 @@ struct MachineClass {
>                                                           unsigned cpu_index);
>      const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
>      int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
> +    ram_addr_t (*machine_align_ram)(ram_addr_t size);
>  };
>  
>  /**
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 1d33a28340..12b5758d12 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2601,6 +2601,9 @@ static bool set_memory_options(uint64_t *ram_slots, 
> ram_addr_t *maxram_size,
>      }
>  
>      sz = QEMU_ALIGN_UP(sz, 8192);
> +    if (mc->machine_align_ram) {
> +        sz = mc->machine_align_ram(sz);
> +    }
>      ram_size = sz;
>      if (ram_size != sz) {
>          error_report("ram size too large");
> 

1. You're missing the maxram changes from my patch.

2. Shouldn't we error out in case ram_size is not aligned in sclp.c (new
machines)?

-- 
Thanks,

David / dhildenb