All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Christoph Hellwig <hch@infradead.org>,
	Jiho Chu <jiho.chu@samsung.com>
Subject: Re: New subsystem for acceleration devices
Date: Tue, 9 Aug 2022 11:22:21 -0300	[thread overview]
Message-ID: <YvJtnW8gvFzdSfH0@nvidia.com> (raw)
In-Reply-To: <CAK8P3a2149buJammrS=hqHPjKOYLRjJOxpSuT8-D_avYPZndOA@mail.gmail.com>

On Tue, Aug 09, 2022 at 02:46:36PM +0200, Arnd Bergmann wrote:
> On Tue, Aug 9, 2022 at 2:18 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> > On Tue, Aug 09, 2022 at 10:32:27AM +0200, Arnd Bergmann wrote:
> > > On Tue, Aug 9, 2022 at 10:04 AM Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > I think for devices with hardware MMU contexts you actually want to
> > > bind the context to a 'mm_struct', and then ensure that the context
> > > is only ever used from a process that shares this mm_struct,
> > > regardless of who else has access to the same file or inode.
> >
> > I can't think of a security justification for this.
> >
> > If process A stuffs part of its address space into the device and
> > passes the FD to process B which can then access that addresss space,
> > how is it any different from process A making a tmpfs, mmaping it, and
> > passing it to process B which can then access that address space?
> >
> > IMHO the 'struct file' is the security domain and a process must be
> > careful to only allow FDs to be created that meet its security needs.
> >
> > The kernel should not be involved in security here any further than
> > using the standard FD security mechanisms.
> >
> > Who is to say there isn't a meaningful dual process use case for the
> > accelerator? We see dual-process designs regularly in networking
> > accelerators.
> 
> I think there is a lot of value in keeping things simple here, to
> make sure users and developers don't make a mess of it. 

I don't think the kernel should enforce policy on userspace. As long
as the kernel side is simple and reasonable then it should let
userspace make whatever mess it likes.

We have a lot of experiance here now, and userspaces do take advantage
of this flexability in more well established accelerator subsystems,
like DRM and RDMA.

> If the accelerator has access to the memory of one process but I run
> it from another process, I lose the benefits of the shared page
> tables,

There are several accelerator "ASID" models I've seen - devices can
have one or many ASID's and the ASID's can be map/unmap style or forced
1:1 with a mm_struct (usually called SVA/SVM).

Userspace is responsible to figure out how to use this stuff. With
map/unmap there should be no kernel restriction on what mappings can
be created, but often sane userspaces will probably want to stick to
1:1 map/unmap with a single process.

> E.g. attaching a debugger to single-step through the accelerator code
> would then involve at least four address spaces:
>
>  - the addresses of the debugger
>  - the addresses local to the accelerator
>  - addresses in the shared address space of the process that owns
>    the memory
>  - addresses in the process that owns the accelerator context
> 
> which is at least one more than I'd like to deal with.

It is a FD. There is no "owns the accelerator context" - that concept
is an abuse of the FD model, IMHO.

If you are debugging you have the mmu_struct of each of the threads
you are debugging and each of the ASID's loaded in the accelerator to
deal with - it is inherent in the hardware design.

> This is somewhat different for accelerators that have coherent
> access to a process address space and only access explicitly
> shared buffers. On these you could more easily allow sharing the
> file descriptor between any number of processes.

That is just a multi-ASID accelerator and userspace has linked a
unique SVA ASID to each unique process using the FD.

The thing to understand is that the FD represents the security
context, so it is very resonable on a multi-ASID device I could share
the same security context with two co-operating processes, create two
ASID's and do accelerator operations that work jointly across both
memory spaces. For instance I might consume a source from process B,
process it and deliver the result into process A where process A will
then send it over the network or something. We have these kinds of use
models already.

Jason

WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Hellwig <hch@infradead.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Oded Gabbay <oded.gabbay@gmail.com>,
	Dave Airlie <airlied@gmail.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp>,
	Jiho Chu <jiho.chu@samsung.com>,
	"Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>
Subject: Re: New subsystem for acceleration devices
Date: Tue, 9 Aug 2022 11:22:21 -0300	[thread overview]
Message-ID: <YvJtnW8gvFzdSfH0@nvidia.com> (raw)
In-Reply-To: <CAK8P3a2149buJammrS=hqHPjKOYLRjJOxpSuT8-D_avYPZndOA@mail.gmail.com>

