Re: [PATCH v5 11/13] KVM: s390: implement mediated device open callback

[Tony Krowiak]

On 06/11/2018 07:32 AM, Halil Pasic wrote:
> On 06/11/2018 11:23 AM, Pierre Morel wrote:
> > On 08/06/2018 23:59, Tony Krowiak wrote:
> > > On 06/07/2018 01:15 PM, Pierre Morel wrote:
> > > >
> >
> > ...snip...
> >
> > > > > > > Why maintain a list of kvm_ap_matrix structures if we don't have 
> > > > > > > to; it is stored
> > > > > > > with the mediated matrix device which is passed in to all of the 
> > > > > > > vfio_ap driver
> > > > > > > callbacks.
> > > > > >
> > > > > > Because using the vm_list which is a static in kvm makes you 
> > > > > > stick inside the kvm code.
> > >
> > > I understand your point here, but even if we did maintain a list of 
> > > kvm_ap_matrix structures,
> > > we still need the kvm code to configure the guest's CRYCB and 
> > > eventually ECA.28. There is
> > > also code in kvm-ap.c that is called from KVM.
> >
> > The only code from kvm-ap which is called from KVM is temporary code
> > waiting for Harald to offer the clean interface to AP instructions.
> >
> > > The idea behind kvm-ap.c is that all code
> > > related to configuration of AP structures in KVM is in this one spot.
> >
> > This I understand, but the code can be in one spot inside VFIO_AP 
> > instead
> > of inside KVM.
> > Putting the code inside KVM induce dependencies between KVM and AP
> > while the kvm/vfio interface allows to avoid this dependency.
> >
> > The purpose of VFIO_AP is to handle the CRYCB, all get/clear/set 
> > crycb masks
> > functions should be in VFIO AP.
> >
> > If we use wrappers in KVM, since the CRYCB is an a SIE extension,
> > it is legitimate, the KVM interface to the CRYCB should only
> > handle bitmaps and be unaware of the vfio_ap internal structures.
> >
> >
> > Another concern, the kvm_ap_validate_queue_sharing() should not be
> > inside KVM because it is a decision of current VFIO_AP driver
> > to not share the queues between guest of level 2.
> >
> > The Z architecture does not allow to share AP queues between
> > guests of level 1 but we could re-engineer the AP bus and the '
> > VFIO AP to offer queue sharing for guest level 2.
> >
> > This would be a new VFIO_AP driver (and an AP bus extension).
> > We should not have to change KVM for this.
>
>
> Pierre's proposal makes a lot of sense to me. We would not need to take
> the kvm_lock (which we need to traverse the vm_list safely) for the
> validation, and we could have immediate validation (which is in my 
> opinion
> better).
>
> Also your refcount (which is not a refcout) could go away. You simply
> traverse your list and check for duplicates when hooking up the mdev
> with KVM.
>
> And my opinion is if we don't have to add code to the kvm module we
> better not.
>
> @Janosch: Does core KVM share my opinion?

Okay, I'll make the change.

> Regards,
> Halil