public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Robert Richter <rrichter@amd.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Jiang <dave.jiang@intel.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	linux-cxl@vger.kernel.org, linux-kernel@vger.kernel.org,
	Gregory Price <gourry@gourry.net>,
	"Fabio M. De Francesco" <fabio.m.de.francesco@linux.intel.com>,
	Terry Bowman <terry.bowman@amd.com>,
	Joshua Hahn <joshua.hahnjy@gmail.com>,
	Borislav Petkov <bp@alien8.de>,
	Yazen Ghannam <yazen.ghannam@amd.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	John Allen <john.allen@amd.com>
Subject: Re: [PATCH v9 10/13] cxl: Enable AMD Zen5 address translation using ACPI PRMT
Date: Mon, 19 Jan 2026 15:33:33 +0100	[thread overview]
Message-ID: <aW5AvfcjLkSP2j42@rric.localdomain> (raw)
In-Reply-To: <20260116143838.GC1890602@noisy.programming.kicks-ass.net>

(+Rafael and some AMD folks)

Hi Peter,

On Fri, Jan 16, 2026 at 03:38:38PM +0100, Peter Zijlstra wrote:
> On Thu, Jan 15, 2026 at 09:30:10AM +0100, Ard Biesheuvel wrote:
> > On Thu, 15 Jan 2026 at 09:04, Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Wed, Jan 14, 2026 at 06:08:59PM +0000, Jonathan Cameron wrote:
> > >
> > > > Do we have a potential issue wrt to merging this as it stands and improving
> > > > on it later?  i.e. Is this a blocking issue for this patch set?
> > >
> > > Well, why do you *have* to use PRMT at all? And this is a serious
> > > question; PRMT is basically injecting unaudited magic code into the
> > > kernel, and that is a security risk.
> > >
> > > Worse, in order to run this shit, we have to lower or disable various
> > > security measures.
> > >
> > 
> > Only if we decide to keep running it privileged, which the PRM spec no
> > longer requires (as you have confirmed yourself when we last discussed
> > this, right?)
> 
> Indeed. But those very constraints also make me wonder why we would ever
> bother with PRM at all, and not simply require a native driver. Then you
> actually *know* what the thing does and can debug/fix it without having
> to rely on BIOS updates and whatnot.

an address translation driver needs the configuration data from the
Data Fabric, which is only known to firmware but not to the kernel.
Other ways would be necessary to expose and calculate that data, if it
is even feasible to make this information available.

So using PRM looks reasonable to me as this abstracts the logic and
data behind a method, same as doing a library call. Of course, you
don't want to trust that, but that could be addressed running it
unprivileged.

> Worse, you might have to deal with various incompatible buggy PRM
> versions because BIOS :/

The address translation functions are straight forward. I haven't
experienced any issues here. If there would be any, this will be
solvable, e.g. by requiring a specific minimum version or uuid to run
PRM.

> 
> > > If I had my way, we would WARN and TAINT the kernel whenever such
> > > garbage got used.
> > 
> > These are things that used to live in SMM, requiring all CPUs to
> > disappear into SMM mode in a way that was completely opaque to the OS.
> > 
> > PRM runs under the control of the OS, does not require privileges and
> > only needs MMIO access to the regions it describes in its manifest
> > (which the OS can inspect, if desired). So if there are security
> > concerns with PRM today, it is because we were lazy and did not
> > implement PRM securely from the beginning.
> > 
> > In my defense, I wasn't aware of the unprivileged requirement until
> > you spotted it recently: it was something I had asked for when the PRM
> > spec was put up for "review" by the Intel and MS authors, and they
> > told me they couldn't possibly make any changes at that point, because
> > it had already gone into production. But as it turns out, the change
> > was made after all.
> > 
> > I am a total noob when it comes to how x86 does its ring0/ring3
> > switching, but with some help, I should be able to prototype something
> > to call into the PRM service unprivileged, running under the efi_mm.
> 
> The ring transition itself is done using IRET; create a iret frame with
> userspace CS and the right IP (and flag etc.) and off you go. The
> problem is getting back in the kernel I suppose. All the 'normal' kernel
> entry points assume the kernel stack is empty and all that.
> 
> The whole usermodehelper stuff creates a whole extra thread, sets
> everything up and drops into userspace. Perhaps that is the easiest
> solution. Basically you set the thread's mm to efi_mm, populate
> task_pt_regs() with the right bits and simply drop into 'userspace'.
> 
> Then it can complete by terminating itself (sys_exit()) and the calling
> context reaps the thing and continues.

I can help with testing and also work on securing the PRM calls.
Thanks Ard for also looking into this.

> 
> > Would that allay your concerns?
> 
> Yeah, running it as userspace would be fine; we don't trust that.
> 
> But again; a native driver is ever so much better than relying on PRM.
> 
> In this case it is AMD doing a driver for their own chips, they know how
> they work, they should be able to write this natively.

