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

[David Hildenbrand]

On 27.09.24 20:05, Halil Pasic wrote:
> 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.

I assume you skimmed the QEMU patches, including the one that suggest 
changing (or rather extending) that.

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

I'll document wherever people fancy. I'll even document in multiple 
locations :)

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

I don't see a reason to do that, but in the end I don't care as long as 
people let me know what to do instead.

Thanks for taking a look!

--
Cheers,

David / dhildenb