[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 6/7] s390x/kvm: handle AP instruction interce

From: Halil Pasic
Subject: Re: [Qemu-devel] [PATCH v3 6/7] s390x/kvm: handle AP instruction interception
Date: Fri, 6 Apr 2018 14:09:24 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 04/06/2018 11:11 AM, David Hildenbrand wrote:
> On 06.04.2018 10:40, Cornelia Huck wrote:
>> On Thu, 5 Apr 2018 19:17:47 +0200
>> Halil Pasic <address@hidden> wrote:
>>> On 04/05/2018 06:38 PM, Tony Krowiak wrote:
>>>>> Hard to really give good advice without access to the documentation, but:
>>>>> - If we tell the guest that the feature is available, but it does not
>>>>>    get any cards to use, returning an empty matrix makes the most sense
>>>>>    to me.
>>>>> - I would not tie starting the guest to the presence of a vfio-ap
>>>>>    device. Having the feature available in theory but without any
>>>>>    devices actually being usable by the guest does not really sound
>>>>>    wrong (can we hotplug this later?)  
>>>> For this phase of development, it is my opinion that introducing AP 
>>>> instruction
>>>> interception handlers is superfluous for the following reasons:
>>>> 1. Interception handling was introduced solely to ensure an operation 
>>>> exception would
>>>>    not be injected into the guest when CPU model feature for AP (i.e., 
>>>> ap=on)
>>>>    is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path)
>>>>    is not.
>>>> 2. The implementation of guest dedicated crypto adapters uses AP 
>>>> instruction
>>>>    interpretation to virtualize AP devices for a guest. As such, the NQAP,
>>>>    DQAP and most variants of the PQAP instructions will not be
>>>>    intercepted.
>>>> 3. Hot plugging AP devices is not being supported for this phase of 
>>>> development.
>>>> It is my opinion that introducing these interception handlers at this time 
>>>> is
>>>> unnecessary and risks opening a can of worms that would be
>>>> better dealt with in a subsequent phase. For that reason and the reasons 
>>>> stated
>>>> above, I think the better option is to terminate starting the guest if the
>>>> CPU model feature for AP is enabled but an AP device is not defined for the
>>>> guest. This restriction, of course, will be removed when hot plug is 
>>>> implemented
>>>> in a subsequent development phase.  
>>> I second that! I agree that having ap instructions but not having the
>>> possibility to actually do AP crypto is probably not what the user wants.
>>> Preventing such a guest form starting (with a nice message) sounds 
>>> reasonable
>>> to me.
>> One problem I have with that is that it feels backwards to me.
>> The situation "you cannot add this device unless $FEATURE is present"
>> is quite common and thus easily understood. Now, this would introduce
>> the situation "you cannot present $FEATURE unless this device is also
>> present, and that right at the start". I'm not sure how you are
>> supposed to correlate a cpu feature with the existence of a device.

I think it can be done straightforward and with less LOC than interception
and emulation of 'nothing to see here' requires.

> I agree. Don't make things harder than they are. This smells like "cpu
> feature can only be provided if another magical QEMU command line option
> is present". Don't do that.

Yes it is conceptually ugly. I'm 100% with you. That's why it should go
away soon. From the practicality perspective however I would even argue that 
helpful to the user: tells 'oops you have forgotten something'. IMHO
it's a shortcut of type make the problem smaller. Regarding what is
harder and what is easier: the author is probably the most fit to decide
that. If it is harder, it makes no sense, as this is all about cutting

> Is it really that hard to implement a very simple interception handler
> that says t all instructions "yeah, I'm alive, but no, nothing to see here".

I find it somewhat difficult to reason about what is static and what is
dynamic in the AP architecture. To put something together that seems to
work should be relatively easy. I could even say, I hope Tony tested the
no device case with v3 and it apparently seemed to work -- as I don't see
any does not work disclaimer. But getting all the stuff correct is IMHO
a bit more of a challenge.

>>> I agree with Connie, the approach 'hold the line' (until future hotplugs)
>>> is the most reasonable thing to do *in the long run*. But I think it's 
>>> better
>>> to limit ourselves to the simplest case for now, I don't see any problems
>>> with doing the hotplug support later.
>> Yes, having to add handlers that add very little benefit sucks, I
>> agree. But I fear if we add the "feature needs device" dependency, we
>> open another can of worms, including the question what happens if we
>> want to support hotplug in the future (I'm not altogether sure how to
>> handle the whole checking from qemu).

We do want hotplug in the future AFAIK. The idea was to just remove the
limitation when everything is in place.

Regarding the implementation, the idea was to use 
qemu_add_machine_init_done_notifier and  only catch both of the following true
* the cpu model has ap=on
* on the ap bus (I think it would be nice to have a bus with max_dev = 1)
  there is no device

When hotplug becomes available for vfio-ap we would just remove that code.
For me it seemed legit. We have a precedence for not really complete stuff
with vfio-ccw. So if it was OK to defer the stuff that was deferred there,
I think, ap=on suddenly working without the strange device should be OK too.

>> Making sure that we have both the feature and the device when using a
>> management software (e.g. libvirt) makes a lot of sense (and is
>> probably also easier to implement), but it won't help us with the issue
>> of the interception handlers, unfortunately.

I disagree. Conceptually hotplug of vfio-ap is perfectly legit. So doing
something like this in management software sounds wrong. The idea here
is cutting corners in order to have something that works reasonably well
but with a couple of well defined limitations sooner.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]