Re: [PATCH RFCv3 4/9] s390x: prepare for more diag500 hypercalls

[Cornelia Huck <cohuck@redhat.com>]

On Fri, 24 Jul 2020 16:37:45 +0200
David Hildenbrand <david@redhat.com> wrote:

> Let's generalize, abstacting the virtio bits. diag500 is now a generic
> hypercall to handle QEMU/KVM specific things. Explicitly specify all
> already defined subcodes, including legacy ones (so we know what we can
> use for new hypercalls). While at it, move exception handling into the
> handler.

IIRC, diag 500 had been reserved as "KVM stuff" and not just "virtio
stuff", so that should be fine.

The kernel documentation explicitly talks about "KVM virtio functions",
though; you may want to tweak this (and also add a reference to any new
subcodes.)

[Do we have a good resting place for documenting non-virtio-specific
subcodes?]

> 
> We'll rename the files separately, so git properly detects the rename.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-virtio-hcall.c | 14 +++++++-------
>  hw/s390x/s390-virtio-hcall.h | 12 ++++++------
>  target/s390x/kvm.c           | 15 +++------------
>  target/s390x/misc_helper.c   |  3 ++-
>  4 files changed, 18 insertions(+), 26 deletions(-)
>

(...)

> diff --git a/hw/s390x/s390-virtio-hcall.h b/hw/s390x/s390-virtio-hcall.h
> index 67e11ea39a..2214216ce8 100644
> --- a/hw/s390x/s390-virtio-hcall.h
> +++ b/hw/s390x/s390-virtio-hcall.h
> @@ -1,5 +1,5 @@
>  /*
> - * Support for virtio hypercalls on s390x
> + * Support for QEMU/KVM-specific hypercalls on s390
>   *
>   * Copyright IBM Corp. 2012, 2017
>   * Author(s): Cornelia Huck <cornelia.huck@de.ibm.com>
> @@ -12,10 +12,10 @@
>  #ifndef HW_S390_VIRTIO_HCALL_H
>  #define HW_S390_VIRTIO_HCALL_H
>  
> -#include "standard-headers/asm-s390/virtio-ccw.h"
> +#define DIAG500_VIRTIO_NOTIFY          0 /* legacy, implemented as a NOP */
> +#define DIAG500_VIRTIO_RESET           1 /* legacy */
> +#define DIAG500_VIRTIO_SET_STATUS      2 /* legacy */
> +#define DIAG500_VIRTIO_CCW_NOTIFY      3 /* KVM_S390_VIRTIO_CCW_NOTIFY */
>  
> -/* The only thing that we need from the old kvm_virtio.h file */
> -#define KVM_S390_VIRTIO_NOTIFY 0

It feels a bit odd to define it here; but this is host/guest api and
won't change anyway.

> -
> -int s390_virtio_hypercall(CPUS390XState *env);
> +void handle_diag_500(CPUS390XState *env, uintptr_t ra);
>  #endif /* HW_S390_VIRTIO_HCALL_H */

(...)