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

[David Hildenbrand]

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

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 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).
> 
> 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 agree.


-- 

Thanks,

David / dhildenb