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

[Halil Pasic]

On Thu, 12 Sep 2024 10:19:00 +0200
Thomas Huth <thuth@redhat.com> wrote:

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

According to Documentation/virt/kvm/s390/s390-diag.rst in the
linux source tree DIAG 500 is for kvm virtio funcions. 

Based on the commit message this storagelimit DIAG is rather loosely
tied to virtio if at all, so from that perspective DIAG may not be a
perfect fit. OTOH I don't see a better fit either. I would prefer to
have Christian's opinion on this. I have no strong opinion myself.

If we decide to go with a DIAG, I would like to see
Documentation/virt/kvm/s390/s390-diag.rst
updated accordingly.

Also if decide to go with DIAG 500, maybe leaving some space
between the codes more closely tied to virtio and between
the ones less closely tied to virito (for the unlikely case
that we end up wanting another DIAG 500 subcode for virtio) 
might make sense. I.e e could make DIAG500_STORAGE_LIMIT 
8 or 16 instead of 4. Again nothing important, just an idea.

Regards,
Halil