Since a native driver introduces additional issues, as explained
above, I would prefer to use PRM for address translation and instead
ensure the PRM call is secure.

Dan, Dave, regarding this series, the cxl driver just uses existing
PRM kernel code and does not implement anything new here. Is there
anything that would prevent this series from being accepted? We are
already at v10 and review is complete:

https://patchwork.kernel.org/project/cxl/list/?series=1042412

I will follow up with working on unprivileged PRM calls. I think, that
will be the best solution here.

Thanks,

-Robert

  reply	other threads:[~2026-01-19 14:33 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-10 11:46 [PATCH v9 00/13] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement Robert Richter
2026-01-10 11:46 ` [PATCH v9 01/13] cxl/region: Rename misleading variable name @hpa to @hpa_range Robert Richter
2026-01-14  3:12   ` Alison Schofield
2026-01-10 11:46 ` [PATCH v9 02/13] cxl/region: Store root decoder in struct cxl_region Robert Richter
2026-01-14  3:13   ` Alison Schofield
2026-01-10 11:46 ` [PATCH v9 03/13] cxl/region: Store HPA range " Robert Richter
2026-01-14  3:14   ` Alison Schofield
2026-01-10 11:46 ` [PATCH v9 04/13] cxl: Simplify cxl_root_ops allocation and handling Robert Richter
2026-01-14  3:16   ` Alison Schofield
2026-01-10 11:46 ` [PATCH v9 05/13] cxl/region: Separate region parameter setup and region construction Robert Richter
2026-01-14  3:17   ` Alison Schofield
2026-01-10 11:46 ` [PATCH v9 06/13] cxl/region: Add @hpa_range argument to function cxl_calc_interleave_pos() Robert Richter
2026-01-14  3:17   ` Alison Schofield
2026-01-10 11:46 ` [PATCH v9 07/13] cxl/region: Use region data to get the root decoder Robert Richter
2026-01-14  3:19   ` Alison Schofield
2026-01-10 11:46 ` [PATCH v9 08/13] cxl: Introduce callback for HPA address ranges translation Robert Richter
2026-01-14  3:20   ` Alison Schofield
2026-01-10 11:46 ` [PATCH v9 09/13] cxl/acpi: Prepare use of EFI runtime services Robert Richter
2026-01-10 11:46 ` [PATCH v9 10/13] cxl: Enable AMD Zen5 address translation using ACPI PRMT Robert Richter
2026-01-14  7:47   ` Ard Biesheuvel
2026-01-14 14:00     ` Robert Richter
2026-01-14 15:21       ` Ard Biesheuvel
2026-01-14 18:08         ` Jonathan Cameron
2026-01-15  8:04           ` Peter Zijlstra
2026-01-15  8:30             ` Ard Biesheuvel
2026-01-16 14:38               ` Peter Zijlstra
2026-01-19 14:33                 ` Robert Richter [this message]
2026-01-19 15:00                   ` Gregory Price
2026-01-19 15:15                   ` Dave Jiang
2026-01-19 16:03                   ` Yazen Ghannam
2026-01-21  0:35                     ` dan.j.williams
2026-01-21 14:58                       ` Yazen Ghannam
2026-01-21 22:09                         ` dan.j.williams
2026-01-21 23:12                           ` Gregory Price
2026-01-22  2:05                             ` dan.j.williams
2026-01-22  6:09                               ` dan.j.williams
2026-01-20 21:23                   ` dan.j.williams
2026-01-10 11:46 ` [PATCH v9 11/13] cxl/atl: Lock decoders that need address translation Robert Richter
2026-01-10 11:46 ` [PATCH v9 12/13] cxl/region: Factor out code into cxl_region_setup_poison() Robert Richter
2026-01-13 22:39   ` Dave Jiang
2026-01-14  3:32   ` Alison Schofield
2026-01-14 18:17     ` Jonathan Cameron
2026-01-10 11:46 ` [PATCH v9 13/13] cxl: Disable HPA/SPA translation handlers for Normalized Addressing Robert Richter
2026-01-13 23:15   ` Dave Jiang
2026-01-14  3:59   ` Alison Schofield
2026-01-14 11:32     ` Robert Richter
2026-01-14 18:22   ` Jonathan Cameron
2026-02-03 18:52 ` [PATCH v9 00/13] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement Dave Jiang
2026-02-03 21:35   ` Gregory Price
2026-02-04 12:58   ` Robert Richter
2026-02-04 17:56     ` Dave Jiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aW5AvfcjLkSP2j42@rric.localdomain \
    --to=rrichter@amd.com \
    --cc=alison.schofield@intel.com \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=fabio.m.de.francesco@linux.intel.com \
    --cc=gourry@gourry.net \
    --cc=ira.weiny@intel.com \
    --cc=john.allen@amd.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=joshua.hahnjy@gmail.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=terry.bowman@amd.com \
    --cc=vishal.l.verma@intel.com \
    --cc=yazen.ghannam@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox