Re: [PATCH v2 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM if PV

[Viktor Mihajlovski <mihajlov@linux.ibm.com>]

On 6/10/20 12:24 PM, David Hildenbrand wrote:
> On 10.06.20 12:07, David Gibson wrote:
>> On Wed, Jun 10, 2020 at 09:22:45AM +0200, David Hildenbrand wrote:
>>> On 10.06.20 06:31, David Gibson wrote:
>>>> On Tue, Jun 09, 2020 at 12:44:39PM -0400, Michael S. Tsirkin wrote:
>>>>> On Tue, Jun 09, 2020 at 06:28:39PM +0200, Halil Pasic wrote:
>>>>>> On Tue, 9 Jun 2020 17:47:47 +0200
>>>>>> Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
>>>>>>
>>>>>>> On Tue, 9 Jun 2020 11:41:30 +0200
>>>>>>> Halil Pasic <pasic@linux.ibm.com> wrote:
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>> I don't know. Janosch could answer that, but he is on vacation. Adding
>>>>>>>> Claudio maybe he can answer. My understanding is, that while it might
>>>>>>>> be possible, it is ugly at best. The ability to do a transition is
>>>>>>>> indicated by a CPU model feature. Indicating the feature to the guest
>>>>>>>> and then failing the transition sounds wrong to me.
>>>>>>>
>>>>>>> I agree. If the feature is advertised, then it has to work. I don't
>>>>>>> think we even have an architected way to fail the transition for that
>>>>>>> reason.
>>>>>>>
>>>>>>> What __could__ be done is to prevent qemu from even starting if an
>>>>>>> incompatible device is specified together with PV.
>>>>>>
>>>>>> AFAIU, the "specified together with PV" is the problem here. Currently
>>>>>> we don't "specify PV" but PV is just a capability that is managed by the
>>>>>> CPU model (like so many other).
>>>>>
>>>>> So if we want to keep it user friendly, there could be
>>>>> protection property with values on/off/auto, and auto
>>>>> would poke at host capability to figure out whether
>>>>> it's supported.
>>>>>
>>>>> Both virtio and CPU would inherit from that.
>>>>
>>>> Right, that's what I have in mind for my 'host-trust-limitation'
>>>> property (a generalized version of the existing 'memory-encryption'
>>>> machine option).  My draft patches already set virtio properties
>>>> accordingly, it should be possible to set (default) cpu properties as
>>>> well.
>>>
>>> No crazy CPU model hacks please (at least speaking for the s390x).
>>
>> Uh... I'm not really sure what you have in mind here.
>>
> 
> Reading along I got the impression that we want to glue the availability
> of CPU features to other QEMU cmdline parameters (besides the
> accelerator). ("to set (default) cpu properties as well"). If we are
> talking about other CPU properties not expressed as CPU features (e.g.,
> -cpu X,Y=on ...), then there is no issue.
> 

The reason that the capability to run in PV mode is expressed in the CPU
model is that this capability *is* provided by the CPU in terms of
available instructions. I wouldn't see a benefit in providing
a meta-property that needs to be synced with the CPU model.

So, if something has to be concluded from the fact that a VM
could run in PV mode, that decision should be derived from the
CPU model.

-- 
Kind Regards,
    Viktor