Subject: Re: [PATCH kernel v4 03/19] powerpc/vfio/iommu/kvm: Do not pin device memory

From: Paul Mackerras <[email-masked]>
Date: Thu, 13 Dec 2018 14:25:32 +1100

On Fri, Nov 23, 2018 at 04:52:48PM +1100, Alexey Kardashevskiy wrote:
> This new memory does not have page structs as it is not plugged to
> the host so gup() will fail anyway.
> 
> This adds 2 helpers:
> - mm_iommu_newdev() to preregister the "memory device" memory so
> the rest of API can still be used;
> - mm_iommu_is_devmem() to know if the physical address is one of thise
> new regions which we must avoid unpinning of.
> 
> This adds @mm to tce_page_is_contained() and iommu_tce_xchg() to test
> if the memory is device memory to avoid pfn_to_page().
> 
> This adds a check for device memory in mm_iommu_ua_mark_dirty_rm() which
> does delayed pages dirtying.

This mostly looks good, but I have one concern:

> -static bool tce_page_is_contained(struct page *page, unsigned page_shift)
> +static bool tce_page_is_contained(struct mm_struct *mm, unsigned long hpa,
> +		unsigned int page_shift)
>  {
> +	struct page *page;
> +
> +	if (mm_iommu_is_devmem(mm, hpa, page_shift))
> +		return true;
> +
> +	page = pfn_to_page(hpa >> PAGE_SHIFT);

Is it possible for userspace or a guest to cause us to get here with
hpa value that is bogus?  If so what does pfn_to_page do with that
pfn, and do we handle that correctly?

(I realize that if there is a problem here, it's a problem that
already exists in the code without this patch.)

Paul.