Re: [PATCH] iommu/vt-d: Add side channel for huge page support to Intel IOMMU.

From: Lu Baolu
Date: Wed Nov 20 2019 - 20:22:29 EST

Next message: Davidlohr Bueso: "[PATCH 1/4] x86/mm, pat: Convert pat tree to generic interval tree"
Previous message: Mimi Zohar: "Re: [PATCH v8 2/5] IMA: Define an IMA hook to measure keys"
In reply to: Lu Baolu: "Re: [PATCH] iommu/vt-d: Add side channel for huge page support to Intel IOMMU."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Yonghyun,

Linux VT-d driver is developed and maintained according to VT-d spec
rev3.0. I didn't find anything about super page support for ATS in the
spec. I am not the spec maintainer so I have no idea about how the spec
evolves.

Best regards,
baolu

On 11/21/19 5:54 AM, Yonghyun Hwang wrote:

Hello Baolu,

As farÂas I know, PCIe standard doesn't define a field for large page attribution in a response message. This was the motivation to implement the side channel.

I think I could miss something on your proposal. Could you elaborate how VT-d can be involved to include the large page attribution in the response message?

KindÂregards,
Yonghyun

On Tue, Nov 19, 2019 at 5:50 PM Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx <mailto:baolu.lu@xxxxxxxxxxxxxxx>> wrote:

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@xxxxxxxxxx
<mailto:yonghyun@xxxxxxxxxx>>
> ---
>Â Â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,
>Â Â};
>
>

Next message: Davidlohr Bueso: "[PATCH 1/4] x86/mm, pat: Convert pat tree to generic interval tree"
Previous message: Mimi Zohar: "Re: [PATCH v8 2/5] IMA: Define an IMA hook to measure keys"
In reply to: Lu Baolu: "Re: [PATCH] iommu/vt-d: Add side channel for huge page support to Intel IOMMU."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]