Skip to content

[26.04_linux-nvidia] NVIDIA: SAUCE: ARM FF-A EC services on GB10 — DEN0077A v1.3 support - #575

Open
abhsahu wants to merge 10 commits into
NVIDIA:26.04_linux-nvidiafrom
abhsahu:nvidia_ffa_ec_secure_servies_07_Sep_2026
Open

[26.04_linux-nvidia] NVIDIA: SAUCE: ARM FF-A EC services on GB10 — DEN0077A v1.3 support#575
abhsahu wants to merge 10 commits into
NVIDIA:26.04_linux-nvidiafrom
abhsahu:nvidia_ffa_ec_secure_servies_07_Sep_2026

Conversation

@abhsahu

@abhsahu abhsahu commented Sep 7, 2026

Copy link
Copy Markdown

Overview

This series brings the nvidia-ffa-ec driver in line with the released
ARM DEN0077A v1.3 firmware framework spec, fixes the boot ordering that
kept early ACPI consumers from finding the FF-A bridge, decouples
firmware-declared AML cookies from the FF-A framework's physical
notification-ID space, and papers over transient SPMC busy responses
during early boot.

Motivation

GB10 platforms ship EC services behind the ARM FF-A bridge. Two
firmware generations coexist in the field:

  • MSFT000C — the older OpenDevicePartnership draft spec ([1]).
  • ARML0002 — the released DEN0077A v1.3 spec ([2]) sections 18.7 and
    18.8, with inter-partition setup, the FFH offset-2 packet layout,
    and the cookie / notification-id split. DEN0077A v1.3 also updated
    the outer FF-A Device Properties GUID between ALP3 (daffd814-...)
    and ALP4 (c08c3233-...) drafts. The OpenDevicePartnership released-
    spec companion covering the EC-services layer is at [3]. The FFH
    operation-region layout, which the two packet formats key off, is
    defined by ARM DEN0048D "Arm Functional Fixed Hardware Specification
    (FFH)" v1.3 section 2.3 ([4]).

Before this series the driver only handled MSFT000C, and it probed too
late in boot for consumers that declare _DEP on the FF-A bridge and
evaluate FFH regions from _STA / _INI / _DSM during the initial ACPI
namespace scan. Those consumers ended up permanently missing (battery
absent, lid not detected, UCSI dead) until a manual rebind.

References

[1] OpenDevicePartnership draft EC-services overview (MSFT000C):
https://github.com/OpenDevicePartnership/documentation/blob/0f7b6bad77a3eb07b66b66d0e3af718db2ec1c24/bookshelf/Shelf%204%20Specifications/EC%20Interface/src/secure-ec-services-overview.md

[2] Arm Firmware Framework for Arm A-profile (DEN0077A v1.3):
https://support.arm.com/documentation/den0077

[3] OpenDevicePartnership released-spec EC-services overview
(odp-embedded-controller):
https://github.com/OpenDevicePartnership/odp-embedded-controller/blob/main/docs/src/specs/ec_interface/secure-ec-services-overview.md

[4] Arm Functional Fixed Hardware Specification (DEN0048D v1.3):
https://developer.arm.com/documentation/den0048

@nirmoy nirmoy added the help wanted Extra attention is needed label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

PR Validation Report

Patchscan ✅ No Missing Fixes

All cherry-picked commits checked — no missing upstream fixes found.

PR Lint ❌ Errors found

Details
Checking 10 commits...

