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: <20251024114039.GA847003@nvidia.com>
Date: Fri, 24 Oct 2025 08:40:39 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Wei Wang <wei.w.wang@...mail.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 Fri, Oct 24, 2025 at 03:05:05AM +0000, Wei Wang wrote:
> 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?

Sorry not page_is_ram(), but prot - it starts out with something that
had IOMMU_MMIO and got mangled.

> > 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

It is not a santiy check, a sanity check would fail the mapping. This
is changing what PTE is created by trying to guess properties about
the pfn.

> 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).

As I understand AMD's architecture the hypervisor runs with all ram as
encrypted and has to set the C bit for any dram. The MMIO is only
protected by the RMP and does not have a C bit set.

So even in a TDISP world with private MMIO we still end up with
system DRAM being treated differently than MMIO.

It really does seem like IOMMU_MMIO is the right direction here.

Again we should not be trying to guess if something is "ram" or not
deep inside the iommu code. We have IOMMU_MMIO specifically to tell
the iommu if it is ram or not.

Jason