Re: [Qemu-devel] [PATCH RFC v3 14/14] intel_iommu: enable vfio devices

[Tian, Kevin]

> From: Peter Xu [mailto:address@hidden
> Sent: Thursday, January 19, 2017 11:17 AM
> 
> On Wed, Jan 18, 2017 at 09:38:55AM +0000, Tian, Kevin wrote:
> > > From: Peter Xu [mailto:address@hidden
> > > Sent: Wednesday, January 18, 2017 4:46 PM
> > >
> > > On Wed, Jan 18, 2017 at 04:36:05PM +0800, Jason Wang wrote:
> > > >
> > > >
> > > > On 2017年01月18日 16:11, Peter Xu wrote:
> > > > >On Wed, Jan 18, 2017 at 11:10:53AM +0800, Jason Wang wrote:
> > > > >>
> > > > >>On 2017年01月17日 22:45, Peter Xu wrote:
> > > > >>>On Mon, Jan 16, 2017 at 05:54:55PM +0800, Jason Wang wrote:
> > > > >>>>On 2017年01月16日 17:18, Peter Xu wrote:
> > > > >>>>>>>  static void vtd_iotlb_page_invalidate(IntelIOMMUState *s, 
> > > > >>>>>>> uint16_t
> > > domain_id,
> > > > >>>>>>>                                        hwaddr addr, uint8_t am)
> > > > >>>>>>>  {
> > > > >>>>>>>@@ -1222,6 +1251,7 @@ static void
> > > vtd_iotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
> > > > >>>>>>>      info.addr = addr;
> > > > >>>>>>>      info.mask = ~((1 << am) - 1);
> > > > >>>>>>>      g_hash_table_foreach_remove(s->iotlb,
> vtd_hash_remove_by_page,
> > > &info);
> > > > >>>>>>>+    vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am);
> > > > >>>>>>Is the case of GLOBAL or DSI flush missed, or we don't care it at 
> > > > >>>>>>all?
> > > > >>>>>IMHO we don't. For device assignment, since we are having CM=1 
> > > > >>>>>here,
> > > > >>>>>we should have explicit page invalidations even if guest sends
> > > > >>>>>global/domain invalidations.
> > > > >>>>>
> > > > >>>>>Thanks,
> > > > >>>>>
> > > > >>>>>-- peterx
> > > > >>>>Is this spec required?
> > > > >>>I think not. IMO the spec is very coarse grained on describing cache
> > > > >>>mode...
> > > > >>>
> > > > >>>>Btw, it looks to me that both DSI and GLOBAL are
> > > > >>>>indeed explicit flush.
> > > > >>>Actually when cache mode is on, it is unclear to me on how we should
> > > > >>>treat domain/global invalidations, at least from the spec (as
> > > > >>>mentioned earlier). My understanding is that they are not "explicit
> > > > >>>flushes", which IMHO should only mean page selective IOTLB
> > > > >>>invalidations.
> > > > >>Probably not, at least from the view of performance. DSI and global 
> > > > >>should
> > > > >>be more efficient in some cases.
> > > > >I agree with you that DSI/GLOBAL flushes are more efficient in some
> > > > >way. But IMHO that does not mean these invalidations are "explicit
> > > > >invalidations", and I suspect whether cache mode has to coop with it.
> > > >
> > > > Well, the spec does not forbid DSI/GLOBAL with CM and the driver codes 
> > > > had
> > > > used them for almost ten years. I can hardly believe it's wrong.
> > >
> > > I think we have misunderstanding here. :)
> > >
> > > I never thought we should not send DSI/GLOBAL invalidations with cache
> > > mode. I just think we should not do anything special even if we have
> > > cache mode on when we receive these signals.
> > >
> > > In the spec, "explicit invalidation" is mentioned in the cache mode
> > > chapter:
> > >
> > >     The Caching Mode (CM) field in Capability Register indicates if
> > >     the hardware implementation caches not-present or erroneous
> > >     translation-structure entries. When the CM field is reported as
> > >     Set, any software updates to any remapping structures (including
> > >     updates to not-present entries or present entries whose
> > >     programming resulted in translation faults) requires explicit
> > >     invalidation of the caches.
> > >
> > > And I thought we were discussion about "what is explicit invalidation"
> > > mentioned above.
> >
> > Check 6.5.3.1 Implicit Invalidation on Page Requests
> >
> >     In addition to the explicit invalidation through invalidation commands
> >     (see Section 6.5.1 and Section 6.5.2) or through deferred invalidation
> >     messages (see Section 6.5.4), identified above, Page Requests from
> >     endpoint devices invalidate entries in the IOTLBs and paging-structure
> >     caches.
> >
> > My impression is that above indirectly defines invalidation commands (
> > PSI/DSI/GLOBAL) as explicit invalidation, because they are explicitly
> > issued by driver. Then section 6.5.3.1 further describes implicit
> > invalidations caused by other VT-d operations.
> >
> > I will check with VT-d spec owner to clarify.
> 
> Above spec is clear to me. So now I agree that both DSI/GLOBAL iotlb
> invalidations are explicit invalidations.
> 

still copy response from spec owner here:-)

        Explicit invalidation is anytime software is explicitly invalidating 
something (
        through any descriptor) as opposed to something hardware does 
implicitly.  
        The only time hardware does implicit invalidation is during the 
handling of a page 
        request (recoverable page-fault) from an endpoint device.

Thanks
Kevin