Re: [PATCH v1 07/14] s390x/s390-hypercall: introduce DIAG500 STORAGE_LIMIT

[Thomas Huth <thuth@redhat.com>]

On 10/09/2024 19.58, David Hildenbrand wrote:
> A guest OS that supports memory hotplug / memory devices must during
> boot be aware of the maximum possible physical memory address that it might
> have to handle at a later stage during its runtime
> 
> For example, the maximum possible memory address might be required to
> prepare the kernel virtual address space accordingly (e.g., select page
> table hierarchy depth).
> 
> On s390x there is currently no such mechanism that is compatible with
> paravirtualized memory devices, because the whole SCLP interface was
> designed around the idea of "storage increments" and "standby memory".
> Paravirtualized memory devices we want to support, such as virtio-mem, have
> no intersection with any of that, but could co-exist with them in the
> future if ever needed.
> 
> In particular, a guest OS must never detect and use device memory
> without the help of a proper device driver. Device memory must not be
> exposed in any firmware-provided memory map (SCLP or diag260 on s390x).
> For this reason, these memory devices will be places in memory *above*
> the "maximum storage increment" exposed via SCLP.
> 
> Let's provide a new diag500 subcode to query the memory limit determined in
> s390_memory_init().
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   hw/s390x/s390-hypercall.c | 3 +++
>   hw/s390x/s390-hypercall.h | 1 +
>   2 files changed, 4 insertions(+)
> 
> diff --git a/hw/s390x/s390-hypercall.c b/hw/s390x/s390-hypercall.c
> index f09e8a1d81..ac48fc0961 100644
> --- a/hw/s390x/s390-hypercall.c
> +++ b/hw/s390x/s390-hypercall.c
> @@ -68,6 +68,9 @@ int handle_diag_500(CPUS390XState *env)
>       case DIAG500_VIRTIO_CCW_NOTIFY:
>           env->regs[2] = handle_virtio_ccw_notify(env->regs[2], env->regs[3]);
>           return 0;
> +    case DIAG500_STORAGE_LIMIT:
> +        env->regs[2] = s390_get_memory_limit() - 1;
> +        return 0;
>       default:
>           return -EINVAL;
>       }
> diff --git a/hw/s390x/s390-hypercall.h b/hw/s390x/s390-hypercall.h
> index b7ac29f444..f0ca62bcbb 100644
> --- a/hw/s390x/s390-hypercall.h
> +++ b/hw/s390x/s390-hypercall.h
> @@ -18,6 +18,7 @@
>   #define DIAG500_VIRTIO_RESET            1 /* legacy */
>   #define DIAG500_VIRTIO_SET_STATUS       2 /* legacy */
>   #define DIAG500_VIRTIO_CCW_NOTIFY       3 /* KVM_S390_VIRTIO_CCW_NOTIFY */
> +#define DIAG500_STORAGE_LIMIT           4
>   
>   int handle_diag_500(CPUS390XState *env);

Reviewed-by: Thomas Huth <thuth@redhat.com>

Sounds very reasonable to me - but it would be good to get an 
Ack/Reviewed-by from IBM folks here (in case they prefer a different 
interface)... hope they'll join the discussion!

  Thomas