Skip to contentSkip to conversation

iommu/vt-d: Preset Access/Dirty bits for IOVA over FL

PATCH In review Lu Baolu 2 participants 6 messages May 17, 2021

Subsystems (3)
Stable branch IOMMU Intel IOMMU (VT-d)
lore
Share & tools
lore

Lu Baolu

author maintainer
#0 lore raw

PATCH 1/1 iommu/vt-d: Preset Access/Dirty bits for IOVA over FL

[ Upstream commit a8ce9ebbecdfda3322bbcece6b3b25888217f8e3 ]

The Access/Dirty bits in the first level page table entry will be set whenever a page table entry was used for address translation or write permission was successfully translated. This is always true when using the first-level page table for kernel IOVA. Instead of wasting hardware cycles to update the certain bits, it's better to set them up at the beginning.

Suggested-by Ashok Raj <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Joerg Roedel <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
---
 drivers/iommu/intel/iommu.c | 14 ++++++++++++--
 include/linux/intel-iommu.h |  2 ++
 2 files changed, 14 insertions(+), 2 deletions(-)

[Note:
- This is a reworked patch of
https://lore.kernel.org/stable/[email protected]/T/#m65267f0a0091c2fcbde097cea91089775908faad.
- It aims to fix a reported issue of
https://bugzilla.kernel.org/show_bug.cgi?id=213077.
- Please help to review and test.]

drivers/iommu/intel/iommu.c +12 -2
index 3295e5e162a4..33ffbcb53dec 100644
@@ -1028,8 +1028,11 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
 
 			domain_flush_cache(domain, tmp_page, VTD_PAGE_SIZE);
 			pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) << VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
-			if (domain_use_first_level(domain))
+			if (domain_use_first_level(domain)) {
 				pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
+				if (domain->domain.type == IOMMU_DOMAIN_DMA)
+					pteval |= DMA_FL_PTE_ACCESS;
+			}
 			if (cmpxchg64(&pte->val, 0ULL, pteval))
 				/* Someone else set it while we were thinking; use theirs. */
 				free_pgtable_page(tmp_page);

@@ -2354,9 +2357,16 @@ static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
 		return -EINVAL;
 
 	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
