ksummit.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: <ksummit@lists.linux.dev>, <linux-cxl@vger.kernel.org>,
	<linux-rdma@vger.kernel.org>, <netdev@vger.kernel.org>,
	<jgg@nvidia.com>, <shiju.jose@huawei.com>,
	Borislav Petkov <bp@alien8.de>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>
Subject: Re: [MAINTAINERS SUMMIT] Device Passthrough Considered Harmful?
Date: Mon, 29 Jul 2024 13:45:12 +0100	[thread overview]
Message-ID: <20240729134512.0000487f@Huawei.com> (raw)
In-Reply-To: <668c67a324609_ed99294c0@dwillia2-xfh.jf.intel.com.notmuch>


> Enter the fwctl proposal [1]. From the CXL subsystem perspective it
> looks like a long-term solution to the problem of managing expectations
> between hardware vendors and mainline subsystems. It disclaims support
> for the fast-path (data-plane) and is targeted at the long tail of
> slow-path (config/debug plane) device-specific operations that are often
> uninteresting to mainline. It sets expectations that the device must
> advertise the effect of all commands so that the kernel can deploy
> reasonable Kernel Lockdown policy, or otherwise require CAP_SYS_RAWIO
> for commands that may affect user-data. It sets common expectations for
> device designers, distribution maintainers, and kernel developers. It is
> complimentary to the Linux-command path for operations that need deeper
> kernel coordination.

I'm reasonably on board with the basic justification for the
fwctl proposal (and it may solve some long term challenges for us), but
I'm concerned by ABI guarantees.  In particularly what happens when
we get decision wrong and expose something via fwctl that we later
have more general kernel support for.

This was triggered by Dave Jiang's (perhaps unintended) use of Patrol Scrub
as a test case for the CXL FWCTL RFC.
https://lore.kernel.org/linux-cxl/20240729130528.0000139b@Huawei.com/T/#t
I'm using this specific example here, but I think it is a more general
question.

By exposing the Get / Set feature controls, a lot of effective user space
ABI is added (some of it setting a taint).

Scrub control is an interesting example, because there is an active
proposal to extend EDAC to cover this and similar RAS related control
features, something we are going to be discussing at LPC.
https://lore.kernel.org/linux-cxl/20240726160556.2079-1-shiju.jose@huawei.com/
One of the key bits of feedback we've had on that series is that it
should be integrated with EDAC.  Part of the reason being need to get
appropriate RAS expert review. Something fwctl won't naturally get.

If we expose that particular Feature via Set Feature we may run into
future problems.  It is probably possible to make the driver stateless
so any interference from a userspace program using fwctl is not fatal
- in this case userspace code should probably be safe to state changes
anyway. We know about this clash today, so could easily block fwctl
from exposing this feature, but it is illustrative of a wider problem.

We will get some decisions about what should be exposed via fwctl wrong
in the long term, even if they are correct at time of initial decision.
So how do we cope with that?

1) Make no guarantees on ABI for taint causing operations.
   So we can block this FWCTL in a kernel if EDAC / ras control is in place
   for the same feature.  I'm fine with this but it's not obviously
   a correct thing to do!
2) Allow the footgun. Keep the fwctl interface and harden the other kernel
   support against state changes that result. If userspace code breaks,
   then tough luck.  (Another form of ABI break, perhaps comprehended by
   existing proposed FWCTL rules).
3) We are stuck for ever with not supporting anything via other interfaces
   that would break if fwctl was in use.  Ouch.
Note that I think this only matters for the Set path as Get side shouldn't
have side effects and is fine to expose without synchronization with
a clear statement that values read are a snapshot only.

So before I'd be happy with fwctl in CXL, I'd want a very clear policy
decision on this that isn't going leave us in a mess long term.

We could say it can only be used for features we have 'opted' in +
vendor defined features, but I'm not sure that helps.  If a vendor
defines a feature for generation A, and does what we want them to by
proposing a spec addition they use in generation B, we would want a
path to single upstream interface for both generations.  So I don't
think restricting this to particular classes of command helps us.

