[PATCH v3 0/2] iommu: Avoid setting C-bit for MMIO addresses

[PATCH v3 0/2] iommu: Avoid setting C-bit for MMIO addresses

[Wei Wang]

AMD APM specifies that any pages corresponding to MMIO addresses must be
configured with the C-bit clear. The current iommu implementation sets
the C-bit on all PTEs in the IOMMU page tables. This is incorrect for PTEs
backed by MMIO, and can break PCIe peer-to-peer communication when IOVA is
used. Fix this by avoiding the C-bit for MMIO-backed mappings.

v2->v3 changes:
- re-implement the iommu part based on the iommu tree which has the
  iommupt patches merged.
  v2 link: https://lkml.org/lkml/2025/11/3/878

v1->v2 changes:
- 1 used page_is_ram() in the AMD IOMMU driver to detect non-RAM
  addresses, avoiding changes to upper-layer callers (vfio and iommufd).
  v2 instead lets upper layers explicitly indicate MMIO mappings via the
  IOMMU_MMIO prot flag. This avoids the potential overhead of
  page_is_ram(). (suggested by Jason Gunthorpe) 
  v1 link: https://lkml.org/lkml/2025/10/23/1211

Wei Wang (2):
  iommupt: Do not set C-bit on MMIO backed PTEs
  vfio/type1: Set IOMMU_MMIO in dma->prot for MMIO-backed addresses

 drivers/iommu/generic_pt/fmt/amdv1.h  |  3 ++-
 drivers/iommu/generic_pt/fmt/x86_64.h |  3 ++-
 drivers/vfio/vfio_iommu_type1.c       | 13 ++++++++-----
 3 files changed, 12 insertions(+), 7 deletions(-)

base-commit: d22ff9a783a792263baca22f80a64cfc7ec319ea
-- 
2.51.1

[PATCH v3 1/2] iommupt: Do not set C-bit on MMIO backed PTEs

[Wei Wang]

AMD Secure Memory Encryption (SME) marks individual memory pages as
encrypted by setting the C-bit in page table entries. According to the
AMD APM,any pages corresponding to MMIO addresses must be configured
with the C-bit clear.

The current *_iommu_set_prot() implementation sets the C-bit on all PTEs
in the IOMMU page tables. This is incorrect for PTEs backed by MMIO, and
can break PCIe peer-to-peer communication when IOVA is used. Fix this by
avoiding the C-bit for MMIO-backed mappings.

For amdv2 IOMMU page tables, there is a usage scenario for GVA->GPA
mappings, and for the trusted MMIO in the TEE-IO case, the C-bit will need
to be added to GPA. However, SNP guests do not yet support vIOMMU, and the
trusted MMIO support is not ready in upstream. Adding the C-bit for trusted
MMIO can be considered once those features land.

Fixes: 879ced2bab1b ("iommupt: Add the AMD IOMMU v1 page table format")
Fixes: aef5de756ea8 ("iommupt: Add the x86 64 bit page table format")
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
---
 drivers/iommu/generic_pt/fmt/amdv1.h  | 3 ++-
 drivers/iommu/generic_pt/fmt/x86_64.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/generic_pt/fmt/amdv1.h b/drivers/iommu/generic_pt/fmt/amdv1.h
