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

[Tony Krowiak <akrowiak@linux.ibm.com>]

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
>>
>