On Tue, Aug 09, 2022 at 02:46:36PM +0200, Arnd Bergmann wrote:
> On Tue, Aug 9, 2022 at 2:18 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> > On Tue, Aug 09, 2022 at 10:32:27AM +0200, Arnd Bergmann wrote:
> > > On Tue, Aug 9, 2022 at 10:04 AM Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > I think for devices with hardware MMU contexts you actually want to
> > > bind the context to a 'mm_struct', and then ensure that the context
> > > is only ever used from a process that shares this mm_struct,
> > > regardless of who else has access to the same file or inode.
> >
> > I can't think of a security justification for this.
> >
> > If process A stuffs part of its address space into the device and
> > passes the FD to process B which can then access that addresss space,
> > how is it any different from process A making a tmpfs, mmaping it, and
> > passing it to process B which can then access that address space?
> >
> > IMHO the 'struct file' is the security domain and a process must be
> > careful to only allow FDs to be created that meet its security needs.
> >
> > The kernel should not be involved in security here any further than
> > using the standard FD security mechanisms.
> >
> > Who is to say there isn't a meaningful dual process use case for the
> > accelerator? We see dual-process designs regularly in networking
> > accelerators.
> 
> I think there is a lot of value in keeping things simple here, to
> make sure users and developers don't make a mess of it. 

I don't think the kernel should enforce policy on userspace. As long
as the kernel side is simple and reasonable then it should let
userspace make whatever mess it likes.

We have a lot of experiance here now, and userspaces do take advantage
of this flexability in more well established accelerator subsystems,
like DRM and RDMA.

> If the accelerator has access to the memory of one process but I run
> it from another process, I lose the benefits of the shared page
> tables,

There are several accelerator "ASID" models I've seen - devices can
have one or many ASID's and the ASID's can be map/unmap style or forced
1:1 with a mm_struct (usually called SVA/SVM).

Userspace is responsible to figure out how to use this stuff. With
map/unmap there should be no kernel restriction on what mappings can
be created, but often sane userspaces will probably want to stick to
1:1 map/unmap with a single process.

> E.g. attaching a debugger to single-step through the accelerator code
> would then involve at least four address spaces:
>
>  - the addresses of the debugger
>  - the addresses local to the accelerator
>  - addresses in the shared address space of the process that owns
>    the memory
>  - addresses in the process that owns the accelerator context
> 
> which is at least one more than I'd like to deal with.

It is a FD. There is no "owns the accelerator context" - that concept
is an abuse of the FD model, IMHO.

If you are debugging you have the mmu_struct of each of the threads
you are debugging and each of the ASID's loaded in the accelerator to
deal with - it is inherent in the hardware design.

> This is somewhat different for accelerators that have coherent
> access to a process address space and only access explicitly
> shared buffers. On these you could more easily allow sharing the
> file descriptor between any number of processes.

That is just a multi-ASID accelerator and userspace has linked a
unique SVA ASID to each unique process using the FD.

The thing to understand is that the FD represents the security
context, so it is very resonable on a multi-ASID device I could share
the same security context with two co-operating processes, create two
ASID's and do accelerator operations that work jointly across both
memory spaces. For instance I might consume a source from process B,
process it and deliver the result into process A where process A will
then send it over the network or something. We have these kinds of use
models already.

