linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Tony Krowiak <akrowiak@linux.ibm.com>,
	Tony Krowiak <akrowiak@linux.vnet.ibm.com>,
	linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Cc: freude@de.ibm.com, schwidefsky@de.ibm.com,
	heiko.carstens@de.ibm.com, borntraeger@de.ibm.com,
	cohuck@redhat.com, kwankhede@nvidia.com,
	bjsdjshi@linux.vnet.ibm.com, pbonzini@redhat.com,
	alex.williamson@redhat.com, pmorel@linux.vnet.ibm.com,
	alifm@linux.vnet.ibm.com, mjrosato@linux.vnet.ibm.com,
	jjherne@linux.vnet.ibm.com, thuth@redhat.com,
	pasic@linux.vnet.ibm.com, berrange@redhat.com,
	fiuczy@linux.vnet.ibm.com, buendgen@de.ibm.com,
	frankja@linux.ibm.com
Subject: Re: [PATCH v10 11/26] s390: vfio-ap: implement mediated device open callback
Date: Mon, 24 Sep 2018 21:55:04 +0200	[thread overview]
Message-ID: <a59965e3-0624-65e7-f1dd-8656219dcbe2@redhat.com> (raw)
In-Reply-To: <df269178-b486-b091-4da4-731fc3f77e85@linux.ibm.com>

On 24/09/2018 21:46, Tony Krowiak wrote:
> On 09/24/2018 02:40 PM, David Hildenbrand wrote:
>> On 24/09/2018 18:07, Tony Krowiak wrote:
>>> On 09/24/2018 04:40 AM, David Hildenbrand wrote:
>>>>
>>>>>     /**
>>>>> - * Verify that the AP instructions are available on the guest. This is
>>>>> indicated
>>>>> - * via the  KVM_S390_VM_CPU_FEAT_AP CPU model feature.
>>>>> + * Verify that the AP instructions are being interpreted by firmware
>>>>> for the
>>>>> + * guest. This is indicated by the kvm->arch.crypto.apie flag.
>>>>>      */
>>>>>     static int kvm_ap_validate_crypto_setup(struct kvm *kvm)
>>>>>     {
>>>>> -	if (test_bit_inv(KVM_S390_VM_CPU_FEAT_AP, kvm->arch.cpu_feat))
>>>>> +	if (kvm->arch.crypto.apie)
>>>>>     		return 0;
>>>>
>>>> I wonder if this check makes sense, because apie can be toggled during
>>>> runtime. I guess it would be sufficient to check if the ap control block
>>>> is available and apie is supported by the HW.
>>>
>>> I am not clear about what you are getting at here, but I'll attempt
>>> to respond. There is no need to check if the AP control block (CRYCB)
>>> is available as the address is set in the CRYCBD three instructions
>>> above, even if AP instructions are not available. Regarding whether apie
>>> is supported by the hardware, the value of vcpu->kvm->arch.crypto.apie
>>> can not be set unless it is supported by the HW. In the patch (24/26)
>>> that provides the VM attributes to toggle this value, it can only be
>>> turned on if the AP instructions are available. I might also note that
>>> the kvm_ap_validate_crypto_setup() function is called whenever one of
>>> the VM crypto attributes is changed, so it makes sense that decisions
>>> made in this function are based on a change to a VM crypto attribute. In
>>> my first pass at changing this function, I checked
>>> ap_instructions_available() here, but after considering all of the
>>> above, it made sense to me to check the apie flag.
>>>
>>
>> I prefer ap_instructions_available(). As I said, kvm->arch.crypto.apie
>> is a moving target.
> 
> Looking at this again, I think I responded before my brain shifted from
> digesting comments about patch 24/26 (enable/disable APIE) to the
> context for your comment here; namely, the device open callback. My
> comment above makes no sense in this context. From the perspective of
> the vfio_ap device driver, there is one requirement that must be met in
> order to provide pass-through functionality: The AP instructions must be
> must be interpreted by the HW (i.e., ECA.28 == 1). Checking whether AP
> instructions are available does not tell us whether they are being
> interpreted by HW. Checking whether the AP control block (i.e., CRYCB)
> is available, even when combined with the instruction availability
> check, does not provide any more insight into the value of ECA.28
> becuase the CRYCB will be provided if the MSAX3 facility is installed
> (STFLE.76) for the guest regardless of whether AP instructions are 
> available or not. There is no doubt that if the AP instructions are
> not available, then the mdev open callback should fail, but it doesn't
> tell the whole story.
> 
> I realize that our CPU model protects against configuring a vfio-ap
> device for the guest if ap=off, but this function knows nothing about
> userspace. I can make a similar argument that kvm->arch.crypto.apie
> will be switched on only if ap=on but again, that is userspace
> configuration.
> 
> Having said all of the above, maybe it doesn't really matter whether
> AP instructions are being interpreted or not. If ECA.28 == 0, then
> the AP masks may very well be ignored since all AP instructions will
> be intercepted; so, maybe checking AP instruction availability is all
> that is needed. I will verify this and if I'm correct, I'll make the
> change you suggested.

Yes, that was exactly what I had in mind - we just have to make sure
that the ap control block exists, so we can set the right mask bits. If
QEMU asks for an intercept, it shall get an intercept.

But please proceed with whatever you think is best!


-- 

Thanks,

David / dhildenb

  reply	other threads:[~2018-09-24 19:55 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-12 19:42 [PATCH v10 00/26] guest dedicated crypto adapters Tony Krowiak
2018-09-12 19:42 ` [PATCH v10 01/26] KVM: s390: vsie: simulate VCPU SIE entry/exit Tony Krowiak
2018-09-24 10:32   ` Christian Borntraeger
2018-09-24 16:53     ` Tony Krowiak
2018-09-12 19:42 ` [PATCH v10 02/26] KVM: s390: introduce and use KVM_REQ_VSIE_RESTART Tony Krowiak
2018-09-24 10:49   ` Christian Borntraeger
2018-09-24 16:48     ` Tony Krowiak
2018-09-12 19:42 ` [PATCH v10 03/26] KVM: s390: refactor crypto initialization Tony Krowiak
2018-09-21 23:18   ` Tony Krowiak
2018-09-24  8:35     ` David Hildenbrand
2018-09-24 10:34     ` Cornelia Huck
2018-09-12 19:42 ` [PATCH v10 04/26] s390: vfio-ap: base implementation of VFIO AP device driver Tony Krowiak
2018-09-20 15:31   ` Cornelia Huck
2018-09-20 15:53     ` Tony Krowiak
2018-09-12 19:42 ` [PATCH v10 05/26] s390: vfio-ap: register matrix device with VFIO mdev framework Tony Krowiak
2018-09-20 15:50   ` Cornelia Huck
2018-09-20 20:35     ` Tony Krowiak
2018-09-12 19:42 ` [PATCH v10 06/26] s390: vfio-ap: sysfs interfaces to configure adapters Tony Krowiak
2018-09-21  9:40   ` Cornelia Huck
2018-09-21  9:52     ` Harald Freudenberger
2018-09-21 14:07     ` Tony Krowiak
2018-09-12 19:42 ` [PATCH v10 07/26] s390: vfio-ap: sysfs interfaces to configure domains Tony Krowiak
2018-09-24 10:45   ` Cornelia Huck
2018-09-12 19:42 ` [PATCH v10 08/26] s390: vfio-ap: sysfs interfaces to configure control domains Tony Krowiak
2018-09-24 10:57   ` Cornelia Huck
2018-09-12 19:42 ` [PATCH v10 09/26] s390: vfio-ap: sysfs interface to view matrix mdev matrix Tony Krowiak
2018-09-24 10:59   ` Cornelia Huck
2018-09-12 19:43 ` [PATCH v10 10/26] KVM: s390: interfaces to clear CRYCB masks Tony Krowiak
2018-09-24 11:01   ` Cornelia Huck
2018-09-24 11:50     ` Halil Pasic
2018-09-24 12:01       ` Cornelia Huck
2018-09-24 15:33         ` Tony Krowiak
2018-09-24 14:49     ` Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 11/26] s390: vfio-ap: implement mediated device open callback Tony Krowiak
2018-09-18 17:00   ` Halil Pasic
2018-09-18 21:57     ` Tony Krowiak
2018-09-21 23:28   ` Tony Krowiak
2018-09-24  8:40     ` David Hildenbrand
2018-09-24 16:07       ` Tony Krowiak
2018-09-24 18:40         ` David Hildenbrand
2018-09-24 18:43           ` Tony Krowiak
2018-09-24 19:46           ` Tony Krowiak
2018-09-24 19:55             ` David Hildenbrand [this message]
2018-09-25 19:54               ` Tony Krowiak
2018-09-25 19:55                 ` David Hildenbrand
2018-09-12 19:43 ` [PATCH v10 12/26] s390: vfio-ap: implement VFIO_DEVICE_GET_INFO ioctl Tony Krowiak
2018-09-24 11:43   ` Cornelia Huck
2018-09-24 16:29     ` Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 13/26] s390: vfio-ap: zeroize the AP queues Tony Krowiak
2018-09-24 11:36   ` Cornelia Huck
2018-09-24 12:16     ` Halil Pasic
2018-09-24 12:32       ` Cornelia Huck
2018-09-24 13:22       ` Harald Freudenberger
2018-09-24 16:42         ` Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 14/26] s390: vfio-ap: implement VFIO_DEVICE_RESET ioctl Tony Krowiak
2018-09-24 11:43   ` Cornelia Huck
2018-09-12 19:43 ` [PATCH v10 15/26] KVM: s390: Clear Crypto Control Block when using vSIE Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 16/26] KVM: s390: vsie: Do the CRYCB validation first Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 17/26] KVM: s390: vsie: Make use of CRYCB FORMAT2 clear Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 18/26] KVM: s390: vsie: Allow CRYCB FORMAT-2 Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 19/26] KVM: s390: vsie: allow CRYCB FORMAT-1 Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 20/26] KVM: s390: vsie: allow CRYCB FORMAT-0 Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 21/26] KVM: s390: vsie: allow guest FORMAT-0 CRYCB on host FORMAT-1 Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 22/26] KVM: s390: vsie: allow guest FORMAT-1 CRYCB on host FORMAT-2 Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 23/26] KVM: s390: vsie: allow guest FORMAT-0 " Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 24/26] KVM: s390: device attrs to enable/disable AP interpretation Tony Krowiak
2018-09-17  8:51   ` David Hildenbrand
2018-09-21 23:40     ` Tony Krowiak
2018-09-24 11:23       ` David Hildenbrand
2018-09-24 16:25         ` Tony Krowiak
2018-09-24 18:42           ` Tony Krowiak
2018-09-24 18:51             ` David Hildenbrand
2018-09-25 13:24               ` Tony Krowiak
2018-09-25  7:32             ` David Hildenbrand
2018-09-25 13:26               ` Tony Krowiak
2018-09-24 18:46           ` David Hildenbrand
2018-09-25 13:31             ` Tony Krowiak
2018-09-12 19:43 ` [PATCH v10 25/26] KVM: s390: CPU model support for AP virtualization Tony Krowiak
2018-09-21 23:31   ` Tony Krowiak
2018-09-24  8:33     ` David Hildenbrand
2018-09-12 19:43 ` [PATCH v10 26/26] s390: doc: detailed specifications " Tony Krowiak
2018-09-24 10:10 ` [PATCH v10 00/26] guest dedicated crypto adapters Christian Borntraeger
2018-09-24 11:53   ` Cornelia Huck
2018-09-24 16:46     ` Tony Krowiak
2018-09-24 16:50   ` Tony Krowiak
2018-09-24 11:49 ` Cornelia Huck
2018-09-24 16:45   ` Tony Krowiak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a59965e3-0624-65e7-f1dd-8656219dcbe2@redhat.com \
    --to=david@redhat.com \
    --cc=akrowiak@linux.ibm.com \
    --cc=akrowiak@linux.vnet.ibm.com \
    --cc=alex.williamson@redhat.com \
    --cc=alifm@linux.vnet.ibm.com \
    --cc=berrange@redhat.com \
    --cc=bjsdjshi@linux.vnet.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=buendgen@de.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=fiuczy@linux.vnet.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=freude@de.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jjherne@linux.vnet.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mjrosato@linux.vnet.ibm.com \
    --cc=pasic@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=pmorel@linux.vnet.ibm.com \
    --cc=schwidefsky@de.ibm.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).