index aa8e1a8ec95f..3b2c41d9654d 100644
--- a/drivers/iommu/generic_pt/fmt/amdv1.h
+++ b/drivers/iommu/generic_pt/fmt/amdv1.h
@@ -354,7 +354,8 @@ static inline int amdv1pt_iommu_set_prot(struct pt_common *common,
 	 * Ideally we'd have an IOMMU_ENCRYPTED flag set by higher levels to
 	 * control this. For now if the tables use sme_set then so do the ptes.
 	 */
-	if (pt_feature(common, PT_FEAT_AMDV1_ENCRYPT_TABLES))
+	if (pt_feature(common, PT_FEAT_AMDV1_ENCRYPT_TABLES) &&
+	    !(iommu_prot & IOMMU_MMIO))
 		pte = __sme_set(pte);
 
 	attrs->descriptor_bits = pte;
diff --git a/drivers/iommu/generic_pt/fmt/x86_64.h b/drivers/iommu/generic_pt/fmt/x86_64.h
index a86353f1481e..ec2143488e8b 100644
--- a/drivers/iommu/generic_pt/fmt/x86_64.h
+++ b/drivers/iommu/generic_pt/fmt/x86_64.h
@@ -227,7 +227,8 @@ static inline int x86_64_pt_iommu_set_prot(struct pt_common *common,
 	 * Ideally we'd have an IOMMU_ENCRYPTED flag set by higher levels to
 	 * control this. For now if the tables use sme_set then so do the ptes.
 	 */
-	if (pt_feature(common, PT_FEAT_X86_64_AMD_ENCRYPT_TABLES))
+	if (pt_feature(common, PT_FEAT_X86_64_AMD_ENCRYPT_TABLES) &&
+	    !(iommu_prot & IOMMU_MMIO))
 		pte = __sme_set(pte);
 
 	attrs->descriptor_bits = pte;
-- 
2.51.1

[PATCH v3 2/2] vfio/type1: Set IOMMU_MMIO in dma->prot for MMIO-backed addresses

[Wei Wang]

Before requesting the IOMMU driver to map an IOVA to a physical address,
set the IOMMU_MMIO flag in dma->prot when the physical address corresponds
to MMIO. This allows the IOMMU driver to handle MMIO mappings specially.
For example, on AMD CPUs with SME enabled, the IOMMU driver avoids setting
the C-bit if iommu_map() is called with IOMMU_MMIO set in prot. This
prevents issues with PCIe P2P communication when IOVA is used.

Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
---
 drivers/vfio/vfio_iommu_type1.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 5167bec14e36..cde7cfcfd61e 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -583,7 +583,7 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
  * returned initial pfn are provided; subsequent pfns are contiguous.
  */
 static long vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
-			   unsigned long npages, int prot, unsigned long *pfn,
+			   unsigned long npages, int *prot, unsigned long *pfn,
 			   struct vfio_batch *batch)
 {
 	unsigned long pin_pages = min_t(unsigned long, npages, batch->capacity);
@@ -591,7 +591,7 @@ static long vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 	unsigned int flags = 0;
 	long ret;
 
-	if (prot & IOMMU_WRITE)
+	if (*prot & IOMMU_WRITE)
 		flags |= FOLL_WRITE;
 
 	mmap_read_lock(mm);
@@ -601,6 +601,7 @@ static long vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 		*pfn = page_to_pfn(batch->pages[0]);
 		batch->size = ret;
 		batch->offset = 0;
+		*prot &= ~IOMMU_MMIO;
 		goto done;
 	} else if (!ret) {
 		ret = -EFAULT;
@@ -615,7 +616,7 @@ static long vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 		unsigned long addr_mask;
 
 		ret = follow_fault_pfn(vma, mm, vaddr, pfn, &addr_mask,
-				       prot & IOMMU_WRITE);
+				       *prot & IOMMU_WRITE);
 		if (ret == -EAGAIN)
 			goto retry;
 
@@ -629,6 +630,8 @@ static long vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 				ret = -EFAULT;
 			}
 		}
+		if (vma->vm_flags & VM_IO)
+			*prot |= IOMMU_MMIO;
 	}
 done:
 	mmap_read_unlock(mm);
@@ -709,7 +712,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
 			cond_resched();
 
 			/* Empty batch, so refill it. */
-			ret = vaddr_get_pfns(mm, vaddr, npage, dma->prot,
+			ret = vaddr_get_pfns(mm, vaddr, npage, &dma->prot,
 					     &pfn, batch);
 			if (ret < 0)
 				goto unpin_out;
@@ -850,7 +853,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
 
 	vfio_batch_init_single(&batch);
 
-	ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, &batch);
+	ret = vaddr_get_pfns(mm, vaddr, 1, &dma->prot, pfn_base, &batch);
 	if (ret != 1)
 		goto out;
 
-- 
2.51.1

Re: [PATCH v3 1/2] iommupt: Do not set C-bit on MMIO backed PTEs

[Jason Gunthorpe]

