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

[Tony Krowiak]

On 06/11/2018 07:49 AM, Janosch Frank wrote:
> On 11.06.2018 13:32, 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.
> Yes, please!
>
> > > 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).
> Please do not use the kvm_lock if possible.
>
> > 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?
> At least I do.
>
> KVM does not care about who has which crypto queue/card.
> I'd like to have a driver that does internal bookkeeping and then
> registers the crycb with KVM, so the VM can use it.

I am not sure what you mean by "registers the crycb with KVM".
Can you provide more detail?

> > Regards,
> > Halil