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

[Jason Wang]

On 2016年10月22日 03:47, Michael S. Tsirkin wrote:
> On Fri, Oct 21, 2016 at 03:14:00PM +0800, Jason Wang wrote:
> > >
> > >
> > >On 2016年10月17日 23:44, 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.
> > > > >
> > > > >Signed-off-by: Aviv Ben-David<address@hidden>
> > > > >---
> > > > >   hw/i386/intel_iommu.c          | 5 +++++
> > > > >   hw/i386/intel_iommu_internal.h | 1 +
> > > > >   include/hw/i386/intel_iommu.h  | 2 ++
> > > > >   3 files changed, 8 insertions(+)
> > >
> > >As I asked in previous version, this may not be sufficient.
> > >
> > >CM requires to cache fault translations which is not implemented in this
> > >patch.
> I'm not sure why would there be a requirement to cache
> fault information. Cache can always be invalidated for
> any reason, in particular an empty cache is always OK.

s/requires/may/. But what did here is "don't". Isn't this an obvious 
violation?

Empty cache only work if we don't implement an real IOTLB but traverse 
the IO page tables each time.

> > >Guest can easily notice this kind of spec violation.
> How?

I guess this may do the detection:

1) map iova A to be non-present.
2) invalidate iova A
3) access iova A
4) map iova A to addr B
5) access iova A

A correct implemented CM may meet fault in step 5, but with this patch, 
we don't.