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

[Cornelia Huck]

On Wed,  1 Apr 2020 04:50:14 -0400
Christian Borntraeger <address@hidden> wrote:

> Older QEMU versions did fixup the ram size to match what can be reported
> via sclp. We need to mimic this behaviour for machine types 4.2 and
> older to not fail on inbound migration for memory sizes that do not fit.
> Old machines with proper aligned memory sizes are not affected.
> 
> Alignment table:
>  VM size (<=) | Alignment
> --------------------------
>       1020M   |     1M
>       2040M   |     2M
>       4080M   |     4M
>       8160M   |     8M
>      16320M   |    16M
>      32640M   |    32M
>      65280M   |    64M
>     130560M   |   128M
>     261120M   |   256M
>     522240M   |   512M
>    1044480M   |     1G
>    2088960M   |     2G
>    4177920M   |     4G
>    8355840M   |     8G
> 
> Suggested action is to replace unaligned -m value with a suitable

"to replace any unaligned -m value" ?

> aligned one or to use a machine version >= 5.0 as future versions might
> remove the compatibility handling.

I'm confused by the second part of the sentence. Warning about possible
future removal of the compat stuff is fine, but I don't understand the
suggestion to use a machine type >= 5.0. If I create a new machine that
does not need be migrated to an old QEMU, using the latest machine type
always seems like the best idea, right? And for a migration target it's
not like we can choose the version freely anyway.

> 
> For machine types >= 5.0 we can simply use an increment size of 1M and
> use the full range of increment number which allows for all possible
> memory sizes. The old limitation of having a maximum of 1020 increments
> was added for standby memory, which we no longer support. With that we
> can now support even weird memory sizes like 10001234 MB.
> 
> 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-skeys.c        |  2 +-
>  hw/s390x/s390-stattrib-kvm.c |  4 ++--
>  hw/s390x/s390-virtio-ccw.c   | 19 +++++++++++++++++++
>  hw/s390x/sclp.c              | 19 ++++++-------------
>  include/hw/boards.h          |  7 +++++++
>  softmmu/vl.c                 |  3 +++
>  6 files changed, 38 insertions(+), 16 deletions(-)
> 

> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 236d239c19..0532143327 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -152,6 +152,12 @@ typedef struct {
>   *    It also will be used as a way to optin into "-m" option support.
>   *    If it's not set by board, '-m' will be ignored and generic code will
>   *    not create default RAM MemoryRegion.
> + * @fixup_ram_size:
> + *    amends user provided ram size (with -m option) using machine
> + *    specific algorithm. to be used by old machine types for compat
> + *    purposes only.
> + *    Applies only to default memory backend, i.e. explicit memory backend
> + *    wasn't used.

"Applies only to the default memory backend, i.e., an explicitly
specified memory backend will not be affected."

?

>   */
>  struct MachineClass {
>      /*< private >*/