Re: [PATCH v5 06/13] KVM: s390: interfaces to manage guest's AP matrix

[Tony Krowiak]

On 05/16/2018 09:15 AM, Pierre Morel wrote:
> On 16/05/2018 15:12, Tony Krowiak wrote:
> > On 05/16/2018 03:48 AM, Pierre Morel wrote:
> > > On 15/05/2018 18:07, Tony Krowiak wrote:
> > > > On 05/15/2018 10:55 AM, Pierre Morel wrote:
> > > > > On 07/05/2018 17:11, Tony Krowiak wrote:
> > > > > > Provides interfaces to manage the AP adapters, usage domains
> > > > > > and control domains assigned to a KVM guest.
> > > > > >
> > > > > > The guest's SIE state description has a satellite structure 
> > > > > > called the
> > > > > > Crypto Control Block (CRYCB) containing three bitmask fields
> > > > > > identifying the adapters, queues (domains) and control domains
> > > > > > assigned to the KVM guest:
> > > ...snip...
> > > > > > +}
> > > > >
> > > > > This function (ap_validate_queue_sharing) only verifies that VM 
> > > > > don't share queues.
> > > > > What about the queues used by a host application?
> > > >
> > > > How can that be verified from this function? I suppose I could put 
> > > > a check in here to
> > > > verify that the queues are reserved by the vfio_ap device driver, 
> > > > but that would
> > > > be redundant because an AP queue can not be assigned to a mediated 
> > > > matrix device
> > > > via its sysfs attributes unless it is reserved by the vfio_ap 
> > > > device driver (see
> > > > patches 7, 8 and 9).
> > > >
> > > > > I understand that you want to implement  these checks within KVM 
> > > > > but this is
> > > > > related to which queue devices are bound to the matrix and which 
> > > > > one are not.
> > > >
> > > > See my comments above and below about AP queue assignment to the 
> > > > mediated matrix
> > > > device. The one verification we can't do when the devices are 
> > > > assigned is whether
> > > > another guest is using the queue because assignment occurs before 
> > > > the guest using
> > > > the queue is started in which case we have no access to KVM. It 
> > > > makes no sense to
> > > > do so at assignment time anyway because it doesn't matter until the 
> > > > guest using
> > > > the mediated matrix device is started, so that check is done in KVM.
> > > >
> > > > > I think that this should be related somehow to the bounded queue 
> > > > > devices and
> > > > > therefor implemented inside the matrix driver.
> > > >
> > > > As I stated above, when an AP queue is assigned to the mediated 
> > > > matrix device via
> > > > its sysfs attributes, a check is done to verify that it is bound to 
> > > > the vfio_ap
> > > > device driver (see patches 7, 8 and 9). If not, then assignment 
> > > > will be rejected;
> > > > therefore, it will not be possible to configure a CRYCB with AP 
> > > > queues that are
> > > > not bound to the device driver.
> > >
> > > This patch and te followed patches take care that the queues are 
> > > bound to the
> > > matrix driver when they are assigned to the matrix using the sysfs 
> > > entries.
> > >
> > > But they do not take care that the queue can not be unbound before 
> > > you start
> > > the guest, and they are not in the path if the admin decide to 
> > > unbind a queue
> > > at some later time.
> >
> > That is a good point. I need to put a check in the device driver at 
> > the time
> > the mediated device fd is opened to verify that the queues being 
> > configured in
> > the guest's CRYCB are bound to the driver.
>
> not only, you also need to avoid the possibility of unbinding the device.
> For this you need to use the remove callback from the driver.

I thought I addressed this already. The definition of the remove 
callback does
not specify a return value, so there is currently no way to prevent the 
AP bus
from removing the queue device on unbind. I sent an email to Harald to 
discuss
adding a return value to the callback.

> > > > > Regards,
> > > > >
> > > > > Pierre