On Thu, Nov 13, 2025 at 11:54:06PM +0800, Wei Wang wrote:
> AMD Secure Memory Encryption (SME) marks individual memory pages as
> encrypted by setting the C-bit in page table entries. According to the
> AMD APM,any pages corresponding to MMIO addresses must be configured
> with the C-bit clear.
> 
> The current *_iommu_set_prot() implementation sets the C-bit on all PTEs
> in the IOMMU page tables. This is incorrect for PTEs backed by MMIO, and
> can break PCIe peer-to-peer communication when IOVA is used. Fix this by
> avoiding the C-bit for MMIO-backed mappings.
> 
> For amdv2 IOMMU page tables, there is a usage scenario for GVA->GPA
> mappings, and for the trusted MMIO in the TEE-IO case, the C-bit will need
> to be added to GPA. However, SNP guests do not yet support vIOMMU, and the
> trusted MMIO support is not ready in upstream. Adding the C-bit for trusted
> MMIO can be considered once those features land.
> 
> Fixes: 879ced2bab1b ("iommupt: Add the AMD IOMMU v1 page table format")
> Fixes: aef5de756ea8 ("iommupt: Add the x86 64 bit page table format")
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
> ---
>  drivers/iommu/generic_pt/fmt/amdv1.h  | 3 ++-
>  drivers/iommu/generic_pt/fmt/x86_64.h | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

RE: [PATCH v3 2/2] vfio/type1: Set IOMMU_MMIO in dma->prot for MMIO-backed addresses

[Wei Wang]

On Thursday, November 13, 2025 11:54 PM, Wei Wang wrote:
> Before requesting the IOMMU driver to map an IOVA to a physical address,
> set the IOMMU_MMIO flag in dma->prot when the physical address
> corresponds to MMIO. This allows the IOMMU driver to handle MMIO
> mappings specially.
> For example, on AMD CPUs with SME enabled, the IOMMU driver avoids
> setting the C-bit if iommu_map() is called with IOMMU_MMIO set in prot.
> This prevents issues with PCIe P2P communication when IOVA is used.
> 
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)