Cherry-pick digest:
┌──────────────┬──────────────────────────────────────────────────────────────────┬────────────┬─────────┬───────────────────────────┐
│ Local        │ Referenced upstream / Patch subject                              │ Patch-ID   │ Subject │ SoB chain                 │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ a76283eece6f │ [SAUCE] platform/arm64: nvidia-ffa-ec: suppress driver bind/unbi │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ 62c56d6df7c7 │ [SAUCE] platform/arm64: nvidia-ffa-ec: retry ffa send on transie │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ 0167a0eef634 │ [SAUCE] platform/arm64: nvidia-ffa-ec: add oem ffa service uuids │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ efef9d929b5d │ [SAUCE] platform/arm64: nvidia-ffa-ec: inline uuids in ffa_devic │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ 11799ebeb01e │ [SAUCE] platform/arm64: nvidia-ffa-ec: remove acpi rescan workar │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ 9c8eb0e45aea │ [SAUCE] platform/arm64: nvidia-ffa-ec: map aml cookies to physic │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ 712b18073281 │ [SAUCE] acpi: battery: clear consumer dependencies after success │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ 1ac565bc2ac2 │ [SAUCE] acpi: scan: add ff-a bridge hids to acpi_ignore_dep_ids[ │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ d57cba359a9f │ [SAUCE] acpi / firmware / platform: probe arm ff-a early before  │ N/A        │ N/A     │ abhsahu                   │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ f8e0f46081e1 │ [SAUCE] platform/arm64: nvidia-ffa-ec: add arml0002 / den0077a r │ N/A        │ N/A     │ abhsahu                   │
└──────────────┴──────────────────────────────────────────────────────────────────┴────────────┴─────────┴───────────────────────────┘

Lint: all checks passed.

PR metadata:
E: PR targets 26.04_linux-nvidia but body has no https://bugs.launchpad.net/... link

@nirmoy

nirmoy commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

BaseOS Kernel Review

Warning

⚠️ Review needs attention

NVIDIA FF-A EC has two removal-time UAF risks: notifier callbacks may use freed cb_data, and FFH sends may access a released ffa_device. COMPILE_TEST can also allow a built-in driver with a modular transport, breaking early probe.

Findings: Critical 0 · High 2 · Medium 2 · Low 5

🔍 Review artifacts

📦 Kernel deb builds — 🟢 2/2 passed

Note

Build reports and debs are retained for 10 days after the PR closes.

Review metadata
  • Reviewed head: a76283eece6f
  • Overall status: attention needed
  • Architectures: 2/2 successful

This comment is maintained by BaseOS Reviewer and updated when the GitHub watcher publishes a newer review.

@jamieNguyenNVIDIA

Copy link
Copy Markdown
Collaborator

Hi @abhsahu,

  1. What is the current read on the specs that this work is based on? Are they any closer to getting ratified now than they were when this work first began? Should we assume nvidia-ffa-ec will need to be carried as SAUCE perpetually?

  2. No action needed yet, but once the review comments here are sorted out, we will need this targeted at 26.04_linux-nvidia-bos as well.

@clsotog

clsotog commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Some findings with Codex

  1. [P1] drivers/platform/arm64/nvidia-ffa-ec.c:513 leaves ARML0002 X5/X6 zero during notification registration. The inter-partition setup layout expects the service UUID in X5/X6 as well as X7/X8, and the ODP text explicitly says the service being registered is stored in X5/X6. This can make ARML0002 notification setup bind to a null/unknown service or fail. Populate ffa_data.data[1] and [2] with svc_uuid too, unless the firmware intentionally diverges from the spec.
  2. [P1] drivers/platform/arm64/nvidia-ffa-ec.c:775 truncates MSFT000C responses to the request length. The old handler copied back the full FFH rawdata buffer because AML requests can be short, for example a one-byte command, while responses can be much larger. Passing pkt->length as both input and output length will leave most legacy response bytes stale. Keep pkt->length for input, but copy back rawdata_buflen bounded by sizeof(struct ffa_send_direct_data2::data).

@jamieNguyenNVIDIA

Copy link
Copy Markdown
Collaborator

In addition to what Carol reported, here is a pass over all nine commits
against f8c1047, with CONFIG_ARM_FFA_TRANSPORT=y and
CONFIG_NVIDIA_FFA_EC=y per debian.nvidia/config/annotations.

One piece of history behind Carol's second finding: the code it replaces is
itself the fix for that exact problem. 6b00b7f ("NVIDIA: SAUCE: Fix FFH
data response length", LP: #2118357, Fixes: d0038ee) says "While copying
the data back into ACPI FFH packet, it uses the request length. The response
data can be larger than request length." So this looks like a reintroduction
of LP: #2118357 rather than a new bug, which may matter for how it is
tracked. That commit also anticipated this series: "When we move to latest
revision, then we need to copy all data bytes for both request and response."

One correction to the automated review: the use-after-free it flags in
nvidia_ffh_do_ffa_send() is not introduced here. f8c1047 already dropped
nvidia_ffa_lock before dereferencing ec_dev. See item 5.

Items 1 to 3 are one design question rather than three.

  1. 3d376f4 - the early-scanned bridge gets no parent device

acpi_get_devices() runs before acpi_root is assigned (scan.c:2891), so
acpi_find_parent_acpi_dev() finds no ancestor acpi_device and
acpi_init_device_object() sets device->dev.parent = NULL (scan.c:1815). The
full scan does not fix this up - acpi_bus_check_add() returns early on an
existing acpi_device. Three consequences:

  • ARML0002:00 lands at the top of /sys/devices/, and is unreachable by
    acpi_dev_for_each_child() from an ancestor.
  • acpi_device_get_busid() takes its root shortcut and sets
    pnp.bus_id = "ACPI" instead of "FFA0", which is what
    acpi_fwnode_get_name() and /proc/acpi/wakeup report.
  • acpi_init_coherency() loses ancestor _CCA inheritance; arm64 selects
    ACPI_CCA_REQUIRED (arch/arm64/Kconfig:5), so a bridge with no _CCA of its
    own gets coherent_dma = 0.

Would creating acpi_root first, or scanning _SB before the bridge, avoid
this?

  1. bdfa342 - would acpi_honor_dep_ids[] be the better list

The message says "Only ACPI scan-handlers are hooked into the enumeration
loop's acpi_dev_clear_dependencies() path". That function is exported and
called from around twenty drivers, mostly plain platform drivers -
drivers/acpi/ec.c registers a platform_driver (ec.c:2269) and calls it from
acpi_ec_probe() (ec.c:1749). 5195c03 in this series does the same for
the battery.

Existing acpi_ignore_dep_ids[] entries were all added because Linux has no
driver for the supplier (2ef33ee, 9272e97, fa153b7). The
opposite move has recent precedent: c28527c ("ACPI: scan: Honor _DEP for
Intel CVS devices") moved INTC10DE/INTC10E0 into acpi_honor_dep_ids[]
precisely because the driver exists and calls acpi_dev_clear_dependencies()
at the end of probe. acpi_honor_dep_ids[] already carries ARMH0003.

Would honor_dep_ids plus that call in nvidia_ffa_probe() work here? If so
the early enumeration pass and the initcall changes both become unnecessary,
which also removes two unguarded full-namespace walks from core ACPI code
that every architecture pays for at boot.

  1. 3d376f4 - the arch_initcall move was reverted upstream

0e0546e moved ffa_init() to rootfs_initcall; cc7e8f2 reverted it as
patch 1 of Sudeep Holla's "Move core init to platform driver probe" series:

"That initcall ordering workaround is not a proper solution and potentially
conflicts with pKVM FF-A proxy requirement."

That series landed e659fc8 and 3acc80a, and this tree already has
it - the diff context shows ffa_probe(struct platform_device *pdev). Is this
re-applying the pattern on top of its replacement?

It also has no effect where ffa_probe() returns -EPROBE_DEFER under pKVM,
since deferred probe waits for late_initcall (drivers/base/dd.c:376). Note
the automated review raised this hunk on modular-build grounds; those do not
apply to the nvidia and nvidia-64k flavours, which build both symbols in, but
the pKVM case applies to a built-in kernel.

  1. 5c61f90 - notification IDs are never released

nvidia_ffa_next_notif_id only increments, is not rolled back on the error
path, and is not returned by nvidia_ffa_remove_notifications(). The message
says relinquish "is sufficient for teardown", but that releases the FFA
registration, not the counter. Built-in means no module unload, so after a
cumulative 64 notifications across all bind cycles every probe fails -ENOSPC
until reboot. Worth a bitmap instead?

  1. 5c61f90 - the guard added earlier in the series is removed

36b0c01 added guard(mutex)(&nvidia_ffa_lock) to nvidia_ffh_do_ffa_send()
with a comment naming the use-after-free it prevents; this commit replaces it
with a lock covering only the list walk. That matches f8c1047 so it is
not a net regression, but the commit message does not mention it. Was it
deliberate? 5cf2f49 then adds a usleep_range() retry inside the same
unlocked region.

  1. Commit message and comment accuracy

36b0c01 "released-spec support" / "ARM has released the framework":
Arm's version index for den0077 lists only ALP0 to ALP5 for v1.3, latest ALP5
(2026-07-14). The last released FF-A is 1.2 REL0, whose appendix stops at
18.6. Could this say "v1.3 ALP5"?

36b0c01: both FFH offsets attributed to DEN0048D section 2.3, which
defines 0x0/0x1/0x2 and reserves the rest. Offset 4 is from the ODP draft.

36b0c01: the packet comment says recvid is "(OS populates)" and status
carries a framework error code; nothing touches recvid and status is set to
0 or 1. The MSFT000C URL in the comment also 404s now.

3d376f4: "from rootfs_initcall (level 6)" - the parent has
module_init(ffa_init), which is device_initcall; rootfs_initcall is level 5.

5195c03: beyond the acpi_battery_add() and acpi_driver points already
raised, "ACPI scan handlers (e.g. the FFA provider) call it explicitly in
their .attach" is also wrong - nvidia-ffa-ec.c is a platform_driver, not a
scan handler, and never calls acpi_dev_clear_dependencies().

c18cae7: ffa_setup_partitions() does register a device for every
partition regardless of id_table. What is missing is the bind.

  1. Smaller items

36b0c01: struct nvidia_ec_ffa_packet keeps its name for the new 144-byte
layout while the old one is renamed, so offsetof(..., rawdata) silently goes
18 to 32 and ->status u8 to u64. Would _arml0002 be safer for backports?

3d376f4: acpi_scan_one_handle_cb() discards acpi_bus_scan()'s return and
both acpi_get_devices() statuses, so enumeration failure is silent; the
helper could be __init. The diff also has an unrelated hunk removing the
blank line between ffa_probe() and ffa_remove().

5cf2f49: only -EBUSY is retried, while FFA_RET_RETRY and
FFA_RET_NOT_READY both map to -EAGAIN and fall through. Deliberate for the
GB10 secure partition?

b636ed3 leaves a double blank line in nvidia_ffa_ec_service_probe().

Checked and found clean

9d43c55 is a true no-op - the compiled .rodata is identical before and
after. c18cae7 adds eight UUIDs with no duplicates. Sleeping in
nvidia_ffh_do_ffa_send() is safe: ACPICA exits the interpreter for
non-default address spaces and sync_send_receive2() already sleeps on
FFA_YIELD. The ARML0002 FFH packet layout matches DEN0048D 2.3.1.2 and its
32 + 8*N rule, and NVIDIA_EC_NOTIF_TUPLE() is self-consistent and correctly
parenthesised - note this was checked for internal consistency only, not
against DEN0077A, so it says nothing either way about Carol's first finding
on the X5/X6 assignment. Initcall link order is correct (drivers/firmware/
before drivers/platform/, ffa-core.o before ffa-module.o). Re-entering
ACPICA from the acpi_get_devices() callback is safe (ACPI_NS_WALK_UNLOCK).
The battery acpi_dev_clear_dependencies() call cannot double-decrement on
rebind. The missing pkg3->package.count check is pre-existing. Both arm64
debs build clean.

… released-spec support

Background
==========
When DGX Spark launched, the ARM FF-A EC services specification was
still a draft maintained in the OpenDevicePartnership documentation
repository. The initial deployed firmware tracks that draft and
identifies its FF-A bridge with the ACPI HID MSFT000C. The pinned
draft is preserved at

  https://github.com/OpenDevicePartnership/documentation/blob/0f7b6bad77a3eb07b66b66d0e3af718db2ec1c24/bookshelf/Shelf%204%20Specifications/EC%20Interface/src/secure-ec-services-overview.md

Since then ARM has released the framework. FF-A itself is now the
"Arm Firmware Framework for Arm A-profile" v1.3 ALP5 (DEN0077A), the
FFH operation-region interface it rides on is the "Arm Functional
Fixed Hardware Specification" (DEN0048D), and the EC secure-services
layer has moved into a companion repository at

  https://github.com/OpenDevicePartnership/odp-embedded-controller/blob/a018ff0ca21ac95c84b3d3559bda0d11962a0344/docs/src/specs/ec_interface/secure-ec-services-overview.md

Platforms that ship this released stack identify their FF-A bridge
with the ACPI HID ARML0002.

Extend the driver to support the released stack (ACPI HID ARML0002) in
addition to the existing draft-spec support (ACPI HID MSFT000C). The
two specs share most of the driver machinery but differ in packet
layout, FFH operation-region offset, notification encoding, and
inter-partition setup.

Key changes in this patch:

  * Add a boolean ffa_ec_legacy_spec that is populated from the ACPI
    device-ID match at probe time:
      MSFT000C -> true  (legacy draft spec)
      ARML0002 -> false (released spec)
    All spec-dependent branches key off this flag.

  * New packet structure for ARML0002 (144 bytes = 18 x 8-byte
    registers X0..X17) alongside the existing 18-byte MSFT000C packet.
    Fields:
      u64 status   bytes  0..7   driver-side status: AML writes 0;
                                 FFH handler sets 1 on error
      u64 recvid   bytes  8..15  receiver endpoint ID, populated by
                                 the FFA framework path
      u8  uuid[16] bytes 16..31  service UUID
      u8  rawdata  bytes 32+     payload mapped to X4..X17

  * Two FFH operation-region handlers, dispatched on info->offset:
      0x02 -> nvidia_ffh_handler_arml0002() (released), operation-
              region defined by ARM DEN0048D "Arm Functional Fixed
              Hardware Specification (FFH)" v1.3 section 2.3
      0x04 -> nvidia_ffh_handler_msft000c() (draft), operation-region
              defined by the OpenDevicePartnership secure-ec-services
              draft, not by DEN0048D
    Both delegate to a shared nvidia_ffh_do_ffa_send() helper that
    performs UUID lookup, FFA send, and response copy. For MSFT000C,
    the response copy uses ffh_copy_len = min(rawdata_buflen,
    sizeof_field(struct ffa_send_direct_data2, data)) so the full
    response register file is copied back and legacy consumers do not
    see stale bytes past pkt->length.

  * Notification map property parsing handles both revisions:
      MSFT000C: pkg4_rev = 1
      ARML0002: pkg4_rev = 0x00010000 (packed v1.0 per DEN0077A v1.3
                                       section 18.8.2)

  * Released-spec notification registration uses the inter-partition
    setup protocol (UUID e474d87e-...) per DEN0077A v1.3 section 18.7
    with FFA_MSG_SEND_DIRECT_REQ2 and the encoded 64-bit notification
    tuple (cookie << 32 | (notif_id & 0x1FF) << 23 | per_vCPU_flag).

  * Consolidate MSFT000C notify and ARML0002 setup handling into a
    single nvidia_ffa_control_service_driver whose id_table lists both
    UUIDs; the FFA framework matches whichever the platform exposes.
    The shared probe stores it in control_ffa_dev and guards against
    duplicate registration by returning -EBUSY instead of silently
    overwriting the global pointer.

  * Accept either FF-A Device Properties GUID as the outer _DSD
    identifier. DEN0077A v1.3 renamed the identifier between drafts:

      ALP3 and earlier: daffd814-6eba-4d8c-8a91-bc9bbf4aa301
                        (classic ACPI 6.x Device Properties GUID)
      ALP4 and later:   c08c3233-b316-4723-a9d7-e21b7ac0fb6a
                        (FF-A specific Device Properties GUID)

    Firmware in the field ships either one depending on which draft of
    the spec the BIOS was locked against; accepting both keeps a single
    Linux driver working across revisions.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
…e consumer enumeration

Several ACPI consumer devices on ARM platforms declare _DEP on the FF-A
bridge device (HID ARML0002 for the DEN0077A released spec or MSFT000C
for the OpenDevicePartnership draft spec) and call into \_SB.FFA0.FFAC
and \_SB.FFA0.AVAL() from their _STA / _DSM methods during ACPI device
enumeration. If the FF-A core driver or the NVIDIA EC vendor driver are
not bound at that point, the FFH operation-region handler is not yet
installed, every FFAC store hits AE_NOT_EXIST, and the consumer method
aborts, leaving those devices in a permanently broken state.

Four coordinated changes make the FF-A stack available before consumer
ACPI devices are enumerated:

1. drivers/acpi/scan.c
   In acpi_scan_init(), before the full namespace scan
   (acpi_bus_scan(ACPI_ROOT_OBJECT)), walk the namespace for both
   ARML0002 and MSFT000C and enumerate those devices first using
   acpi_get_devices(). A new helper acpi_scan_one_handle_cb() bridges
   the acpi_get_devices callback into acpi_bus_scan(); it logs any
   acpi_bus_scan() failure with acpi_handle_warn() so a silent
   enumeration miss is visible, and returns AE_CTRL_TERMINATE
   unconditionally because a given platform exposes at most one node
   per bridge HID.

2. drivers/firmware/arm_ffa/bus.c
   Move the FF-A bus registration from subsys_initcall (level 4) to
   arch_initcall (level 3) so the bus exists before any driver
   registers FFA devices on it.

3. drivers/firmware/arm_ffa/driver.c
   Move the FF-A core driver initialization from module_init
   (device_initcall, level 6) to arch_initcall (level 3) so the FFA
   partition list and FFA devices are populated before acpi_init runs
   at subsys_initcall (level 4).

4. drivers/platform/arm64/nvidia-ffa-ec.c
   Move the NVIDIA EC vendor driver registration from module_init
   (device_initcall, level 6) to arch_initcall (level 3) for the same
   reason. NVIDIA_FFA_EC is switched from tristate to bool (with
   depends on ARM_FFA_TRANSPORT=y) so that arch_initcall really runs
   at level 3 -- for a module it would only be a load-time
   initializer, not a level-3 built-in initializer, so with
   CONFIG_NVIDIA_FFA_EC=m or CONFIG_ARM_FFA_TRANSPORT=m the early ACPI
   scan would enumerate the bridge while the FFH handler is absent.

With these changes, when acpi_scan_init() enumerates ARML0002 /
MSFT000C early, the FF-A bus + core + NVIDIA EC vendor driver are
already in place, the FFH custom-offset handler is installed, and
subsequent enumeration of consumer ACPI devices succeeds on the first
invocation of their _STA / _DSM methods.

Note on upstream direction. An earlier upstream change,
commit 0e0546e ("firmware: arm_ffa: Change initcall level of
ffa_init() to rootfs_initcall"), moved ffa_init() to rootfs_initcall
to work around an IMA / TPM boot-ordering issue. That workaround was
withdrawn by commit cc7e8f2 ("Revert "firmware: arm_ffa: Change
initcall level of ffa_init() to rootfs_initcall"") once IMA started
retrying its TPM lookup at late_initcall_sync, and ffa_init() went
back to module_init upstream by design. Our concern here is
different: the FFH region handler must be installed before consumer
ACPI devices under \_SB.FFA0 have their _STA / _DSM evaluated during
the initial acpi_bus_scan(). Neither module_init (level 6) nor
rootfs_initcall (level 5) runs before acpi_init (subsys_initcall,
level 4), so we move ffa_init() (and the
FF-A bus + our vendor driver) to arch_initcall (level 3), the earliest
level at which the transport, bus, and vendor driver can all be
present before namespace enumeration. This is a SAUCE change contained
to the downstream tree; a future upstream submission of this driver in
a vendor-neutral form is expected to have a different approach.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
ARM FF-A EC service consumers declare _DEP on the FF-A bridge device,
with HID ARML0002 for the DEN0077A released spec, or MSFT000C for the
OpenDevicePartnership draft spec. Without ignoring these HIDs the
consumers accumulate dep_unmet >= 1 forever and never probe, for two
reasons:

1. The FF-A bridge driver is a plain platform driver, not an ACPI
   scan-handler. Only ACPI scan-handlers are hooked into the
   enumeration loop's acpi_dev_clear_dependencies() path, so when the
   bridge driver successfully probes nothing decrements the dep_unmet
   counter on its consumers.

2. The companion patch in this series moves the FF-A bus, core, and
   NVIDIA EC vendor drivers to arch_initcall and enumerates the
   ARML0002 / MSFT000C platform devices before the namespace scan. At
   that point no consumer ACPI device has been scanned yet, so even
   an explicit acpi_dev_clear_dependencies() call from the bridge
   would walk an empty consumer list. The dep entries on the consumers
   are created later, during acpi_scan_add_dep(), with no remaining
   opportunity to clear them.

The clean fix is to skip the _DEP entry creation entirely. Add both
HIDs to acpi_ignore_dep_ids[] so the FF-A bridge dep is not tracked,
and consumers can probe as soon as their other dependencies (if any)
are satisfied.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
…ssful probe

Some platforms declare _DEP on a battery device for unrelated
consumers. On those systems the consumer driver's probe keeps
returning -EPROBE_DEFER because adev->dep_unmet for the consumer
never drops to zero.

acpi_dev_clear_dependencies() is what walks the consumer list and
decrements their dep_unmet counters. ACPI scan handlers (e.g. the FFA
provider) call it explicitly in their .attach. This battery driver is
registered as a platform_driver whose .probe path does not do that,
so a successful battery probe currently leaves consumer dependencies
pending forever.

Call acpi_dev_clear_dependencies() at the end of acpi_battery_probe()
when initialization has succeeded, so any device that legitimately
depends on the battery can finish enumerating.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
…ical FFA notification IDs

The previous implementation conflated the AML cookie value (read from
the ACPI _DSD "arm-arml0002-ffa-ntf-bind" property) with the physical
FFA notification ID (the 0..63 bitmap position registered with the FFA
framework via notify_request).

This worked accidentally on GB10 platforms where every AML cookie
value happened to be < 64, but it is wrong in general:

  * AML cookies are u32 values defined by firmware and can exceed the
    FFA framework's 6-bit notification ID space.
  * Two services that happen to use the same low cookie value would
    collide in the FFA framework, causing one of the notify_request
    calls to fail.
  * When a notification fires, the FFA framework only knows the
    physical notification ID; the AML _DSM Function 1 (notify) expects
    the cookie that was declared in _DSD.

Decouple the two:

  * Introduce struct nvidia_ec_notification { u32 cookie; u8 notif_id; }
    and replace the bare u8 notification_id[] array with an array of
    these pairs.

  * Add a global DECLARE_BITMAP(nvidia_ffa_notif_id_map,
    NVIDIA_FFA_MAX_NOTIFICATIONS) used to hand out physical
    notification IDs for ARML0002 across all EC services. A free bit
    is picked with find_first_zero_bit() and set with set_bit() under
    nvidia_ffa_lock; the bit is released with clear_bit() at every
    notify_relinquish() site (create-time rollback and remove) so
    repeated probe/remove cycles do not exhaust the 0..63 range.

    For MSFT000C the older draft-spec firmware rejects NOTIFY_SETUP
    when the upper and lower halves of X8 differ, so the cookie itself
    is used as the physical notif_id and the bitmap is bypassed; the
    AML-declared cookie must fit in the FFA framework's 6-bit
    notification ID space.

  * In nvidia_ffa_create_notifications(): for ARML0002 allocate the
    next free physical notif_id from the bitmap under the lock; for
    MSFT000C set notif_id = cookie. Store both the cookie (from _DSD)
    and the assigned notif_id in the device's notifications[] array,
    and pass both to the setup helpers.

  * Update nvidia_ffa_notification_setup_msft000c() and
    nvidia_ffa_notification_setup_arml0002() to encode cookie in the
    upper 32 bits and physical notif_id in the lower bits of the
    notification-setup register field (X8 for MSFT000C, X11 for
    ARML0002 with the spec's bits[31:23] placement).

Refcount the per-service state so lookup + use in the FFH send helper
and the notification callback can straddle nvidia_ffa_lock safely.
The mutex cannot be held across sync_send_receive2() or across
acpi_evaluate_dsm() because those paths enter the SPMC / AML on the
same CPU and can re-enter our own FFH region handler; that would
deadlock on the same non-recursive mutex. Instead:

  * Add struct kref refcnt to nvidia_ec_ffa_device with a kfree-based
    release, and allocate with kzalloc/kfree instead of devm_kzalloc
    so the lifetime is decoupled from the ffa_device devm resource
    ordering.

  * kref_init at probe before publishing on the global list; drop this
    reference in nvidia_ffa_ec_service_remove() after list_del.

  * nvidia_ffh_do_ffa_send(): kref_get inside the list walk under
    nvidia_ffa_lock, drop the mutex, do the FFA send, and kref_put at
    return via a common out_put label. This closes the UAF window
    where nvidia_ffa_ec_service_remove() could free ec_dev between
    the mutex drop and the sync_send_receive2() dereference.

  * Pass ec_ffa_dev as the notify_request cb_data (instead of
    ffa_dev), take a reference per successful notify_request in
    nvidia_ffa_create_notifications(), and drop it per
    notify_relinquish() in nvidia_ffa_remove_notifications(). In
    nvidia_ffa_ec_service_notif_callback() read the cookie directly
    from ec_ffa_dev->notifications[] with no list walk, so an
    in-flight callback the framework snapshotted before we
    relinquished still resolves the cookie correctly.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
…around

The driver previously walked the ACPI bus after each EC service FFA
device probe to rescan consumer ACPI devices. This was needed because
the FFH operation-region handler became available only after the FFA
core driver was bound, by which time consumer ACPI devices' _STA / _DSM
methods (called during the initial acpi_init() namespace scan) had
already failed against \_SB.FFA0.FFAC.

With the FF-A bus, FF-A core driver, and NVIDIA FFA EC vendor driver
all moved to arch_initcall, and with acpi_scan_init() now explicitly
enumerating the ARML0002 platform device first via acpi_get_devices()
before the rest of the namespace scan, the FFH handler is installed
before any consumer ACPI method runs. The rescan walk is no longer
needed.

Remove:
  - nvidia_ffa_rescan_acpi_device()
  - nvidia_get_acpi_id_from_uuid()
  - the acpi_bus_for_each_dev() call site and the local acpi_id

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
…ice_id tables

Each per-service uuid_t constant was used exactly once -- as the
initializer for its slot in an ffa_device_id[] table -- so the
named-constant indirection adds no value and makes adding a new
service a two-step edit. Drop the ten `static const uuid_t
nvidia_ec_*_uuid = UUID_INIT(...)` declarations and put UUID_INIT()
inline in the two id_table arrays (nvidia_ffa_ec_service_ids and
nvidia_ffa_control_service_ids), matching the convention already
used by drivers/tee/optee/ffa_abi.c.

Each table entry now carries a brief comment naming the service it
maps to, so the table itself is self-describing. Adding a new EC
service becomes a single-line addition.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
ACPI tables collected from GB10 platforms issue FFA
requests through the FFAC channel for service UUIDs not yet listed in
nvidia_ffa_ec_service_ids[]. Without a matching entry the FFA bus
framework does not create a per-service device for those partitions,
so the FFH handler cannot route the AML packet to the correct FFA
endpoint and the AML method aborts.

Add eight OEM-specific service UUIDs observed across GB10 platforms.
These cover platform-defined services such as vendor management, EC
RAM access, WMI BIOS attributes, SMBIOS extensions, and other custom
services.

These are grouped under a single "custom UUIDs used by OEM platforms"
comment because their FF-A service definitions are platform-defined
and not published in a public spec.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
…ient failures

Some Secure Partition implementations return a transient failure when
a previous FFA request is still in flight or the SP is not yet ready.
The FFA framework maps these to two Linux errnos: FFA_RET_BUSY becomes
-EBUSY, and both FFA_RET_RETRY and FFA_RET_NOT_READY become -EAGAIN.
This has been observed on GB10 platforms during early ACPI enumeration
when an SP-originated notification arrives while the kernel is
servicing a back-to-back AML FFH write, causing the second write to
fail.

AML cannot retry within a single FFAC field write (the handler must
return success or the whole method aborts), so do the retry here in
the unified FFH send helper.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
…bind

The FF-A bridge and its EC subunits are a single firmware fixture on
the systems this driver targets:

  * They expose the ACPI FFH region handler that consumer ACPI devices
    depend on for the life of the system.
  * There is no way to re-create the FFH region or the EC secure
    services once torn down.
  * The driver is built-in only (NVIDIA_FFA_EC is bool, depends on
    ARM_FFA_TRANSPORT=y), so module removal is not possible.

The only live-runtime way for the .remove callbacks of the three
in-tree drivers (nvidia_ffa_driver, nvidia_ffa_ec_service_driver,
nvidia_ffa_control_service_driver) to fire is via sysfs bind/unbind
attributes -- an admin-triggered debug/test path that has no legitimate
use here.

Set driver.suppress_bind_attrs = true on all three drivers to remove
the sysfs unbind path.

Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
@abhsahu
abhsahu force-pushed the nvidia_ffa_ec_secure_servies_07_Sep_2026 branch from 5cf2f49 to a76283e Compare September 10, 2026 09:34
@abhsahu

abhsahu commented Sep 10, 2026

Copy link
Copy Markdown
Author

Commit 1 — ARML0002 / DEN0077A support

1.1 clsotog — P1-A — X5/X6 zero during notification registration

ARML0002 X5/X6 zero during notification registration; inter-partition setup expects service UUID in X5/X6 as well as X7/X8.

Reply:

Thanks for the catch. Could you point me to the specific spec / ODP-EC text that pins X5/X6 for the OS-originated notification-registration message? TF-A EL3 SPMC treats X5/X6 as opaque payload — spmc_main.c extracts src_id/dst_id from X1 and forwards X2..X17 to the target SP without inspecting X5/X6 — so the "Sender service UUID" contract for an OS-originated call is SP-side, not enforced by the SPMC or by the DEN0077A text I have.


1.2 clsotog — P1-B — pkt->length truncates MSFT000C response

Reply:

Fixed. Restored ffh_copy_len = min(rawdata_buflen, sizeof(ffa_data.data)) in nvidia_ffh_handler_msft000c() — matches the pre-existing 4668e71ed358 pattern that uses the same ffh_copy_len variable name.


1.3 jamieNguyenNVIDIA — 6a — DEN0077 wording ("v1.3 ALP5" not "released")

Reply:

Fixed. Commit message updated to reference "v1.3 ALP5" instead of "released spec".


1.4 jamieNguyenNVIDIA — 6b — FFH offsets attributed to wrong spec

Reply:

Fixed. Dispatcher block comment and commit message split cleanly:

  • FFH offset 2 → DEN0048D §2.3 (ARML0002 / DEN0077A ALP5)
  • FFH offset 4 → OpenDevicePartnership draft (MSFT000C)

1.5 jamieNguyenNVIDIA — 6c — Packet comment inaccuracies, broken URL

Reply:

Fixed. ARML0002 packet block comment rewritten:

  • status: AML writes 0 to indicate "no error yet"; the FFH handler sets 1 on error.
  • recvid: FFA framework populates this; the handler does not touch it.
  • Broken URL removed.

1.6 jamieNguyenNVIDIA — 7a — Struct naming safety for backports

Reply:

Renamed nvidia_ec_ffa_packetnvidia_ec_ffa_packet_arml0002 for symmetry with the existing _msft000c struct.

Keeping the nvidia_ prefix intentionally as a transitional choice — this driver is a placeholder for an upstream-generic FF-A EC-services driver. When submitted upstream in vendor-neutral form we plan to drop the nvidia_ prefix; keeping ARML0002 as the "base" today reduces the eventual rename diff.


Commit 2 — probe FF-A early before consumer enumeration

2.1 boro (AI) — boro-1 (High) — _INI runs before early enumeration

_INI runs in acpi_initialize_objects() before acpi_scan_init() reaches early enumeration.

Reply:

Correct on the ordering. Considered moving the FF-A bridge probe ahead of acpi_initialize_objects() (adds acpi_ffa_bridge_early_scan() plus pulls acpi_platform_init() into acpi_bus_init() before _INI) and it works, but on the GB10 platforms we support today no _INI method touches \_SB.FFA0.FFAC or _AVAL() — all FFH access happens later from _STA / _DSM, which our current placement already handles. Leaving the early scan where it is for this SAUCE series to keep the ACPI core change minimal. If a future GB10 platform starts using _INI to touch FFA regions, we will promote the enumeration hook.


2.2 boro (AI) — boro-2 (High) — arch_initcall on modular driver

Reply:

Fixed. NVIDIA_FFA_EC changed from tristate to bool and now depends on ARM_FFA_TRANSPORT=y (or COMPILE_TEST). Both the FF-A transport and this driver are guaranteed built-in, so arch_initcall behaves as level-3 init as intended.


2.3 jamieNguyenNVIDIA — item 1 / CodeRabbit finding 3 — Early bridge has no parent device

Reply:

Correct — when acpi_get_devices() runs from acpi_scan_init() before the full acpi_bus_scan(ACPI_ROOT_OBJECT), the FF-A bridge acpi_device is attached with a NULL parent instead of being placed under \_SB. Considered two alternatives (delaying enumeration, or scanning \_SB early), both defeated the point of the change or added substantial early work. Concrete side effects on GB10 are cosmetic sysfs placement only; runtime PM / _DEP / removal are unaffected (or covered by companion patches). Leaving as-is.


2.4 jamieNguyenNVIDIA — item 3 — arch_initcall re-applies upstream-reverted pattern

Reply:

Fixed (message extended). To close the loop: not the same pattern — upstream revert cc7e8f21b9f0 undid an IMA/TPM boot-ordering workaround. Our motivation is different: the FFH region handler must be installed before consumer ACPI devices under \_SB.FFA0 have their _STA / _DSM evaluated. Neither module_init (level 6) nor rootfs_initcall (level 5) runs before acpi_init (subsys_initcall, level 4); we land at arch_initcall (level 3). Downstream SAUCE only.


2.5 jamieNguyenNVIDIA — 6d — rootfs_initcall level

Reply:

Fixed. Numbered-list item corrected: rootfs_initcall is level 5. Baseline updated to module_init (device_initcall, level 6) per the upstream refactor.


2.6 jamieNguyenNVIDIA — 7b — acpi_scan_one_handle_cb() discards return silently

Reply:

Fixed. acpi_scan_one_handle_cb() now logs failures via acpi_handle_warn() and returns AE_CTRL_TERMINATE so the walk stops on error.


Commit 3 — add FF-A bridge HIDs to acpi_ignore_dep_ids[]

3.1 jamieNguyenNVIDIA — item 2 / CodeRabbit finding 1 — acpi_honor_dep_ids[] may be better

Reply:

Thanks for the pointer — honor_dep_ids[] does not work here given the ordering commit 2 introduces, and we have kept acpi_ignore_dep_ids[].

When ARML0002 / MSFT000C are placed in honor_dep_ids[], consumers _DEPing on the bridge get honor_deps = 1, blocking enumeration until dep_unmet drops to zero. acpi_dev_clear_dependencies() (called from the FFA vendor driver's .probe) walks an empty consumer list because commit 2 drives our probe before the full acpi_bus_scan(). So consumers see dep_unmet > 0 forever. I tried this but it didn't help.


Commit 4 — battery: clear consumer dependencies

4.1 boro (AI) — boro-6 — Stale "acpi_driver framework" wording

Reply:

Fixed. Code comment reworded to "platform_driver .probe path"; commit message references acpi_battery_probe().


Commit 5 — map AML cookies to physical FFA notification IDs

5.1 boro (AI) — boro-3 (High) / CodeRabbit finding 2 — UAF in nvidia_ffh_do_ffa_send()

Reply:

Fixed for live runtime via a kref refactor:

  • Added struct kref refcnt on nvidia_ec_ffa_device.
  • Allocation via kzalloc + a release function that runs kfree.
  • kref_get inside the list walk under the mutex; drop the mutex; do the FFA send; kref_put at out_put.

Full-scope mutex isn't used because sync_send_receive2() re-enters our own FFH region handler on the same CPU — full-scope would deadlock on the non-recursive mutex.

Also: commit 10 (suppress driver bind/unbind) removes the only live-runtime path into .remove. The residual shutdown-teardown race is acknowledged and invisible on our platforms.


5.2 boro (AI) — boro-4 (Med) — Callback loses cookie mapping during teardown

Reply:

Fixed for live runtime by changing notify_request's cb_data from ffa_device * to nvidia_ec_ffa_device *, taking a kref per notify_request, and dropping it per notify_relinquish. The callback reads the cookie directly off cb_data — no list walk.

Residual race: the FFA framework's handle_notif_callbacks releases notify_lock before invoking the callback (Sudeep's intentional design — avoids AB/BA deadlock when a callback wants to self-relinquish). Considered a SAUCE framework patch to extend that lock scope; declined because it restricts other in-tree callbacks. With commit 10's suppress_bind_attrs, the residual is shutdown-only.


5.3 boro (AI) — boro-7 (Low) — "bitmap" wording vs monotonic counter

Reply:

Fixed. Comment reworded, and the monotonic counter was ultimately replaced with a real bitmap per jamie's item 4 below.


5.4 jamieNguyenNVIDIA — item 4 — Notification IDs never released; 64-cycle exhaustion

Reply:

Fixed. Replaced the monotonic counter with DECLARE_BITMAP(nvidia_ffa_notif_id_map, NVIDIA_FFA_MAX_NOTIFICATIONS).

  • Allocation: find_first_zero_bit() + set_bit() paired under the mutex to make the "find-then-set" atomic.
  • Release: clear_bit() at every notify_relinquish() site. clear_bit is atomic on its own and does not require the mutex, which also avoids a self-deadlock in the shutdown teardown path (service_remove holds the mutex across remove_notifications).
  • Failure-path release added in both nvidia_ffa_notification_setup() and notify_request's error path in nvidia_ffa_create_notifications().

5.5 jamieNguyenNVIDIA — item 5 — guard(mutex) replaced with narrower coverage

Reply:

Fixed (folded into the kref refactor). Commit message explains: full-scope mutex is impossible because the send helper and callback both re-enter our own FFH region handler; narrow mutex + kref is the correct combination.


Commit 6 — remove ACPI rescan workaround

6.1 boro (AI) — boro-5 (Med) — Rescan may be needed with CONFIG_NVIDIA_FFA_EC=m

Reply:

The module case cannot occur in this series. Commit 2 changes NVIDIA_FFA_EC from tristate to bool and adds depends on ARM_FFA_TRANSPORT=y || COMPILE_TEST, so both the transport and this driver are guaranteed built-in. There is no =m arrangement in which acpi_scan_init() could run before the FFH region handler is installed.


Commit 7 — inline UUIDs in ffa_device_id tables

No review findings.


Commit 8 — add OEM FFA service UUIDs

No review findings.


Commit 9 — retry FFA send on transient failures

9.1 boro (AI) — boro-8 (Low) — Retry off-by-one; stray sleep after last attempt

Reply:

Fixed. Rewrote as a wall-clock timeout loop (same idiom as drivers/char/tpm/tpm_crb_ffa.c and drivers/firmware/arm_scmi/):

deadline = ktime_add_ms(ktime_get(), NVIDIA_FFA_SEND_TIMEOUT_MS);
while (true) {
    ret = ffa_ops->msg_ops->sync_send_receive2(...);
    if (ret != -EBUSY && ret != -EAGAIN)
        break;
    if (ktime_after(ktime_get(), deadline))
        break;
    usleep_range(...);
}

No stray trailing sleep; no off-by-one on the retry count.


9.2 jamieNguyenNVIDIA — 7c — Only -EBUSY retried; FFA_RET_RETRY / FFA_RET_NOT_READY bypass

Reply:

Fixed. Loop retries on both -EBUSY and -EAGAIN. The FFA framework error map confirms the coverage:

  • FFA_RET_BUSY-EBUSY
  • FFA_RET_RETRY / FFA_RET_NOT_READY-EAGAIN

Both transient conditions are now retried within the wall-clock deadline.


Commit 10 — suppress driver bind/unbind (NEW)

Proactive addition motivated by boro-3 / boro-4 residual concerns. No reviewer has flagged this yet; posting the rationale here for completeness.

  • Sets .driver.suppress_bind_attrs = true on nvidia_ffa_driver, nvidia_ffa_ec_service_driver, and nvidia_ffa_control_service_driver.
  • Removes the only live-runtime path into .remove (sysfs unbind).
  • Combined with the built-in-only build (bool, depends on ARM_FFA_TRANSPORT=y), the only remaining trigger for .remove is the shutdown path introduced upstream by commit 16a3622577cd ("firmware: arm_ffa: Tear down driver during shutdown"). At that point the system is going down and any residual teardown-race event is invisible.
  • Idiom used by 443 mainline drivers for the same "single firmware fixture consumers depend on" reason (drivers/soc/tegra/, drivers/soc/qcom/, drivers/firmware/, drivers/iommu/).

Runtime-validated on the target platform: /sys/bus/{arm_ffa,platform}/drivers/nvidia-* show only uevent and per-device symlinks — no bind / unbind attrs.

@abhsahu

abhsahu commented Sep 10, 2026

Copy link
Copy Markdown
Author

Thanks @nirmoy, @jamieNguyenNVIDIA, and @clsotog.

I have addressed all the review comments and updated the PR. I also replied directly to each inline comment with its status.

Regarding the commit message and code comments referencing the commit hash:

Because the Open Device Partnership specification undergoes frequent updates and the external links often break or shift, there isn't a reliable way to reference specific historical versions of the spec. Since this downstream driver must support both older draft revisions and the current spec, I included the exact commit hash instead. This ensures we can reliably correlate the code with the exact state of the spec at that point in time, especially given that the current Open Device Partnership repositories lack release tags.

What is the current read on the specs that this work is based on? Are they any closer to getting ratified now than they were when this work first began?

Since the spec is now incorporated into the standard Arm specification, it is much closer to being finalized.

DGX Spark was the first platform to adopt this specification. However, because the DGX Spark launch schedule did not align directly with the specification development timeline, we had to write the firmware against the draft spec version available at the time.

Should we assume nvidia-ffa-ec will need to be carried as SAUCE perpetually?

One potential resolution is to update the firmware to align with the latest specification and ship it as part of an upcoming OTA release. Since upstream kernels will not support the older draft specification, we will need to continue carrying nvidia-ffa-ec as SAUCE if we are unable to update the firmware to the final specification.

No action needed yet, but once the review comments here are sorted out, we will need this targeted at 26.04_linux-nvidia-bos as well.

Understood. I will send out the backport PR targeting 26.04_linux-nvidia-bos as soon as we conclude the review on this PR.

@clsotog

clsotog commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the fixes.
This is the finding by Codex:
P2 drivers/platform/arm64/nvidia-ffa-ec.c:699 drops the last notification-held reference immediately after notify_relinquish(), but FF-A dispatch can already have snapshotted cb_data and then invokes the callback after releasing notify_lock (handle_notif_callbacks()). That means a notification callback can still run with the ec_ffa_dev pointer passed at line 653 after remove continues to line 928 and frees it. Suppressing bind/unbind lowers exposure, but ACPI/device removal still makes this a UAF. The callback data needs a lifetime that is synchronized with already-snapshotted callbacks, or the FF-A notifier core needs to invoke under a lifetime-safe scheme.

@abhsahu

abhsahu commented Sep 10, 2026

Copy link
Copy Markdown
Author

Thanks for the fixes. This is the finding by Codex: P2 drivers/platform/arm64/nvidia-ffa-ec.c:699 drops the last notification-held reference immediately after notify_relinquish(), but FF-A dispatch can already have snapshotted cb_data and then invokes the callback after releasing notify_lock (handle_notif_callbacks()). That means a notification callback can still run with the ec_ffa_dev pointer passed at line 653 after remove continues to line 928 and frees it. Suppressing bind/unbind lowers exposure, but ACPI/device removal still makes this a UAF. The callback data needs a lifetime that is synchronized with already-snapshotted callbacks, or the FF-A notifier core needs to invoke under a lifetime-safe scheme.

Thanks @clsotog. This is a framework-level race in drivers/firmware/arm_ffa/, not something the nvidia-ffa-ec driver can close on its own.

handle_notif_callbacks() snapshots (cb, cb_data) under notify_lock and then invokes cb() after dropping the lock. __ffa_notify_relinquish() clears the hash-table entry under
notify_lock and returns — but nothing waits for a dispatch that has already snapshotted cb_data to finish. So any driver that frees cb_data-referenced storage after
notify_relinquish() returns is exposed to the UAF, regardless of the local lifetime discipline it uses.

In-tree caller is also affected with this:

  • drivers/tee/optee/ffa_abi.c — cb_data = struct optee *; optee_ffa_remove() calls notify_relinquish() then destroy_workqueue() then kfree(optee). optee is tristate (module-removable), so this is a live-runtime UAF, independent of and predating our series.

The correct fix is in the FF-A notifier core, along the lines of the SRCU notifier chain (kernel/notifier.c: srcu_notifier_chain_unregister() uses synchronize_srcu() for exactly
this drain), netfilter's nf_hook_entries_free() (call_rcu), or free_irq() (synchronize_irq()). Concretely: wrap the snapshot + invoke sequence in handle_notif_callbacks() /
handle_fwk_notif_callbacks() with srcu_read_lock/srcu_read_unlock, and add synchronize_srcu() in __ffa_notify_relinquish() after write_unlock(&notify_lock). After that,
notify_relinquish() gains the contract "no callback for this (dev, notify_id) is executing or will begin executing" and freeing cb_data-owned storage becomes safe for all callers.

Given this issue is not specific to this patch series and needs changes in core layer. So, I will follow-up regarding this separately in upstream.

@jamieNguyenNVIDIA

Copy link
Copy Markdown
Collaborator

On the callback use-after-free Carol reported.

The framework analysis is right and the optee example holds up, and an SRCU
drain in the FF-A notifier core looks like the correct long-term fix. Two
things suggest not shipping the window while that lands.

The framework does protect its own objects against its own dispatch -
ffa_remove() drains before it frees:

ffa_notifications_cleanup();	/* destroy_workqueue(): drains */
ffa_partitions_cleanup();	/* only then frees ffa_devices */

That is why cb_data == ffa_dev was safe before this series. This commit
changes cb_data to storage freed when .remove() returns, which the framework
makes no promise about. So the generic race predates the series, but does the
exposure in nvidia-ffa-ec?

And one path reaches the free with dispatch fully live and no sysfs involved:
if nvidia_ffa_create_notifications() fails partway, the unwind puts for j < i
and probe does list_del plus the final kref_put, with the FF-A core running
and its workqueue alive. suppress_bind_attrs does not cover that.

Would a local fix work meanwhile? Passing ffa_dev as cb_data again and
resolving the cookie under nvidia_ffa_lock is memory-safe, since the walk and
the free share that mutex. The original complaint was the fabricated cookie
on a lookup miss; skipping the _DSM there addresses it with no framework
change.

Commit message, 9c8eb0e:

The mutex cannot be held across sync_send_receive2() or across
acpi_evaluate_dsm() because those paths enter the SPMC / AML on the
same CPU and can re-enter our own FFH region handler

Correct for acpi_evaluate_dsm(). Does it hold for sync_send_receive2()?
That chain is ffa_msg_send_direct_req2() -> ffa_msg_send_wait_for_completion()
-> invoke_ffa_fn(), a bare SMC plus an FFA_INTERRUPT / FFA_YIELD loop - no
ACPI, no AML, no locks. The real hazard there looks cross-thread: the
notification kworker takes nvidia_ffa_lock via AML while a holder spins in the
unbounded FFA_RUN loop. Worth splitting the two cases, since this is the
stated rationale for the narrow-lock plus kref design.

Two earlier items look partly applied:

"v1.3 ALP5" was added, but the subject still says "released-spec support" and
the body still has "ARM has released the framework", "this released stack",
"(released spec)", "(released)" and "Released-spec notification registration".

battery.c: the comment and the acpi_battery_probe() reference are fixed, but
the message still says "ACPI scan handlers (e.g. the FFA provider) call it
explicitly in their .attach". nvidia-ffa-ec.c is a platform_driver and never
calls acpi_dev_clear_dependencies().

Smaller:

62c56d6 - on timeout ret is still -EBUSY/-EAGAIN, so the new dev_warn()
is followed by the pre-existing dev_err() for the same event, unratelimited.

62c56d6 - -EAGAIN also covers FFA_RET_NOT_READY, which can persist, so
every FFH access would cost the full 10ms in that state. Intended?

9c8eb0e - the comment above list_add in nvidia_ffa_ec_service_probe()
still describes a callback list walk this commit removes.

d57cba3 - acpi_scan_one_handle_cb() could be __init.

a76283e - the FF-A core's platform driver does not set
suppress_bind_attrs, so /sys/bus/platform/drivers/arm-ffa/unbind still reaches
both nvidia .remove callbacks. That path drains first so the race above is
unaffected, but could the message say sysfs unbind of this driver rather than
the only live-runtime path into .remove?

Checked clean: kref accounting is balanced on every path including both error
unwinds; the bitmap is bounds-checked, paired under the mutex, never
double-cleared, and MSFT000C/ARML0002 cannot collide; suppress_bind_attrs does
take effect on struct ffa_driver; AE_CTRL_TERMINATE is handled cleanly and
acpi_handle_warn() is safe there; the retry loop cannot spin forever; and the
Kconfig bool change does not break allmodconfig, since IS_REACHABLE() supplies
stubs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed pending_review_comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants