Re: [PATCH RFCv3 6/9] s390x/diag: subcode to query device memory region

[Cornelia Huck]

On Mon, 27 Jul 2020 11:52:30 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 27.07.20 11:48, Cornelia Huck wrote:
> > On Fri, 24 Jul 2020 16:37:47 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >   
> >> A guest OS that is aware of memory devices (placed into the device
> >> memory region located in guest physical address space) has to know at least
> >> the end address of the device memory region during boot, for example, to
> >> prepare the kernel virtual address space accordingly (e.g., select page
> >> table hierarchy). The device memory region is located above the SCLP
> >> maximum storage increment.
> >>
> >> Let's provide a new diag500 subcode to query the location of the device
> >> memory region under QEMU/KVM. This way, esp. Linux who's wants to support
> >> virtio-based memory devices can query the location of this region and
> >> derive the maximum possible PFN.
> >>
> >> Let's use a specification exception in case no such memory region
> >> exists (e.g., maxmem wasn't specified, or on old QEMU machines). We'll
> >> unlock this with future patches that prepare and instanciate the device
> >> memory region.  
> > 
> > Specification exception on old machines seems reasonable. But maybe
> > newer machines can use a different return value for "no memory regions"?  
> 
> Hm, I don't see any benefit to distinguish the two cases of "no device
> memory region". Should the guest really care?

No idea, was just a random thought.

> 
> [...]
> > 
> > (...)
> >   
> >> diff --git a/hw/s390x/s390-hypercall.h b/hw/s390x/s390-hypercall.h
> >> index e6b958db41..1b179d7d99 100644
> >> --- a/hw/s390x/s390-hypercall.h
> >> +++ b/hw/s390x/s390-hypercall.h
> >> @@ -16,6 +16,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_DEVICE_MEMORY_REGION   4  
> > 
> > Regardless what we end up with, this needs to be specified
> > somewhere(tm).
> >   
> 
> Yeah, there, we should also document the existing subcodes. What would
> be the right place for this? The kernel feels somewhat wrong to me.

The still supported subcode 3 is properly specified in the virtio spec.
That's not a good place for that new one, though.

QEMU is probably a better place than the kernel to specify stuff,
although it's not really ideal, either. OTOH, do we ever expect other
hypervisors to implement this new subcode?

So maybe under doc/specs/?