Gently ping.. any comments on the patch or questions about the fix? 
(Reminder: some questions and discussions could be found at the v2 link:
https://lkml.org/lkml/2025/11/3/878 )

> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c index 5167bec14e36..cde7cfcfd61e
> 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -583,7 +583,7 @@ static int follow_fault_pfn(struct vm_area_struct
> *vma, struct mm_struct *mm,
>   * returned initial pfn are provided; subsequent pfns are contiguous.
>   */
>  static long vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
> -			   unsigned long npages, int prot, unsigned long *pfn,
> +			   unsigned long npages, int *prot, unsigned long *pfn,
>  			   struct vfio_batch *batch)
>  {
>  	unsigned long pin_pages = min_t(unsigned long, npages, batch-
> >capacity); @@ -591,7 +591,7 @@ static long vaddr_get_pfns(struct
> mm_struct *mm, unsigned long vaddr,
>  	unsigned int flags = 0;
>  	long ret;
> 
> -	if (prot & IOMMU_WRITE)
> +	if (*prot & IOMMU_WRITE)
>  		flags |= FOLL_WRITE;
> 
>  	mmap_read_lock(mm);
> @@ -601,6 +601,7 @@ static long vaddr_get_pfns(struct mm_struct *mm,
> unsigned long vaddr,
>  		*pfn = page_to_pfn(batch->pages[0]);
>  		batch->size = ret;
>  		batch->offset = 0;
> +		*prot &= ~IOMMU_MMIO;
>  		goto done;
>  	} else if (!ret) {
>  		ret = -EFAULT;
> @@ -615,7 +616,7 @@ static long vaddr_get_pfns(struct mm_struct *mm,
> unsigned long vaddr,
>  		unsigned long addr_mask;
> 
>  		ret = follow_fault_pfn(vma, mm, vaddr, pfn, &addr_mask,
> -				       prot & IOMMU_WRITE);
> +				       *prot & IOMMU_WRITE);
>  		if (ret == -EAGAIN)
>  			goto retry;
> 
> @@ -629,6 +630,8 @@ static long vaddr_get_pfns(struct mm_struct *mm,
> unsigned long vaddr,
>  				ret = -EFAULT;
>  			}
>  		}
> +		if (vma->vm_flags & VM_IO)
> +			*prot |= IOMMU_MMIO;
>  	}
>  done:
>  	mmap_read_unlock(mm);
> @@ -709,7 +712,7 @@ static long vfio_pin_pages_remote(struct vfio_dma
> *dma, unsigned long vaddr,
>  			cond_resched();
> 
>  			/* Empty batch, so refill it. */
> -			ret = vaddr_get_pfns(mm, vaddr, npage, dma->prot,
> +			ret = vaddr_get_pfns(mm, vaddr, npage, &dma->prot,
>  					     &pfn, batch);
>  			if (ret < 0)
>  				goto unpin_out;
> @@ -850,7 +853,7 @@ static int vfio_pin_page_external(struct vfio_dma
> *dma, unsigned long vaddr,
> 
>  	vfio_batch_init_single(&batch);
> 
> -	ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, &batch);
> +	ret = vaddr_get_pfns(mm, vaddr, 1, &dma->prot, pfn_base, &batch);
>  	if (ret != 1)
>  		goto out;
> 
> --
> 2.51.1

RE: [PATCH v3 1/2] iommupt: Do not set C-bit on MMIO backed PTEs

[Tian, Kevin]

> From: Wei Wang <wei.w.wang@hotmail.com>
> Sent: Thursday, November 13, 2025 11:54 PM
> 
> AMD Secure Memory Encryption (SME) marks individual memory pages as
> encrypted by setting the C-bit in page table entries. According to the
> AMD APM,any pages corresponding to MMIO addresses must be configured
> with the C-bit clear.
> 
> The current *_iommu_set_prot() implementation sets the C-bit on all PTEs
> in the IOMMU page tables. This is incorrect for PTEs backed by MMIO, and
> can break PCIe peer-to-peer communication when IOVA is used. Fix this by
> avoiding the C-bit for MMIO-backed mappings.
> 
> For amdv2 IOMMU page tables, there is a usage scenario for GVA->GPA
> mappings, and for the trusted MMIO in the TEE-IO case, the C-bit will need
> to be added to GPA. However, SNP guests do not yet support vIOMMU, and
> the
> trusted MMIO support is not ready in upstream. Adding the C-bit for trusted
> MMIO can be considered once those features land.
> 
> Fixes: 879ced2bab1b ("iommupt: Add the AMD IOMMU v1 page table
> format")
> Fixes: aef5de756ea8 ("iommupt: Add the x86 64 bit page table format")
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

RE: [PATCH v3 2/2] vfio/type1: Set IOMMU_MMIO in dma->prot for MMIO-backed addresses

[Tian, Kevin]

> From: Wei Wang <wei.w.wang@hotmail.com>
> Sent: Thursday, November 13, 2025 11:54 PM
> 
> @@ -629,6 +630,8 @@ static long vaddr_get_pfns(struct mm_struct *mm,
> unsigned long vaddr,
>  				ret = -EFAULT;
>  			}
>  		}
> +		if (vma->vm_flags & VM_IO)
> +			*prot |= IOMMU_MMIO;

move into "if (is_invalid_reserved_pfn(*pfn)) {}". it's pointless to set
it in the error path.

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

btw another alternative is letting userspace set a map flag explicitly
e.g. introducing a new VFIO_DMA_MAP_FLAG_MMIO flag bit. It's
not considered because your use case requires existing VMMs to
work?

Re: [PATCH v3 2/2] vfio/type1: Set IOMMU_MMIO in dma->prot for MMIO-backed addresses

[Jason Gunthorpe]

On Fri, Dec 12, 2025 at 02:36:46AM +0000, Tian, Kevin wrote:
> > From: Wei Wang <wei.w.wang@hotmail.com>
> > Sent: Thursday, November 13, 2025 11:54 PM
> > 
> > @@ -629,6 +630,8 @@ static long vaddr_get_pfns(struct mm_struct *mm,
> > unsigned long vaddr,
> >  				ret = -EFAULT;
> >  			}
> >  		}
> > +		if (vma->vm_flags & VM_IO)
> > +			*prot |= IOMMU_MMIO;
> 
> move into "if (is_invalid_reserved_pfn(*pfn)) {}". it's pointless to set
> it in the error path.
> 
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> 
> btw another alternative is letting userspace set a map flag explicitly
> e.g. introducing a new VFIO_DMA_MAP_FLAG_MMIO flag bit. It's
> not considered because your use case requires existing VMMs to
> work?

I don't think we should do that, userspace shouldn't be able to create
a mapping inconsistent with the rest of the kernel for security
reasons.

I think if we ever need a cachable mmio through to the iommu then it
should be wired through the dmabuf mechanism the same way vfio drivers
wire the cachability when they make the mmap.

The nvgrace is the only driver that creates a cachable mmap already,
and it would be more correct to have a cachable IOPTE, but the only HW
tha driver works with does not care either way so it has been left.

Jason

RE: [PATCH v3 2/2] vfio/type1: Set IOMMU_MMIO in dma->prot for MMIO-backed addresses

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Friday, December 12, 2025 3:00 PM
> 
> On Fri, Dec 12, 2025 at 02:36:46AM +0000, Tian, Kevin wrote:
> > > From: Wei Wang <wei.w.wang@hotmail.com>
> > > Sent: Thursday, November 13, 2025 11:54 PM
> > >
> > > @@ -629,6 +630,8 @@ static long vaddr_get_pfns(struct mm_struct *mm,
> > > unsigned long vaddr,
> > >  				ret = -EFAULT;
> > >  			}
> > >  		}
> > > +		if (vma->vm_flags & VM_IO)
> > > +			*prot |= IOMMU_MMIO;
> >
> > move into "if (is_invalid_reserved_pfn(*pfn)) {}". it's pointless to set
> > it in the error path.
> >
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> >
> > btw another alternative is letting userspace set a map flag explicitly
> > e.g. introducing a new VFIO_DMA_MAP_FLAG_MMIO flag bit. It's
> > not considered because your use case requires existing VMMs to
> > work?
> 
> I don't think we should do that, userspace shouldn't be able to create
> a mapping inconsistent with the rest of the kernel for security
> reasons.

ah that's a good point!

> 
> I think if we ever need a cachable mmio through to the iommu then it
> should be wired through the dmabuf mechanism the same way vfio drivers
> wire the cachability when they make the mmap.
> 
> The nvgrace is the only driver that creates a cachable mmap already,
> and it would be more correct to have a cachable IOPTE, but the only HW
> tha driver works with does not care either way so it has been left.
> 
> Jason

Re: [PATCH v3 2/2] vfio/type1: Set IOMMU_MMIO in dma->prot for MMIO-backed addresses

[Wei Wang]

On 12/12/25 3:26 PM, Tian, Kevin wrote:
>> From: Jason Gunthorpe <jgg@nvidia.com>
>> Sent: Friday, December 12, 2025 3:00 PM
>>
>> On Fri, Dec 12, 2025 at 02:36:46AM +0000, Tian, Kevin wrote:
>>>> From: Wei Wang <wei.w.wang@hotmail.com>
>>>> Sent: Thursday, November 13, 2025 11:54 PM
>>>>
>>>> @@ -629,6 +630,8 @@ static long vaddr_get_pfns(struct mm_struct *mm,
>>>> unsigned long vaddr,
>>>>   				ret = -EFAULT;
>>>>   			}
>>>>   		}
>>>> +		if (vma->vm_flags & VM_IO)
>>>> +			*prot |= IOMMU_MMIO;
>>>
>>> move into "if (is_invalid_reserved_pfn(*pfn)) {}". it's pointless to set
>>> it in the error path.

Yeah, that looks better, thanks.

>>>
>>> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>>>
>>> btw another alternative is letting userspace set a map flag explicitly
>>> e.g. introducing a new VFIO_DMA_MAP_FLAG_MMIO flag bit. It's
>>> not considered because your use case requires existing VMMs to
>>> work?
>>
>> I don't think we should do that, userspace shouldn't be able to create
>> a mapping inconsistent with the rest of the kernel for security
>> reasons.
> 
> ah that's a good point!
> 
>>
>> I think if we ever need a cachable mmio through to the iommu then it
>> should be wired through the dmabuf mechanism the same way vfio drivers
>> wire the cachability when they make the mmap.
>>
>> The nvgrace is the only driver that creates a cachable mmap already,
>> and it would be more correct to have a cachable IOPTE, but the only HW
>> tha driver works with does not care either way so it has been left.
>>
>> Jason
>