From: "Arnd Bergmann" <arnd@arndb.de>
To: "Ryan Roberts" <ryan.roberts@arm.com>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Catalin Marinas" <catalin.marinas@arm.com>,
"Will Deacon" <will@kernel.org>,
"Mark Rutland" <mark.rutland@arm.com>,
"Jean-Philippe Brucker" <jpb@kernel.org>,
"Oded Gabbay" <ogabbay@kernel.org>,
"Jonathan Corbet" <corbet@lwn.net>
Cc: linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org
Subject: Re: [RFC PATCH v1 8/8] misc/arm-cla: Add userspace interface
Date: Fri, 17 Jul 2026 17:31:46 +0200 [thread overview]
Message-ID: <59d0c4d5-af53-410d-9bf8-8dd2ba17f697@app.fastmail.com> (raw)
In-Reply-To: <5012970d-a0f0-461c-b0b6-61823e0aab2d@arm.com>
On Fri, Jul 17, 2026, at 16:35, Ryan Roberts wrote:
> On 17/07/2026 13:54, Arnd Bergmann wrote:
>> On Fri, Jul 17, 2026, at 12:47, Ryan Roberts wrote:
>>> Expose CLA devices through a character device so userspace can enumerate
>>> the available hardware and map accelerator register frames.
>>>
>>> Define version 1 of the CLA UAPI with a GET_PARAM ioctl. Report device
>>> topology, CPU affinity, domain membership, mmap offsets, architecture
>>> version and attached accelerator masks, together with the IIDR, DEVARCH
>>> and REVIDR of each accelerator.
>>>
>>> CLA registers can only be read from the CPU local to the device, while
>>> enumeration may occur on any CPU. Validate the supported CLA
>>> architecture version during device setup and cache the CLA and
>>> accelerator identification registers for later ioctl queries.
>>
>> This interface looks very raw at the moment, I expect this will have
>> one or more larger redesigns.
>
> Are you referring to the overall UABI or specifically to the ioctl interface
> here? I could imagine the ioctl interface evolving before we get this merged
> (although it is based on similar patterns used by some DRM and accel drivers -
> it's intended to be easily extensible while existing params remain stable).
>
> The aspect where the CLA MMIO is directly mapped into user space is an aspect we
> are keen to keep though since it has significant performance implications if we
> need to redirect through the kernel.
I see no problem with having a per-accelerator VM area for MMIO
operations mapped to userspace, this is obviously part of all
designs like this one that sneak custom coprocessor instructions
in by disguising them as MMIO.
My concern is about how userspace gets to that mapping, both
the chardev itself, but also the ioctls.
>> Most importantly, a single character device to expose an arbitrary
>> number of underlying hardware features is an inherently flawed security
>> model. If any specific accelerator is ever found to have a
>> major vulnerability, that would mean administrators will have to
>> disable all of them by default.
>
> Note that we are exposing MMIO per CLA, not per accelerator. So preventing
> access to a single CLA would prevent access to all accelerators attached to it.
> There is a per-accelerator availability masking control that the kernel can use
> to disable access to selected accelerators though, which might help with the
> vulnerability example.
>
> The rationale for choosing a single device file was driven by performance: At
> domain reassignment time, we need to unmap and invalidate the TLB entries for
> all the devices in the domain from the out-going process's address space. By
> having all the devices in a single file and all devices within the same domain
> adjacent, they can all be mapped to a single VMA, meaning the driver can use a
> single call to the existing zap_special_vma_range(), which will result in a
> single TLBI-by-range instruction, which is faster than a TLBI-by-va for every
> device.
>
> If you think it is important for security to have each CLA exposed by an
> independent device file, I'll take another look.
Without concrete implementation examples, I find it hard to imagine
how granular the CLA and accelerator blocks are. What I'm interested
in is separating things into special character devices when they
refer to units that you want to manage separately in userspace.
If you have e.g. one accelerator for tensor operations and one for
handling gzip, I would very much want to see those have a separate
chardev nodes so a local administrator can give permissions to each
one separately, and have device names that are sensible to the
functionality underneath.
If you have separate accelerators for AES encryption and decryption,
or a large set of identical accelerators that can run concurrently,
those would of course get managed as a single device file.
Most importantly, I don't think a global /dev/cla device node
is a sensible interface from a management perspective as that
would give unprivileged userspace direct control to something
that is essentially arbitrary (or buggy) vendor firmware
with DMA permissions.
>>> Support shared read-write mmap of one or more CLA register pages. Create
>>> a context for every domain covered by the mapping and resolve faults
>>> only while that context owns the domain. Queue unassigned contexts with
>>> the domain scheduler, drop mmap_lock while waiting for assignment and
>>> retry the fault after the context is woken.
>>
>> I still need some time to better understand what this means.
>
> I can probably do a better job of explaining this: The kernel keeps a cla_ctx
> object which represents a single {file description, mm_struct} context that
> wants to use the cla_domain (collection of 1 or more cla_dev). Initially the VMA
> is not populated so when user space tries to access, it will fault to the
> driver. If the cla_domain is assigned to a different cla_ctx, the faulting
> thread is put to sleep until the driver determines that it's the turn of that
> cla_ctx to be assigned the domain. At that point the waiting thread(s) are woken
> and map the devices from the domain to the VMA and return to user space. The
> out-going cla_ctx had it's mappings removed during the reassignment process so
> any user space access will now fault and sleep waiting for assignment.
Got it, thanks
>> Does a CPU have multiple concurrently running contexts?
>
> The HW only has a single HW context, hence the timesliced assignment approach
> described above (assuming there is more than 1 concurrent user).
Sorry, I think we have a clash of terminology. I meant whether
a CPU can start multiple operations on one or more accelerator
in a single cla_domain, and have each of those operate at the
same time while the hardware is asynchronously processing them
in parallel.
It sounds like a single accelerator has one register to wait for
completion and can only have single operation in progress at any
time, but if multiple accelerators are in the same cla_ctx,
can a single thread start an operation in each one before waiting
for the first to complete?
>> Is a
>> user process able to starve the allocation of other processes
>> by just requesting a lot of them?
>
> In the current code, a process can theoretically create the same number of
> cla_ctx as the number of file descriptors it can open(). It would then need to
> mmap and access to get into the queue to be assigned the domain. I see it as
> similar to threads; if process A creates 10 threads and process B creates 1
> thread, then in the long run (ignoring cgroups et al) you'd expect A to get
> 10/11th of the CPU time (IIUC?).
>
> Do you think this consitutes a DoS?
I need to think about it more. Probably not a DoS, but if the resource
is managed like CPU timeslices, I wonder whether you'd have to also
consider things like realtime tasks or priority inversion.
>>> + wait_event_interruptible(ctx->waitq,
>>> + READ_ONCE(domain->assigned_ctx) == ctx ||
>>> + cla_ctx_is_dying(ctx) ||
>>> + READ_ONCE(domain->broken));
>>
>> If you call wait_event_interruptible(), you have to check the return
>> code and deal with it being interrupted.
>
> I believe this is already correct - I'm reeturning VM_FAULT_RETRY
> unconditionally at this point, which is also the correct return code if we get
> interrupted. This unwinds to arm64's do_page_fault() which then notices and
> handles the fault:
>
> /* Quick path to respond to signals */
> if (fault_signal_pending(fault, regs)) {
> if (!user_mode(regs))
> goto no_context;
> return 0;
> }
Ok, makes sense.
> I believe GUP and other handle_mm_fault() callers have similar logic.
>
> I'll add a comment to make that clear.
Thanks
Arnd
next prev parent reply other threads:[~2026-07-17 15:32 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-17 10:47 [RFC PATCH v1 0/8] Arm Core Local Accelerator Driver Ryan Roberts
2026-07-17 10:47 ` [RFC PATCH v1 1/8] misc/arm-cla: Add driver skeleton and documentation Ryan Roberts
2026-07-17 13:49 ` Arnd Bergmann
2026-07-17 15:44 ` Ryan Roberts
2026-07-17 16:10 ` Arnd Bergmann
2026-07-17 10:47 ` [RFC PATCH v1 2/8] misc/arm-cla: Add launch operation helpers Ryan Roberts
2026-07-17 12:16 ` Arnd Bergmann
2026-07-17 10:47 ` [RFC PATCH v1 3/8] misc/arm-cla: Probe firmware-described devices Ryan Roberts
2026-07-17 12:25 ` Arnd Bergmann
2026-07-17 12:36 ` Ryan Roberts
2026-07-17 10:47 ` [RFC PATCH v1 4/8] misc/arm-cla: Initialize devices on CPU bringup Ryan Roberts
2026-07-17 10:47 ` [RFC PATCH v1 5/8] misc/arm-cla: Accelerator context save and restore Ryan Roberts
2026-07-17 10:47 ` [RFC PATCH v1 6/8] misc/arm-cla: Set up memory translation context Ryan Roberts
2026-07-17 10:47 ` [RFC PATCH v1 7/8] misc/arm-cla: Manage domain contexts Ryan Roberts
2026-07-17 10:47 ` [RFC PATCH v1 8/8] misc/arm-cla: Add userspace interface Ryan Roberts
2026-07-17 12:54 ` Arnd Bergmann
2026-07-17 14:35 ` Ryan Roberts
2026-07-17 15:31 ` Arnd Bergmann [this message]
2026-07-17 16:21 ` Ryan Roberts
2026-07-17 20:11 ` Arnd Bergmann
2026-07-17 11:33 ` [RFC PATCH v1 0/8] Arm Core Local Accelerator Driver Will Deacon
2026-07-17 12:09 ` Marc Zyngier
2026-07-17 12:33 ` Ryan Roberts
2026-07-17 12:30 ` Ryan Roberts
2026-07-17 13:32 ` Jason Gunthorpe
2026-07-17 13:42 ` Ryan Roberts
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=59d0c4d5-af53-410d-9bf8-8dd2ba17f697@app.fastmail.com \
--to=arnd@arndb.de \
--cc=catalin.marinas@arm.com \
--cc=corbet@lwn.net \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=jpb@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=ogabbay@kernel.org \
--cc=ryan.roberts@arm.com \
--cc=will@kernel.org \
/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