Re: [RFC PATCH] iommu/vt-d: Add IOTLB flush support for kernel addresses

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]

On Tue, 2015-10-20 at 18:03 +0200, Joerg Roedel wrote:
> Hi David,
> 
> On Tue, Oct 20, 2015 at 04:52:59PM +0100, David Woodhouse wrote:
> >  void flush_tlb_kernel_range(unsigned long start, unsigned long end)
> >  {
> > +> > 	> > intel_iommu_flush_kernel_pasid(start, end);
> 
> A more generic naming would be good, and probably expose it through a
> function in the IOMMU-API.

Yeah. *All* the SVM functionality needs to be exposed through the DMA
API (for native drivers like i915) and the IOMMU API (so VM guests can
see an IOMMU and do SVM on assigned devices).

Can we assume that only one type of SVM-capable IOMMU will be present
in the system at a time? Perhaps we could just register a single
function (intel_iommu_flush_kernel_pasid in the VT-d case) to be used
as a notifier callback from tlb_flush_kernel_range()? Rather than the
overhead of a *list* of notifiers?

> > +void intel_iommu_flush_kernel_pasid(unsigned long start, unsigned long end)
> > +{
> > +> > 	> > struct dmar_drhd_unit *drhd;
> > +> > 	> > struct intel_iommu *iommu;
> > +> > 	> > unsigned long pages;
> 
> And I think, as a performance optimiztion, we should bail out early here
> if the pasid has no users.

If the PASID has no users, it won't exist (iommu->kernel_svm will be
NULL). We still have to walk each IOMMU to see if it has one.

But... that's because the PASID-space is currently per-IOMMU. The plan
is to have a *single* PASID-space system-wide, And then I still want to
retain the property that there can be only *one* kernel PASID.

I have forbidden the use of a given PASID to access *both* kernel and
user addresses. I'm hoping we can get away with putting that
restriction into the generic SVM APIs.

So yeah, perhaps we can set the notifier pointer to NULL when there's
no kernel PASID assigned, and only set it to point to
${MY_IOMMU}_flush_kernel_pasid() if/when there *is* one?

That way, tlb_flush_kernel_range() doesn't even need to make the call
when there's no work to do...

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]