Jonathan


  parent reply	other threads:[~2024-07-29 12:45 UTC|newest]

Thread overview: 126+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-08 22:26 Dan Williams
2024-07-09  6:09 ` Christoph Hellwig
2024-07-09 19:43   ` Dan Williams
2024-07-10 13:05     ` Jason Gunthorpe
2024-07-21 18:51       ` Laurent Pinchart
2024-07-22 13:27         ` Jason Gunthorpe
2024-07-09 10:01 ` Greg KH
2024-07-09 12:25   ` Leon Romanovsky
2024-07-09 12:33     ` Greg KH
2024-07-09 12:47       ` Leon Romanovsky
2024-07-11 14:07         ` Jason Gunthorpe
2024-07-09 20:09   ` Dan Williams
2024-07-09 16:02 ` James Bottomley
2024-07-09 22:15   ` Dan Williams
2024-07-10 13:22     ` Jonathan Cameron
2024-07-11 11:00       ` Jonathan Cameron
2024-07-11 15:05       ` Jason Gunthorpe
2024-07-11 17:01         ` Jonathan Cameron
2024-07-11 17:45           ` Jason Gunthorpe
2024-07-11 16:36       ` Dan Williams
2024-07-12 10:37         ` Jonathan Cameron
2024-07-21 19:25     ` Laurent Pinchart
2024-07-22  7:31       ` Leon Romanovsky
2024-07-22  8:53         ` Laurent Pinchart
2024-07-22 10:44           ` Leon Romanovsky
2024-07-22 11:10             ` Laurent Pinchart
2024-07-22 13:28               ` Leon Romanovsky
2024-07-22 14:13                 ` Laurent Pinchart
2024-07-22 14:43                   ` Jason Gunthorpe
2024-07-22 10:42       ` Ricardo Ribalda Delgado
2024-07-22 11:18         ` Laurent Pinchart
2024-07-22 11:56           ` Ricardo Ribalda Delgado
2024-07-25 20:01             ` Laurent Pinchart
2024-07-26  8:04               ` Ricardo Ribalda Delgado
2024-07-26 10:59                 ` Laurent Pinchart
2024-07-26 15:40                   ` Ricardo Ribalda Delgado
2024-07-28 17:18                     ` Laurent Pinchart
2024-07-29  9:58                       ` Ricardo Ribalda Delgado
2024-07-29 10:31                         ` Laurent Pinchart
2024-07-31 11:54                         ` Sakari Ailus
2024-07-31 13:15                           ` Daniel Vetter
2024-08-02 15:07                             ` Laurent Pinchart
2024-08-13 10:17                             ` Tomasz Figa
2024-08-13 10:26                               ` Laurent Pinchart
2024-08-13 10:33                                 ` Tomasz Figa
2024-08-13 10:58                                   ` Laurent Pinchart
2024-07-11 13:50   ` Jason Gunthorpe
2024-07-11 15:16     ` James Bottomley
2024-07-11 16:29       ` Jason Gunthorpe
2024-07-23 11:20 ` Jiri Kosina
2024-07-23 11:36   ` James Bottomley
2024-07-23 23:22     ` Jiri Kosina
2024-07-24 20:12       ` James Bottomley
2024-07-24 20:00     ` Laurent Pinchart
2024-07-24 20:37       ` James Bottomley
2024-07-24 21:10         ` Steven Rostedt
2024-07-25 19:31         ` Laurent Pinchart
2024-07-25 19:43           ` Jason Gunthorpe
2024-07-25 20:07             ` Laurent Pinchart
2024-07-25 23:39               ` Jason Gunthorpe
2024-07-26  8:04               ` Ricardo Ribalda Delgado
2024-07-26 12:49                 ` Laurent Pinchart
2024-07-26 13:11                   ` Jason Gunthorpe
2024-07-26 14:22                     ` Laurent Pinchart
2024-07-26 15:43                       ` Ricardo Ribalda Delgado
2024-07-28 15:25                         ` Laurent Pinchart
2024-07-29  9:57                           ` Ricardo Ribalda Delgado
2024-07-29 14:20                       ` Jason Gunthorpe
2024-07-26  8:03           ` Ricardo Ribalda Delgado
2024-07-26 13:22             ` Laurent Pinchart
2024-07-26 15:44               ` Ricardo Ribalda Delgado
2024-07-28 17:02                 ` Laurent Pinchart
2024-07-26 16:49           ` James Bottomley
2024-07-28 16:44             ` Laurent Pinchart
2024-07-26 17:33         ` Daniel Vetter
2024-07-29 14:10           ` Jason Gunthorpe
2024-07-25  9:26       ` Ricardo Ribalda Delgado
2024-07-25 10:51         ` Wolfram Sang
2024-07-25 12:23         ` Leon Romanovsky
2024-07-25 13:02           ` Ricardo Ribalda Delgado
2024-07-25 13:20             ` Leon Romanovsky
2024-07-25 13:29               ` Mark Brown
2024-07-25 14:18                 ` Leon Romanovsky
2024-07-25 14:22                   ` James Bottomley
2024-07-25 17:37                     ` Leon Romanovsky
2024-07-26 13:58                       ` James Bottomley
2024-07-25 19:42               ` Laurent Pinchart
2024-07-26  8:02                 ` Ricardo Ribalda Delgado
2024-07-26 13:11                   ` Laurent Pinchart
2024-07-26 15:40                     ` Ricardo Ribalda Delgado
2024-07-28 11:23                       ` Laurent Pinchart
2024-07-29  9:56                         ` Ricardo Ribalda Delgado
2024-07-29 10:38                           ` Laurent Pinchart
2024-07-26 16:01                     ` James Bottomley
2024-07-26 17:56                       ` Laurent Pinchart
2024-07-25 13:44             ` Steven Rostedt
2024-07-26 14:27 ` Laurent Pinchart
2024-07-26 15:34   ` Steven Rostedt
2024-07-28 16:03     ` Laurent Pinchart
2024-07-27  0:16   ` Dan Williams
2024-07-28 11:18     ` Laurent Pinchart
2024-07-28 15:16       ` Greg KH
2024-07-28 15:34         ` Laurent Pinchart
2024-07-28 15:49         ` James Bottomley
2024-07-29  6:10           ` Greg KH
2024-07-31 12:33             ` James Bottomley
2024-07-31 12:45               ` Laurent Pinchart
2024-08-01 14:41               ` Jason Gunthorpe
2024-08-07  0:06                 ` Dan Williams
2024-08-07  0:13                   ` James Bottomley
2024-08-16 11:12                   ` Hannes Reinecke
2024-07-29 14:56           ` Jakub Kicinski
2024-07-29 15:16             ` Greg KH
2024-07-29 15:29             ` Jason Gunthorpe
2024-07-29 12:45 ` Jonathan Cameron [this message]
2024-07-29 13:38   ` Borislav Petkov
2024-07-29 14:29     ` Jonathan Cameron
2024-07-29 14:58       ` Jason Gunthorpe
2024-07-30 13:19         ` Borislav Petkov
2024-08-01 14:23           ` Jason Gunthorpe
2024-07-29 15:42   ` Jason Gunthorpe
2024-07-29 22:37     ` Dan Williams
2024-07-30  7:13       ` Daniel Vetter
2024-08-01 14:22         ` Jason Gunthorpe
2024-08-06  7:14           ` Daniel Vetter
2024-08-06 13:04             ` Jason Gunthorpe

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=20240729134512.0000487f@Huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=jgg@nvidia.com \
    --cc=ksummit@lists.linux.dev \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=shiju.jose@huawei.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