lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 20 Nov 2019 09:46:58 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Yonghyun Hwang <yonghyun@...gle.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Joerg Roedel <joro@...tes.org>
Cc:     baolu.lu@...ux.intel.com, iommu@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org,
        Havard Skinnemoen <hskinnemoen@...gle.com>,
        Deepa Dinamani <deepadinamani@...gle.com>,
        Moritz Fischer <moritzf@...gle.com>
Subject: Re: [PATCH] iommu/vt-d: Add side channel for huge page support to
 Intel IOMMU.

Hi,

On 11/20/19 8:36 AM, Yonghyun Hwang wrote:
> PCIe Address Translation Service (ATS) allows a PCIe device to look up
> VA->PA translations maintained by the IOMMU and cache them. The IOMMU up
> until at least Skylake fractures all ATS responses into 4 KiB pages, even
> if the underlying page is 2 MiB. For added performance, a side channel is
> defined, which lets the device know if the page is actually a 2 MiB page
> and uses that information instead of the ATS Response size information to
> determine the actual page size to use. The side channel is mapped to an
> unused high-order PA bit, 45-bit, that is transferred intact from the IOMMU
> PTE to the device.

BIT 45 of VT-d second level page table entry is not ignored but Reserved
(must be 0). (Spec 3.0 section 9.8). So it couldn't be used for software
purpose.

I guess the right approach should be VT-d hardware involved. For
example, VT-d reads the PTE entry in response of an ATS request, and
check the large page attribution of this PTE then include the large page
information in the response. It's not a good idea to hide the response
information in the pfn and walk around VT-d hardware.

Best regards,
baolu