Jason

  reply	other threads:[~2022-08-09 14:24 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20220731114605epcas1p1afff6b948f542e2062b60d49a8023f6f@epcas1p1.samsung.com>
2022-07-31 11:45 ` New subsystem for acceleration devices Oded Gabbay
2022-07-31 15:37   ` Greg Kroah-Hartman
2022-08-01  2:29     ` yuji2.ishikawa
2022-08-01  8:21       ` Oded Gabbay
2022-08-03  4:39         ` yuji2.ishikawa
2022-08-03  5:34           ` Greg KH
2022-08-03 20:28           ` Oded Gabbay
2022-08-02 17:25   ` Jiho Chu
2022-08-02 19:07     ` Oded Gabbay
2022-08-03 19:04   ` Dave Airlie
2022-08-03 20:20     ` Oded Gabbay
2022-08-03 23:31       ` Daniel Stone
2022-08-04  6:46         ` Oded Gabbay
2022-08-04  9:27           ` Jiho Chu
2022-08-03 23:54       ` Dave Airlie
2022-08-03 23:54         ` Dave Airlie
2022-08-04  7:43         ` Oded Gabbay
2022-08-04  7:43           ` Oded Gabbay
2022-08-04 14:50           ` Jason Gunthorpe
2022-08-04 14:50             ` Jason Gunthorpe
2022-08-04 17:48             ` Oded Gabbay
2022-08-04 17:48               ` Oded Gabbay
2022-08-05  0:22               ` Jason Gunthorpe
2022-08-05  0:22                 ` Jason Gunthorpe
2022-08-07  6:43                 ` Oded Gabbay
2022-08-07  6:43                   ` Oded Gabbay
2022-08-07 11:25                   ` Oded Gabbay
2022-08-07 11:25                     ` Oded Gabbay
2022-08-08  6:10                     ` Greg Kroah-Hartman
2022-08-08  6:10                       ` Greg Kroah-Hartman
2022-08-08 17:55                       ` Jason Gunthorpe
2022-08-08 17:55                         ` Jason Gunthorpe
2022-08-09  6:23                         ` Greg Kroah-Hartman
2022-08-09  6:23                           ` Greg Kroah-Hartman
2022-08-09  8:04                           ` Christoph Hellwig
2022-08-09  8:32                             ` Arnd Bergmann
2022-08-09  8:32                               ` Arnd Bergmann
2022-08-09 12:18                               ` Jason Gunthorpe
2022-08-09 12:18                                 ` Jason Gunthorpe
2022-08-09 12:46                                 ` Arnd Bergmann
2022-08-09 12:46                                   ` Arnd Bergmann
2022-08-09 14:22                                   ` Jason Gunthorpe [this message]
2022-08-09 14:22                                     ` Jason Gunthorpe
2022-08-09  8:45                             ` Greg Kroah-Hartman
2022-08-09  8:45                               ` Greg Kroah-Hartman
2022-08-08 17:46                   ` Jason Gunthorpe
2022-08-08 17:46                     ` Jason Gunthorpe
2022-08-08 20:26                     ` Oded Gabbay
2022-08-08 20:26                       ` Oded Gabbay
2022-08-09 12:43                       ` Jason Gunthorpe
2022-08-09 12:43                         ` Jason Gunthorpe
2022-08-05  3:02           ` Dave Airlie
2022-08-05  3:02             ` Dave Airlie
2022-08-07  6:50             ` Oded Gabbay
2022-08-07  6:50               ` Oded Gabbay
2022-08-09 21:42               ` Oded Gabbay
2022-08-09 21:42                 ` Oded Gabbay
2022-08-10  9:00                 ` Jiho Chu
2022-08-10  9:00                   ` Jiho Chu
2022-08-10 14:05                 ` yuji2.ishikawa
2022-08-10 14:05                   ` yuji2.ishikawa
2022-08-10 14:37                   ` Oded Gabbay
2022-08-10 14:37                     ` Oded Gabbay
2022-08-23 18:23                 ` Kevin Hilman
2022-08-23 18:23                   ` Kevin Hilman
2022-08-23 20:45                   ` Oded Gabbay
2022-08-23 20:45                     ` Oded Gabbay
2022-08-29 20:54                     ` Kevin Hilman
2022-08-29 20:54                       ` Kevin Hilman
2022-09-23 16:21                       ` Oded Gabbay
2022-09-23 16:21                         ` Oded Gabbay
2022-09-26  8:16                         ` Christoph Hellwig
2022-09-29  6:50                           ` Oded Gabbay
2022-09-29  6:50                             ` Oded Gabbay
2022-08-04 12:00         ` Tvrtko Ursulin
2022-08-04 15:03           ` Jeffrey Hugo
2022-08-04 17:53             ` Oded Gabbay
2022-08-04 17:53               ` Oded Gabbay

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=YvJtnW8gvFzdSfH0@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=arnd@arndb.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=jiho.chu@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yuji2.ishikawa@toshiba.co.jp \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.