Re: [PATCH v9 10/13] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[<dan.j.williams@intel.com>]

Robert Richter wrote:
[..]
> > 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.

If it is just data is it amenable to put into a table?

Look at the complexity of the XOR addressing mode already defined in the
CEDT.CFMWS table, is the complexity significantly different than that?
 
> 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.

PRM should always be a last resort relative to an open specification
with a native driver implementation.

At a minimum Peter's feedback reiginited my simmering concerns with PRM
as a system-software design tool, and this should be a test case for
what Linux is willing and not willing to accept moving forward.

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

Can you publish the source to the PRM handler?

[..]
> > 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.

How is this case outside of the typical issues that kernel and its ABI
are meant to abstract?

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

The PRM to ring3 work is important for the PRM handlers that are
converting existing SMM flows to use PRM. For new DSMs the answer to the
"why not a native driver?" question needs to be clear.

That said, I am also interested in the PRM to ring3 work and did some
investigation there especially when the threat of runtime updates to PRM
handlers was being proposed. I think it is an important capability that
might also get some reuse with the confidential computing case for some
interactions with platform security services, but that is separate from
the primary question of enabling wider deployment of PRM solutions.