Re: [Qemu-devel] [PATCH v6 1/3] IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility exposoed to guest

[Aviv B.D.]

On Tue, Nov 22, 2016 at 6:11 AM, Jason Wang <address@hidden> wrote:

>
>
> On 2016年11月21日 21:35, Michael S. Tsirkin wrote:
>
>> On Fri, Nov 11, 2016 at 12:15:48PM +0800, Jason Wang wrote:
>>
>>> >
>>> >
>>> >On 2016年11月11日 11:39, Michael S. Tsirkin wrote:
>>>
>>>> > >On Fri, Nov 11, 2016 at 10:32:42AM +0800, Jason Wang wrote:
>>>>
>>>>> > > >
>>>>> > > >On 2016年11月10日 06:00, Michael S. Tsirkin wrote:
>>>>>
>>>>>> > > > >On Wed, Nov 09, 2016 at 03:28:02PM +0800, Jason Wang wrote:
>>>>>>
>>>>>>> > > > > > >
>>>>>>>> > > > > > >On 2016年11月08日 19:04, Aviv B.D wrote:
>>>>>>>>
>>>>>>>>> > > > > > > > >From: "Aviv Ben-David"<address@hidden>
>>>>>>>>>> > > > > > > > >
>>>>>>>>>> > > > > > > > >This capability asks the guest to invalidate cache
>>>>>>>>>> before each map operation.
>>>>>>>>>> > > > > > > > >We can use this invalidation to trap map
>>>>>>>>>> operations in the hypervisor.
>>>>>>>>>>
>>>>>>>>> > > > > > >Hi:
>>>>>>>> > > > > > >
>>>>>>>> > > > > > >Like I've asked twice in the past, I want to know why
>>>>>>>> don't you cache
>>>>>>>> > > > > > >translation faults as what spec required (especially
>>>>>>>> this is a guest visible
>>>>>>>> > > > > > >behavior)?
>>>>>>>> > > > > > >
>>>>>>>> > > > > > >Btw, please cc me on posting future versions.
>>>>>>>> > > > > > >
>>>>>>>> > > > > > >Thanks
>>>>>>>>
>>>>>>> > > > >Caching isn't guest visible.
>>>>>>
>>>>> > > >Seems not, if one fault mapping were cached by IOTLB. Guest can
>>>>> notice this
>>>>> > > >behavior.
>>>>>
>>>> > >Sorry, I don't get what you are saying.
>>>> > >
>>>>
>>>>> > > > >Spec just says you*can*  cache,
>>>>>> > > > >not that you must.
>>>>>> > > > >
>>>>>>
>>>>> > > >Yes, but what did in this patch is "don't". What I suggest is
>>>>> just a "can",
>>>>> > > >since anyway the IOTLB entries were limited and could be replaced
>>>>> by other.
>>>>> > > >
>>>>> > > >Thanks
>>>>>
>>>> > >Have trouble understanding this. Can you given an example of
>>>> > >a guest visible difference?
>>>>
>>> >
>>> >I guess this may do the detection:
>>> >
>>> >1) map iova A to be non-present.
>>> >2) invalidate iova A
>>> >3) map iova A to addr B
>>> >4) access iova A
>>> >
>>> >A correct implemented CM may meet fault in step 4, but with this patch,
>>> we
>>> >never.
>>>
>> I think that IOTLB is free to invalidate entries at any point,
>> so the fault is not guaranteed on bare metal.
>>
>
> Yes, that's the interesting point. The fault is not guaranteed but
> conditional. And we have similar issue for IEC.
>
> So in conclusion (since I can't find a hardware IOMMU that have CM set):
>
> The spec says that there is no hardware IOMMU with CM bit set. This bit
exists only to enable efficient emulation of iommu.


> 1) If we don't cache fault conditions, we are still in question that
> whether it was spec compatible.
> 2) If we do cache fault conditions, we are 100% sure it was spec
> compatible.
>
Consider 2) is not complicated, we'd better do it I believe?
>
> "For implementations reporting Caching Mode (CM) as 1 in the Capability
Register, above conditions may cause caching of the entry that resulted in
the fault, and require explicit invalidation by software to invalidate such
cached entries." (
http://www.intel.com/content/dam/www/public/us/en/documents/product-specifications/vt-directed-io-spec.pdf,,
section 6.2.2, page 82). The key word is "may", therefore I do not think
that actually faulting the guest is necessary in this case.

However, if you insist on this part I will add the relevant code...

Thanks
>

Aviv.