linux-kernel - RE: [PATCH v1] iommu/amd: Set C-bit only for RAM-backed PTEs in IOMMU page tables

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] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <SI2PR01MB4393D52AD53469388BED8E6CDCF1A@SI2PR01MB4393.apcprd01.prod.exchangelabs.com>
Date: Fri, 24 Oct 2025 03:05:05 +0000
From: Wei Wang <wei.w.wang@...mail.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: "suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
	"thomas.lendacky@....com" <thomas.lendacky@....com>, "joro@...tes.org"
	<joro@...tes.org>, "kevin.tian@...el.com" <kevin.tian@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>
Subject: RE: [PATCH v1] iommu/amd: Set C-bit only for RAM-backed PTEs in IOMMU
 page tables

On Friday, October 24, 2025 12:02 AM, Jason Gunthorpe wrote:
> On Thu, Oct 23, 2025 at 11:15:43PM +0800, Wei Wang wrote:
> > When SME is enabled, iommu_v1_map_pages() currently sets the C-bit for
> > all physical addresses. This is correct for RAM, since the C-bit is
> > required by SME to indicate encrypted memory and ensure proper
> > encryption/decryption.
> >
> > However, applying the C-bit to MMIO addresses is incorrect. Devices
> > and PCIe switches do not interpret the C-bit currently, and doing so
> > can break PCIe peer-to-peer communication. To avoid this, only set the
> > C-bit when the physical address is backed by RAM, and leave MMIO
> mappings unchanged.
> >
> > Fixes: 2543a786aa25 ("iommu/amd: Allow the AMD IOMMU to work with
> > memory encryption")
> > Signed-off-by: Wei Wang <wei.w.wang@...mail.com>
> > ---
> >  drivers/iommu/amd/io_pgtable.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/amd/io_pgtable.c
> > b/drivers/iommu/amd/io_pgtable.c index 70c2f5b1631b..6f395940d0a4
> > 100644
> > --- a/drivers/iommu/amd/io_pgtable.c
> > +++ b/drivers/iommu/amd/io_pgtable.c
> > @@ -353,6 +353,9 @@ static int iommu_v1_map_pages(struct
> io_pgtable_ops *ops, unsigned long iova,
> >  	if (!(prot & IOMMU_PROT_MASK))
> >  		goto out;
> >
> > +	if (sme_me_mask && page_is_ram(PHYS_PFN(paddr)))
> > +		paddr = __sme_set(paddr);
> 
> It needs to use the IOMMU_MMIO flag not page_is_ram, which I think got
> mangled by the time it reached here..

Could you please elaborate how page_is_ram() would be mangled when
reaching here?
(I might not have fully understood your point. My understanding was that
the result from page_is_ram(), regarding whether a physical address
corresponds to system RAM, is stable after the system boots. One exception
would be RAM hotplug, but this is the same for the case using IOMMU_MMIO
from callers. If the stability of this result is indeed problematic, then
other users of page_is_ram() and /proc/iomem would also be at risk)

> 
> Though broadly this points to a larger problem, the iommu domain code
> should not be trying to guess if a mapping is private or not, this needs to be
> passed in from higher level code which knows what state the PFN is..

Please note that this patch is not intended to add an interface allowing users
to specify whether a requested physical address is expected to be mapped as
Private or not. Instead, it implements a sanity or correctness check within the
IOMMU driver to validate whether a user-supplied address _can_ be mapped
with the Private bit (RAM is the case that "can" currently, and since the driver
can already determine whether a PFN is RAM or not, I'm not sure why it needs
an interface for users to tell the driver).

Even if a need arises in the future to add a new interface for users to indicate
that the IOVA->PA mapping expects a Private bit to PA, a new flag (e.g.,
IOMMU_ENCRYPT) would be more semantically appropriate than IOMMU_MMIO.
The validation introduced by this patch would still be necessary. I think this check
Is essentially similar to other guardrails or gatekeeping functions at lower driver
layers.