-	if (domain_use_first_level(domain))
+	if (domain_use_first_level(domain)) {
 		attr |= DMA_FL_PTE_PRESENT | DMA_FL_PTE_XD | DMA_FL_PTE_US;
 
+		if (domain->domain.type == IOMMU_DOMAIN_DMA) {
+			attr |= DMA_FL_PTE_ACCESS;
+			if (prot & DMA_PTE_WRITE)
+				attr |= DMA_FL_PTE_DIRTY;
+		}
+	}
+
 	if (!sg) {
 		sg_res = nr_pages;
 		pteval = ((phys_addr_t)phys_pfn << VTD_PAGE_SHIFT) | attr;
include/linux/intel-iommu.h +2 -0
index 94522685a0d9..ccaa057faf8c 100644
@@ -42,6 +42,8 @@
 
 #define DMA_FL_PTE_PRESENT	BIT_ULL(0)
 #define DMA_FL_PTE_US		BIT_ULL(2)
+#define DMA_FL_PTE_ACCESS	BIT_ULL(5)
+#define DMA_FL_PTE_DIRTY	BIT_ULL(6)
 #define DMA_FL_PTE_XD		BIT_ULL(63)
 
 #define ADDR_WIDTH_5LEVEL	(57)
Signature · 1 line
2.25.1
maintainer
#1 lore raw
Quoted 19 lines · Lu Baolu

[ Upstream commit a8ce9ebbecdfda3322bbcece6b3b25888217f8e3 ]

The Access/Dirty bits in the first level page table entry will be set whenever a page table entry was used for address translation or write permission was successfully translated. This is always true when using the first-level page table for kernel IOVA. Instead of wasting hardware cycles to update the certain bits, it's better to set them up at the beginning.

Suggested-by Ashok Raj <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Joerg Roedel <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
---
 drivers/iommu/intel/iommu.c | 14 ++++++++++++--
 include/linux/intel-iommu.h |  2 ++
 2 files changed, 14 insertions(+), 2 deletions(-)
Quoted 6 lines

[Note:
- This is a reworked patch of
https://lore.kernel.org/stable/[email protected]/T/#m65267f0a0091c2fcbde097cea91089775908faad.
- It aims to fix a reported issue of
https://bugzilla.kernel.org/show_bug.cgi?id=213077.
- Please help to review and test.]

What stable tree(s) is this supposed to be for?

thanks,

greg k-h

author maintainer
#2 lore raw

Hi Greg,

Quoted 20 lines · Greg KH
Quoted 19 lines · Lu Baolu

[ Upstream commit a8ce9ebbecdfda3322bbcece6b3b25888217f8e3 ]

The Access/Dirty bits in the first level page table entry will be set whenever a page table entry was used for address translation or write permission was successfully translated. This is always true when using the first-level page table for kernel IOVA. Instead of wasting hardware cycles to update the certain bits, it's better to set them up at the beginning.

Suggested-by Ashok Raj <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Joerg Roedel <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
---
  drivers/iommu/intel/iommu.c | 14 ++++++++++++--
  include/linux/intel-iommu.h |  2 ++
  2 files changed, 14 insertions(+), 2 deletions(-)
Quoted 8 lines
Quoted 6 lines

[Note:
- This is a reworked patch of
https://lore.kernel.org/stable/[email protected]/T/#m65267f0a0091c2fcbde097cea91089775908faad.
- It aims to fix a reported issue of
https://bugzilla.kernel.org/show_bug.cgi?id=213077.
- Please help to review and test.]

What stable tree(s) is this supposed to be for?

It's for 5.10.37.

Quoted 5 lines

thanks,

greg k-h

Best regards,
baolu

maintainer
#3 lore raw
Quoted 30 lines · Lu Baolu

Hi Greg,

Quoted 27 lines · Greg KH
Quoted 25 lines · Lu Baolu

[ Upstream commit a8ce9ebbecdfda3322bbcece6b3b25888217f8e3 ]

The Access/Dirty bits in the first level page table entry will be set whenever a page table entry was used for address translation or write permission was successfully translated. This is always true when using the first-level page table for kernel IOVA. Instead of wasting hardware cycles to update the certain bits, it's better to set them up at the beginning.

Suggested-by Ashok Raj <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Joerg Roedel <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
---
  drivers/iommu/intel/iommu.c | 14 ++++++++++++--
  include/linux/intel-iommu.h |  2 ++
  2 files changed, 14 insertions(+), 2 deletions(-)

[Note:
- This is a reworked patch of
https://lore.kernel.org/stable/[email protected]/T/#m65267f0a0091c2fcbde097cea91089775908faad.
- It aims to fix a reported issue of
https://bugzilla.kernel.org/show_bug.cgi?id=213077.
- Please help to review and test.]

Quoted 3 lines
Quoted 1 line

What stable tree(s) is this supposed to be for?

It's for 5.10.37.

But the above commit is already in 5.10.y. And what about 5.11 and 5.12, were those backports incorrect?

confused,

greg k-h

author maintainer
#4 lore raw

Hi Greg,

Quoted 31 lines · Greg KH
Quoted 30 lines · Lu Baolu

Hi Greg,

Quoted 27 lines · Greg KH
Quoted 25 lines · Lu Baolu

[ Upstream commit a8ce9ebbecdfda3322bbcece6b3b25888217f8e3 ]

The Access/Dirty bits in the first level page table entry will be set whenever a page table entry was used for address translation or write permission was successfully translated. This is always true when using the first-level page table for kernel IOVA. Instead of wasting hardware cycles to update the certain bits, it's better to set them up at the beginning.

Suggested-by Ashok Raj <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Joerg Roedel <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
---
   drivers/iommu/intel/iommu.c | 14 ++++++++++++--
   include/linux/intel-iommu.h |  2 ++
   2 files changed, 14 insertions(+), 2 deletions(-)

[Note:
- This is a reworked patch of

    https://lore.kernel.org/stable/[email protected]/T/#m65267f0a0091c2fcbde097cea91089775908faad.
- It aims to fix a reported issue of
    https://bugzilla.kernel.org/show_bug.cgi?id=213077.

- Please help to review and test.]

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

What stable tree(s) is this supposed to be for?

It's for 5.10.37.

But the above commit is already in 5.10.y. And what about 5.11 and 5.12, were those backports incorrect?

Above commit is now only in v5.10.37. Other 5.10.y are not impacted.

5.11 and 5.12 both have correct backports.

Quoted 5 lines

confused,

greg k-h

Best regards,
baolu

maintainer
#5 lore raw
Quoted 36 lines · Lu Baolu

Hi Greg,

Quoted 33 lines · Greg KH
Quoted 32 lines · Lu Baolu

Hi Greg,

Quoted 28 lines · Greg KH
Quoted 19 lines · Lu Baolu

[ Upstream commit a8ce9ebbecdfda3322bbcece6b3b25888217f8e3 ]

The Access/Dirty bits in the first level page table entry will be set whenever a page table entry was used for address translation or write permission was successfully translated. This is always true when using the first-level page table for kernel IOVA. Instead of wasting hardware cycles to update the certain bits, it's better to set them up at the beginning.

Suggested-by Ashok Raj <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Joerg Roedel <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
---
   drivers/iommu/intel/iommu.c | 14 ++++++++++++--
   include/linux/intel-iommu.h |  2 ++
   2 files changed, 14 insertions(+), 2 deletions(-)
Quoted 6 lines

[Note:
- This is a reworked patch of

    https://lore.kernel.org/stable/[email protected]/T/#m65267f0a0091c2fcbde097cea91089775908faad.
- It aims to fix a reported issue of
    https://bugzilla.kernel.org/show_bug.cgi?id=213077.

- Please help to review and test.]

What stable tree(s) is this supposed to be for?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

It's for 5.10.37.

But the above commit is already in 5.10.y. And what about 5.11 and 5.12, were those backports incorrect?

Above commit is now only in v5.10.37. Other 5.10.y are not impacted.

5.11 and 5.12 both have correct backports.

Thanks for this, I've reverted the offending commit and added this one in it's place.

greg k-h