Re: [RFC PATCH 05/12] s390/cio: add protected virtualization support to cio

[Halil Pasic <pasic@linux.ibm.com>]

On Wed, 10 Apr 2019 10:25:57 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 10 Apr 2019 02:10:44 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Tue, 9 Apr 2019 19:55:48 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:
> > 
> > > On Fri,  5 Apr 2019 01:16:15 +0200
> > > Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > > > Thus we need to make sure any memory that is used for communication with
> > > > the hypervisor is shared.  
> > > 
> > > In this context, does 'hypervisor' always mean 'QEMU/KVM'? If Other
> > > Hypervisors implement protected virtualization, we probably need to
> > > make sure that all common I/O layer control blocks are in the dma area
> > > (including e.g. QDIO), not just what virtio-ccw devices use.
> > >   
> > 
> > Hypervisor could theoretically be something different than QEMU/KVM. Yet,
> > as stated before, this series is about getting virtio-ccw working
> > (modulo the TODOs). 
> 
> Sure, just wanted to point it out. If this is "enable the common I/O
> layer, except for QDIO" or so, that would sound fine to me :)
> 

Right except QDIO. Series cares only about the bits relevant for
virtio-ccw. I guess I should make that clearer in the commit message.


> > 
> > [..]
> > 
> > > >    
> > > 
> > > So, this leaves some things I'm not sure about, especially as I do not
> > > know the architecture of this new feature.
> > > 
> > > - This applies only to asynchronously handled things, it seems? So
> > >   things like control blocks modified by stsch/msch/etc does not need
> > >   special treatment?  
> > 
> > I had a feeble attempt at explaining this in the cover letter:
> > 
> > * make sure that virtio-ccw specific stuff uses shared memory when
> >   talking to the hypervisor (except communication blocks like ORB, these
> >   are handled by the hypervisor)
> > 
> > Unfortunately the last 'hypervisor' was supposed to be 'ultravisor'.
> > 
> > I.e. the ultravisor will take care of exposing the control blocks
> > to the hypervisor (and of changes as well).
> 
> Yeah, that "control blocks" or "communication blocks" leaves me a bit
> fuzzy :)

Yeah. I don't know if there is an official name for the stuff. For
the ORB of SSCH control block seems quite fitting. For the SCHIB of
STSCH communication seems a better fit.

> 
> So, what is a high-level summary of areas that need the treatment?
> What I get from looking at the patches so far, it's:
> - stuff that is written by the hypervisor's interrupt injection code:
>   IRB, indicators, etc.

In interrupt context IRB behaves like a control block. We don't have
to make it shared.

I moved IRB because of snse:


@@ -329,9 +329,9 @@ ccw_device_do_sense(struct ccw_device *cdev, struct
irb *irb) /*
         * We have ending status but no sense information. Do a basic
sense. */
-       sense_ccw = &to_io_private(sch)->sense_ccw;
+       sense_ccw = &to_io_private(sch)->dma_area->sense_ccw;
        sense_ccw->cmd_code = CCW_CMD_BASIC_SENSE;
-       sense_ccw->cda = (__u32) __pa(cdev->private->irb.ecw);
+       sense_ccw->cda = (__u32) __pa(cdev->private->dma_area->irb.ecw);

as the irb.ecw is used as channel program data. And that needs to be
shared.

> - buffers that are filled by a channel program: sense, sense id, etc.
> - ccws themselves (because of translation?)
> 

Right. The idea is: smallish, basically fixed size a readily available
control blocks (specified as a ) are copied back and forth by the
ultravisor (via SIE SD).

Regards,
Halil