> 
> Signed-off-by: Yonghyun Hwang <yonghyun@...gle.com>
> ---
>   drivers/iommu/intel-iommu.c | 83 ++++++++++++++++++++++++++++++++-----
>   drivers/iommu/intel-pasid.c |  2 +-
>   include/linux/intel-iommu.h | 37 ++++++++++++-----
>   include/linux/iommu.h       |  1 +
>   4 files changed, 102 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index fe8097078669..f748985b8081 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -307,6 +307,9 @@ static int hw_pass_through = 1;
>    */
>   #define DOMAIN_FLAG_LOSE_CHILDREN		BIT(1)
>   
> +/* Domain used for huge-page side channel. */
> +#define DOMAIN_FLAG_A45_HUGE_PAGE		(1 << 2)
> +
>   #define for_each_domain_iommu(idx, domain)			\
>   	for (idx = 0; idx < g_num_of_iommus; idx++)		\
>   		if (domain->iommu_refcnt[idx])
> @@ -552,6 +555,20 @@ static inline int domain_type_is_si(struct dmar_domain *domain)
>   	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;
>   }
>   
> +static inline void domain_type_set_a45_huge_page(struct dmar_domain *domain,
> +					     bool huge_page)
> +{
> +	if (huge_page)
> +		domain->flags |= DOMAIN_FLAG_A45_HUGE_PAGE;
> +	else
> +		domain->flags &= ~DOMAIN_FLAG_A45_HUGE_PAGE;
> +}
> +
> +inline bool domain_type_get_a45_huge_page(struct dmar_domain *domain)
> +{
> +	return (domain->flags & DOMAIN_FLAG_A45_HUGE_PAGE) != 0;
> +}
> +
>   static inline int domain_pfn_supported(struct dmar_domain *domain,
>   				       unsigned long pfn)
>   {
> @@ -922,7 +939,7 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
>   		if (level == 1)
>   			break;
>   
> -		parent = phys_to_virt(dma_pte_addr(pte));
> +		parent = phys_to_virt(dma_pte_addr(pte, domain));
>   		level--;
>   	}
>   
> @@ -958,7 +975,7 @@ static struct dma_pte *dma_pfn_level_pte(struct dmar_domain *domain,
>   			return pte;
>   		}
>   
> -		parent = phys_to_virt(dma_pte_addr(pte));
> +		parent = phys_to_virt(dma_pte_addr(pte, domain));
>   		total--;
>   	}
>   	return NULL;
> @@ -1012,7 +1029,7 @@ static void dma_pte_free_level(struct dmar_domain *domain, int level,
>   			goto next;
>   
>   		level_pfn = pfn & level_mask(level);
> -		level_pte = phys_to_virt(dma_pte_addr(pte));
> +		level_pte = phys_to_virt(dma_pte_addr(pte, domain));
>   
>   		if (level > 2) {
>   			dma_pte_free_level(domain, level - 1, retain_level,
> @@ -1073,7 +1090,7 @@ static struct page *dma_pte_list_pagetables(struct dmar_domain *domain,
>   {
>   	struct page *pg;
>   
> -	pg = pfn_to_page(dma_pte_addr(pte) >> PAGE_SHIFT);
> +	pg = pfn_to_page(dma_pte_addr(pte, domain) >> PAGE_SHIFT);
>   	pg->freelist = freelist;
>   	freelist = pg;
>   
> @@ -1125,7 +1142,8 @@ static struct page *dma_pte_clear_level(struct dmar_domain *domain, int level,
>   		} else if (level > 1) {
>   			/* Recurse down into a level that isn't *entirely* obsolete */
>   			freelist = dma_pte_clear_level(domain, level - 1,
> -						       phys_to_virt(dma_pte_addr(pte)),
> +						phys_to_virt(
> +						       dma_pte_addr(pte, domain)),
>   						       level_pfn, start_pfn, last_pfn,
>   						       freelist);
>   		}
> @@ -2063,7 +2081,7 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
>   			 */
>   			for (agaw = domain->agaw; agaw > iommu->agaw; agaw--) {
>   				ret = -ENOMEM;
> -				pgd = phys_to_virt(dma_pte_addr(pgd));
> +				pgd = phys_to_virt(dma_pte_addr(pgd, domain));
>   				if (!dma_pte_present(pgd))
>   					goto out_unlock;
>   			}
> @@ -2229,6 +2247,9 @@ static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
>   	unsigned long sg_res = 0;
>   	unsigned int largepage_lvl = 0;
>   	unsigned long lvl_pages = 0;
> +	uint64_t large_page = DMA_PTE_LARGE_PAGE |
> +	    (domain_type_get_a45_huge_page(domain) ?
> +	    DMA_PTE_A45_HUGE_PAGE : 0);
>   
>   	BUG_ON(!domain_pfn_supported(domain, iov_pfn + nr_pages - 1));
>   
> @@ -2265,7 +2286,7 @@ static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
>   			if (largepage_lvl > 1) {
>   				unsigned long nr_superpages, end_pfn;
>   
> -				pteval |= DMA_PTE_LARGE_PAGE;
> +				pteval |= large_page;
>   				lvl_pages = lvl_to_nr_pages(largepage_lvl);
>   
>   				nr_superpages = sg_res / lvl_pages;
> @@ -2280,7 +2301,7 @@ static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
>   				dma_pte_free_pagetable(domain, iov_pfn, end_pfn,
>   						       largepage_lvl + 1);
>   			} else {
> -				pteval &= ~(uint64_t)DMA_PTE_LARGE_PAGE;
> +				pteval &= ~large_page;
>   			}
>   
>   		}
> @@ -5377,7 +5398,7 @@ static int prepare_domain_attach_device(struct iommu_domain *domain,
>   		pte = dmar_domain->pgd;
>   		if (dma_pte_present(pte)) {
>   			dmar_domain->pgd = (struct dma_pte *)
> -				phys_to_virt(dma_pte_addr(pte));
> +				phys_to_virt(dma_pte_addr(pte, dmar_domain));
>   			free_pgtable_page(pte);
>   		}
>   		dmar_domain->agaw--;
> @@ -5386,6 +5407,46 @@ static int prepare_domain_attach_device(struct iommu_domain *domain,
>   	return 0;
>   }
>   
> +static int intel_iommu_domain_set_attr(struct iommu_domain *domain,
> +				       enum iommu_attr attr, void *data)
> +{
> +	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> +	int ret = 0;
> +
> +	switch (attr) {
> +	case DOMAIN_ATTR_A45_HUGE_PAGE: {
> +		bool *huge_page = data;
> +
> +		domain_type_set_a45_huge_page(dmar_domain, *huge_page);
> +		break;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int intel_iommu_domain_get_attr(struct iommu_domain *domain,
> +				       enum iommu_attr attr, void *data)
> +{
> +	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> +	int ret = 0;
> +
> +	switch (attr) {
> +	case DOMAIN_ATTR_A45_HUGE_PAGE: {
> +		bool *huge_page  = data;
> +
> +		*huge_page = domain_type_get_a45_huge_page(dmar_domain);
> +		break;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
>   static int intel_iommu_attach_device(struct iommu_domain *domain,
>   				     struct device *dev)
>   {
> @@ -5535,7 +5596,7 @@ static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
>   
>   	pte = pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, &level);
>   	if (pte)
> -		phys = dma_pte_addr(pte);
> +		phys = dma_pte_addr(pte, dmar_domain);
>   
>   	return phys;
>   }
> @@ -5958,6 +6019,8 @@ const struct iommu_ops intel_iommu_ops = {
>   	.capable		= intel_iommu_capable,
>   	.domain_alloc		= intel_iommu_domain_alloc,
>   	.domain_free		= intel_iommu_domain_free,
> +	.domain_set_attr	= intel_iommu_domain_set_attr,
> +	.domain_get_attr	= intel_iommu_domain_get_attr,
>   	.attach_dev		= intel_iommu_attach_device,
>   	.detach_dev		= intel_iommu_detach_device,
>   	.aux_attach_dev		= intel_iommu_aux_attach_device,
> diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
> index 040a445be300..ca7bf35cb4a0 100644
> --- a/drivers/iommu/intel-pasid.c
> +++ b/drivers/iommu/intel-pasid.c
> @@ -553,7 +553,7 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
>   	 */
>   	pgd = domain->pgd;
>   	for (agaw = domain->agaw; agaw > iommu->agaw; agaw--) {
> -		pgd = phys_to_virt(dma_pte_addr(pgd));
> +		pgd = phys_to_virt(dma_pte_addr(pgd, domain));
>   		if (!dma_pte_present(pgd)) {
>   			dev_err(dev, "Invalid domain page table\n");
>   			return -EINVAL;
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index ed11ef594378..4c18fdf8bca3 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -37,6 +37,17 @@
>   #define DMA_PTE_READ (1)
>   #define DMA_PTE_WRITE (2)
>   #define DMA_PTE_LARGE_PAGE (1 << 7)
> +/*
> + * PCIe Address Translation Service (ATS) allows a PCIe device to look up VA->PA
> + * translations maintained by the IOMMU and cache them. Unfortunately, up to at
> + * least Intel Skylake, the IOMMU fragments all ATS Translation Responses to 4
> + * KiB, even if the underlying page size is 2 MiB. In order to know if a page is
> + * mapped as 2 MiB, a side-channel is defined here by setting the
> + * DMA_PTE_A45_HUGE_PAGE bit in all huge-page PTEs. This bit can be received as
> + * part of the ATS translation response and interpreted only as 2 MiB page size
> + * indicator, but ignored otherwise.
> + */
> +#define DMA_PTE_A45_HUGE_PAGE (1UL << 45)
>   #define DMA_PTE_SNP (1 << 11)
>   
>   #define CONTEXT_TT_MULTI_LEVEL	0
> @@ -604,16 +615,6 @@ static inline void dma_clear_pte(struct dma_pte *pte)
>   	pte->val = 0;
>   }
>   
> -static inline u64 dma_pte_addr(struct dma_pte *pte)
> -{
> -#ifdef CONFIG_64BIT
> -	return pte->val & VTD_PAGE_MASK;
> -#else
> -	/* Must have a full atomic 64-bit read */
> -	return  __cmpxchg64(&pte->val, 0ULL, 0ULL) & VTD_PAGE_MASK;
> -#endif
> -}
> -
>   static inline bool dma_pte_present(struct dma_pte *pte)
>   {
>   	return (pte->val & 3) != 0;
> @@ -624,6 +625,22 @@ static inline bool dma_pte_superpage(struct dma_pte *pte)
>   	return (pte->val & DMA_PTE_LARGE_PAGE);
>   }
>   
> +bool domain_type_get_a45_huge_page(struct dmar_domain *domain);
> +static inline u64 dma_pte_addr(struct dma_pte *pte, struct dmar_domain *domain)
> +{
> +#ifdef CONFIG_64BIT
> +	u64 mask = VTD_PAGE_MASK;
> +
> +	if (dma_pte_superpage(pte) && domain_type_get_a45_huge_page(domain))
> +		mask &= ~DMA_PTE_A45_HUGE_PAGE;
> +
> +	return pte->val & mask;
> +#else
> +	/* Must have a full atomic 64-bit read */
> +	return  __cmpxchg64(&pte->val, 0ULL, 0ULL) & VTD_PAGE_MASK;
> +#endif
> +}
> +
>   static inline int first_pte_in_page(struct dma_pte *pte)
>   {
>   	return !((unsigned long)pte & ~VTD_PAGE_MASK);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e28e80dea141..e4231f1759a0 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -126,6 +126,7 @@ enum iommu_attr {
>   	DOMAIN_ATTR_FSL_PAMUV1,
>   	DOMAIN_ATTR_NESTING,	/* two stages of translation */
>   	DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
> +	DOMAIN_ATTR_A45_HUGE_PAGE,  /* huge-page side channel */
>   	DOMAIN_ATTR_MAX,
>   };
>   
> 

Powered by blists - more mailing lists