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

[Joerg Roedel <joro@8bytes.org>]

On Fri, Oct 23, 2015 at 11:33:33AM +0100, David Woodhouse wrote:
> Which means I'm pondering *renaming* tlb_flush_kernel_range() to
> something like arch_tlb_flush_kernel_range() everywhere, then having a
> tlb_flush_kernel_range() inline function which optionally calls
> iommu_flush_kernel_range() first.

That sounds like some work, but would be the super clean solution :)
Given that only a handful of architecture besides x86 will need it
(thinking of ARM64 and PPC), I prefer the solution below:

> Or I could reduce the churn by adding explicit calls to
> iommu_flush_kernel_range() at every location that calls
> tlb_flush_kernel_range(), but that's going to lead to some callers
> missing the IOMMU flush.

Exactly like this, but when do we miss a flush here?

> Not entirely. The device still gets to specify whether it's doing
> supervisor or user mode access, for each request it makes. It doesn't
> open the door to users just using kernel addresses and getting away
> with it!
> 
> Sure, we need to trust the *device* a?? but we need to trust it to
> provide the correct PASID too. Which basically means in the VFIO case
> where the user gets *full* control of the device, we have to ensure
> that it gets its own PASID table with only the *one* PASID in it, and
> *that* PASID can't have supervisor mode.

Exactly, we need to trust the device and the device driver. But thats
not different to a situation without an iommu. We just run into problems
when a device-driver allows sending requests to access kernel-memory
from user-space, so it needs more care from the driver writers/reviewers
too.

> +static inline void do_iommu_flush_ktlb(unsigned long start, unsigned long end)
> +{
> +	iommu_flush_ktlb_fn *fn;
> +	rcu_read_lock();
> +	fn = rcu_dereference(iommu_flush_ktlb);
> +	if (fn)
> +		(*fn)(start, end);
> +	rcu_read_unlock();
> +}

Yes, that'll work too. When you read/update the iommu_flush_ktlb pointer
atomically, you can even get away without rcu. The function it points to
will not go away, so you can still call it even when the pointer turned
NULL.

> Maybe we could keep it simple and just declare that once the function
> pointer is set, it may never be cleared? But I think we really do want
> to avoid the out-of-line function call altogether in the case where
> kernel PASIDs are not being used. Or at *least* the case where SVM
> isn't being used at all.

Yes, thats why I think an inline function which does the checks would be
a better solution. The mmu_notifiers implement it in the same way, so we
would stay consistent with them.


	Joerg

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>