* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Andre Przywara @ 2026-03-13 14:39 UTC (permalink / raw)
To: Nirmoy Das, Salman Nabi, vvidwans, sudeep.holla, mark.rutland,
lpieralisi
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <19aeb934-7e36-4f30-8f7f-a8ae74a797f5@nvidia.com>
Hi Nirmoy,
On 3/13/26 10:46, Nirmoy Das wrote:
> Hi Salman and Andre,
>
>
> We found an bug while testing LFA. See below:
>
> On 19.01.26 14:27, Salman Nabi wrote:
>> The Arm Live Firmware Activation (LFA) is a specification [1] to describe
>> activating firmware components without a reboot. Those components
>> (like TF-A's BL31, EDK-II, TF-RMM, secure paylods) would be updated the
>> usual way: via fwupd, FF-A or other secure storage methods, or via some
>> IMPDEF Out-Of-Bound method. The user can then activate this new firmware,
>> at system runtime, without requiring a reboot.
>> The specification covers the SMCCC interface to list and query available
>> components and eventually trigger the activation.
[ .... ]
>> +
>> + update_fw_images_tree();
>> +
>> + /*
>> + * Removing non-valid image directories at the end of an activation.
>> + * We can't remove the sysfs attributes while in the respective
>> + * _store() handler, so have to postpone the list removal to a
>> + * workqueue.
>> + */
>> + INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
>
>
> This can get invoke multiple times so re-initializing a work item that
> may already be queued or running
>
> is unsafe. This should be moved to lfa_init() so it is only called once.
> I suggest:
Ah, good point, thanks for spotting and reporting. Will fold this into
the next post!
Cheers,
Andre
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/
> lfa_fw.c
> index 90727a66e49a5..135358113104c 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -653,7 +653,6 @@ static int update_fw_images_tree(void)
> * _store() handler, so have to postpone the list removal to a
> * workqueue.
> */
> - INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
> queue_work(fw_images_update_wq, &fw_images_update_work);
>
> return 0;
> @@ -680,7 +679,7 @@ static void lfa_notify_handler(acpi_handle handle,
> u32 event, void *data)
> * of all activable and pending images.
> */
> do {
> - /* Reset activable image flag */
> + flush_workqueue(fw_images_update_wq);
> found_activable_image = false;
> list_for_each_entry(attrs, &lfa_fw_images, image_node) {
> if (attrs->fw_seq_id == -1)
> @@ -782,6 +781,8 @@ static int __init lfa_init(void)
> return -ENOMEM;
> }
>
> + INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
> +
> pr_info("Live Firmware Activation: detected v%ld.%ld\n",
> reg.a0 >> 16, reg.a0 & 0xffff);
>
>
> Regards,
>
> Nirmoy
>
>> + queue_work(fw_images_update_wq, &fw_images_update_work);
>> + mutex_unlock(&lfa_lock);
>> +
>> + return ret;
>> +}
>> +
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Chao Gao @ 2026-03-13 13:54 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Williams, Dan J, dave.hansen@linux.intel.com, kas@kernel.org,
Chatre, Reinette, Weiny, Ira, linux-kernel@vger.kernel.org,
mingo@redhat.com, Verma, Vishal L, nik.borisov@suse.com,
seanjc@google.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, Annapurve, Vishal, Duan, Zhenzhong,
sagis@google.com, paulmck@kernel.org, hpa@zytor.com,
tglx@kernel.org, yilun.xu@linux.intel.com, x86@kernel.org,
bp@alien8.de
In-Reply-To: <aa798a36216fb44969a6abedf96ecf1c7d4e95fe.camel@intel.com>
>> > >
>> > > The TDX Module update process consists of several steps as described in
>> > > Intel® Trust Domain Extensions (Intel® TDX) Module Base Architecture
>> > > Specification, Revision 348549-007, Chapter 4.5 "TD-Preserving TDX Module
>> > > Update"
>> > >
>> > > - shut down the old module
>> > > - install the new module
>> > > - global and per-CPU initialization
>> > > - restore state information
>> > >
>> > > Some steps must execute on a single CPU, others must run serially across
>> > > all CPUs, and some can run concurrently on all CPUs. There are also
>> > > ordering requirements between steps, so all CPUs must work in a step-locked
>> > > manner.
>> >
>> > Does the fact that they can run on other CPUs add any synchronization
>> > requirements? If not I'd leave it off.
>>
>> I'm not sure I understand the concern.
>>
>> Lockstep synchronization is needed specifically because we have both multiple
>> CPUs and multiple steps.
>>
>> If updates only required a single CPU, stop_machine() would be sufficient.
>
>The last part "some can run concurrently on all CPUs", how does it affect the
>design? They can run concurrently, but don't have to... So it's a non-
>requirement?
>
>It seems the main argument here is, this thing has lots of complex ordering
>requirements. So we do it lockstep as a simple pattern to bring sanity. It's a
>fine fuzzy argument I think. The way you list the types of requirements all
>specifically has me trying to find the connection between each requirement and
>lockstep. That is where I get lost. If the reader doesn't need to do the work of
>understanding, don't ask them. And if they do, it probably needs to be clearer.
Got it. I'll keep it simple:
The TDX Module update process consists of several steps as described in
Intel® Trust Domain Extensions (Intel® TDX) Module Base Architecture
Specification, Revision 348549-007, Chapter 4.5 "TD-Preserving TDX Module
Update"
- shut down the old module
- install the new module
- global and per-CPU initialization
- restore state information
There are ordering requirements between steps which mandate lockstep
synchronization across all CPUs.
Or the step details might be irrelevant. Perhaps:
TDX module update consists of several steps. Ordering requirements between
steps mandate lockstep synchronization across all CPUs.
>> > > 1. The entire update process must use stop_machine() to synchronize with
>> > > other TDX workloads
>> > > 2. Update steps must be performed in a step-locked manner
>> > >
>> > > To prepare for implementing concrete TDX Module update steps, establish
>> > > the framework by mimicking multi_cpu_stop(), which is a good example of
>> > > performing a multi-step task in step-locked manner.
>> > >
>> >
>> > Offline Chao pointed that Paul suggested this after considering refactoring out
>> > the common code. I think it might still be worth mentioning why you can't use
>> > multi_cpu_stop() directly. I guess there are some differences. what are they.
>>
>> To be clear, Paul didn't actually suggest this approach. His feedback indicated
>> he wasn't concerned about duplicating some of multi_cpu_stop()'s code, i.e., no
>> need to refactor out some common code.
>
>Right, sorry for oversimplifying.
>
>>
>> https://lore.kernel.org/all/a7affba9-0cea-4493-b868-392158b59d83@paulmck-laptop/#t
>>
>> We can't use multi_cpu_stop() directly because it only provides lockstep
>> execution for its own infrastructure, not for the function it runs. If we
>> passed a function that performs steps A, B, and C to multi_cpu_stop(), there's
>> no guarantee that all CPUs complete step A before any CPU begins step B.
>
>If it could be said more concisely, it seems relevant.
How about:
multi_cpu_stop() executes in lockstep but doesn't synchronize steps within the
callback function it takes. So, implement one based on its pattern.
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Jason Gunthorpe @ 2026-03-13 13:36 UTC (permalink / raw)
To: Xu Yilun
Cc: Aneesh Kumar K.V, Dan Williams, linux-coco, linux-pci, gregkh,
aik, bhelgaas, alistair23, lukas, Arnd Bergmann
In-Reply-To: <abPlt8oNUOqGvbA3@yilunxu-OptiPlex-7050>
On Fri, Mar 13, 2026 at 06:23:51PM +0800, Xu Yilun wrote:
> My understanding is, it is the obfuscated host start pfn of this range,
> if this range has offset to the BAR start, this field should also be
> offsetted.
The OS must get an idea of the bar layout out of the report, so there
have to be restrictions on how it is formed otherwise it is
unparsible. IMHO the PCI spec created this very general mechanism but
the CPU CC specs need to constrain it to be usable by an OS.
> > range_off = tsm_offset & (pci_resource_len(pdev, bar) - 1);
> >
> > So that we correctly handle if the interface report is reporting a range
> > within a bar. The only requirement here is bar address should be aligned
> > to its size and mmio_reporting_offset should not add offsets in that range.
Right.
Jason
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Jason Gunthorpe @ 2026-03-13 13:32 UTC (permalink / raw)
To: Dan Williams
Cc: Greg KH, linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, lukas, Christoph Hellwig, Marek Szyprowski,
Robin Murphy, Roman Kisel, Samuel Ortiz, Rafael J. Wysocki,
Danilo Krummrich
In-Reply-To: <69b38e7427a61_b2b610073@dwillia2-mobl4.notmuch>
On Thu, Mar 12, 2026 at 09:11:32PM -0700, Dan Williams wrote:
> Greg KH wrote:
> > On Mon, Mar 02, 2026 at 04:01:51PM -0800, Dan Williams wrote:
> > > An "accepted" device is one that is allowed to access private memory within
> > > a Trusted Computing Boundary (TCB). The concept of "acceptance" is distinct
> > > from other device properties like usb_device::authorized, or
> > > tb_switch::authorized. The entry to the accepted state is a violent
> > > operation in which the device will reject MMIO requests that are not
> > > encrypted, and the device enters a new IOMMU protection domain to allow it
> > > to access addresses that were previously off-limits.
> >
> > Trying to mix/match "acceptance" with "authorized" is going to be a
> > nightmare, what's the combination that can happen here over time?
>
> I do think Linux needs to mix/match these concepts. "Authorization" is a
> kernel policy to operate a device at all. "Acceptance" is a mechanism
> to operate a device within a hardware TCB boundary.
I'm not sure about these words either, I would revise your table to be
more OS centric, the device can be in one of four security levels:
0 Blocked and disabled
The device cannot attack the system, enforced by the OS not loading a
driver or mapping the MMIO and IOMMU fully blocking everything from it.
1 In use, attacks from a hostile device are possible
A driver can operate the device and is expected to defend against
attacks from the device itself. The IOMMU restricts the device to only
access driver approved data (no ATS, DMA strict only, CC shared
only, interrupt remapping security, bounce partial DMA mappings, etc)
2 In use, no attacks from the device
The device does what the driver says and is not hostile. The driver
does not have to defend itself, the IOMMU can run in faster & lower
security modes (ATS on, DMA-FQ, Identity, still CC shared only)
* Basically our default security level today
3 In use, no attacks, and access to CC private memory
Like #2 and now the IOMMU allows access to CC private memory too.
[*] I'm inclunding all attacks with "hostile device", including MIM on
the PCIe link, compromised/fake device, attacks from a VMM through a
virtual device, etc.
From a CC VM perspective 0 is at boot, 1 is an out of TCB device, 2
doesn't exist (without TDISP there is no way to keep the
hypervisor from attacking?), and 3 is a full accepted TDSIP device.
#2 can happen in bare metal where a OS may activate link encryption
and attest the device, but doesn't have CC private/shared memory.
From a uAPI perspective I'm not sold on having two bools, I think a
level string would be more flexible. TSM and CC properties are
orthogonal, except you can't select #3 without the TSM saying it is in
RUN.
Internally we'd probably turn that dev->trusted thing into an
enum and teach the iommu layer to treat it more dynamically.
Jason
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Greg KH @ 2026-03-13 12:18 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu, bhelgaas,
alistair23, lukas, jgg, Christoph Hellwig, Jason Gunthorpe,
Marek Szyprowski, Robin Murphy, Roman Kisel, Samuel Ortiz,
Rafael J. Wysocki, Danilo Krummrich
In-Reply-To: <69b38e7427a61_b2b610073@dwillia2-mobl4.notmuch>
On Thu, Mar 12, 2026 at 09:11:32PM -0700, Dan Williams wrote:
> Greg KH wrote:
> > On Mon, Mar 02, 2026 at 04:01:51PM -0800, Dan Williams wrote:
> > > An "accepted" device is one that is allowed to access private memory within
> > > a Trusted Computing Boundary (TCB). The concept of "acceptance" is distinct
> > > from other device properties like usb_device::authorized, or
> > > tb_switch::authorized. The entry to the accepted state is a violent
> > > operation in which the device will reject MMIO requests that are not
> > > encrypted, and the device enters a new IOMMU protection domain to allow it
> > > to access addresses that were previously off-limits.
> >
> > Trying to mix/match "acceptance" with "authorized" is going to be a
> > nightmare, what's the combination that can happen here over time?
>
> I do think Linux needs to mix/match these concepts. "Authorization" is a
> kernel policy to operate a device at all. "Acceptance" is a mechanism
> to operate a device within a hardware TCB boundary.
>
> So, the truth table of combinations would be:
>
> accepted authorized result
> 0 0 logically and physically disconnected device
>
> 0 1 connected device, DMA is bounce buffered
> and loses confidentiality, integrity,
> and performance
>
> 1 0 logically disconnected device, but a
> relying party trusts that the device is
> not spoofing authorization.
>
> 1 1 connected device, DMA is direct and gains
> confidentiality, integrity and performance
>
> To say it another way, when the above distinguishes "logically" vs
> "physically" disconnected it is whether the device interface can be
> verified to not be under adversarial control. An unaccepted device can
> do limited damage, but still can bounce buffer secrets out of the TCB if
> so directed.
I really don't agree with this, but I can't think of why at the moment.
I feel like you are looking at this purely in the TCB point of view,
while I don't feel that is something that should be considered "special"
at all here. Linux has, for the most part, always trusted the hardware,
and now you are wanting to not trust the hardware for some things and
parts of the kernel. Which is great, it's something that I have wanted
to change for a very long time now, but let's do it right if at all
possible.
Give me a few days to come up with a better reply, let me think about
this some more...
> > We need to either "trust" or "not trust" the device, and the bus can
> > decide what to do with that value (if anything). The DMA layer can then
> > use that value to do:
>
> Trust is separate. For example, there are deployed use cases today where
> the device is trusted, but unaccepted. Acceptance support for those
> cases is mostly a performance optimization to be able to stop performing
> software encryption on top of DMA bounce buffering.
If "acceptance" is just a performance issue, I think you all need to go
back to the marketing people as that's probably not what they intended
to have happen here. For some reason I thought they were selling this
as "security", not "speed" :)
> > > Subsystems like the DMA mapping layer, that need to modify their behavior
> > > based on the accept state, may only have access to the base 'struct
> > > device'.
> >
> > ^this.
>
> The DMA layer is not operating on a trust concept it is effectively
> being told to select an IOMMU.
Ok, then that's independent of "acceptance", that is "use this IOMMU vs.
that one" type of thing which is just a "basic configuration for speed"
type of thing as you mention above :)
Let's not confuse that with anything else like "acceptance" please.
> > > It is also likely that the concept of TCB acceptance grows beyond
> > > PCI devices over time. For these reasons, introduce the concept of
> > > acceptance in 'struct device_private' which is device common, but only
> > > suitable for buses and built-in infrastructure to consume.
> >
> > Busses are what can control this, but please, let's not make this a
> > cc-only type thing. We have the idea of trust starting to propagate
> > through a number of different busses, let's get it right here, so we
> > don't have to have all of these different bus-specific hacks like we do
> > today.
>
> The conflation of "trust" and "acceptance" has been the main stumbling
> block of past proposals. As you have said before "kernel drivers trust
> their devices". That precedent is not being touched in this proposal.
Ah, but I WANT to touch that. Let's FINALLY solve that! Or at the very
least, provide the infrastructure in the driver core to allow busses
that want to do that, to be able to do so.
> Instead, give userspace all the tools it needs to deploy policy about
> when to operate a device. When it does decide to operate the device give
> it the mechanism to add confidentiality, integrity and performance to
> that operation.
Yes, this is a policy decision, and if you are only saying this is about
"which IOMMU should we select", then that's a dma layer configuration
option. Let's not call that "acceptance" please.
> This is a "CC-only type thing" because only CC partitions the system
> into two device domains. One where "trusted unaccepted" devices can
> operate without CC protections and "trusted accepted" devices can
> operate with CC protections and direct DMA.
In other words, it's an IOMMU switch, so why not use the switch
infrastructure? </me runs away...>
anyway, let me think about this some more...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Chao Gao @ 2026-03-13 12:16 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: Zhao, Yan Y, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Huang, Kai, dave.hansen@linux.intel.com, kas@kernel.org,
Chatre, Reinette, seanjc@google.com, linux-kernel@vger.kernel.org,
binbin.wu@linux.intel.com, Weiny, Ira, nik.borisov@suse.com,
mingo@redhat.com, Verma, Vishal L, tony.lindgren@linux.intel.com,
Annapurve, Vishal, sagis@google.com, Duan, Zhenzhong,
tglx@kernel.org, paulmck@kernel.org, hpa@zytor.com, bp@alien8.de,
yilun.xu@linux.intel.com, x86@kernel.org, Williams, Dan J
In-Reply-To: <89d69e852d3d822b51d3623d02ef6637239ccad5.camel@intel.com>
On Fri, Mar 13, 2026 at 12:56:19AM +0800, Edgecombe, Rick P wrote:
>On Thu, 2026-03-12 at 22:36 +0800, Chao Gao wrote:
>> > > + if (blob->version != 0x100) {
>> > Do we need a macro for this 0x100?
>>
>> Maybe not, as this is a one-off check (i.e., the version/macro won't be used
>> anywhere else). If someone has a strong opinion on this, I can add one.
>
>Seems like kind of a magic number as it is. What would the macro name be, and
>would it make the code more understandable?
Yes. Adding a macro can improve readability. So, will do.
Thanks, Yan and Rick.
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Chao Gao @ 2026-03-13 12:15 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <31936a20-929f-489a-9dc6-0f8fcb9307f1@intel.com>
On Thu, Mar 12, 2026 at 01:40:44PM -0700, Dave Hansen wrote:
>On 2/12/26 06:35, Chao Gao wrote:
>> +static void set_target_state(enum tdp_state state)
>> +{
>> + /* Reset ack counter. */
>> + atomic_set(&tdp_data.thread_ack, num_online_cpus());
>> + /* Ensure thread_ack is updated before the new state */
>> + smp_wmb();
>> + WRITE_ONCE(tdp_data.state, state);
>> +}
>
>This looks overly complicated.
>
>If it doesn't need to be scalable, just make it stupid and simple. Why
>not just protect the whole thing with a spinlock and be done with it?
Good suggestion. I copied this from multi_cpu_stop() without considering
whether it could be simplified.
Regarding scalability, I compared the update time and didn't see a
meaningful difference on a system with 240 CPUs.
I will make changes like this:
(Note: I'm also renaming tdp_data/tdp_state to update_data and
module_update_state for clarity, since "tdp" isn't obvious as Kai pointed
out.)
static struct {
enum module_update_state state;
- atomic_t thread_ack;
- atomic_t failed;
+ int thread_ack;
+ int failed;
+ raw_spinlock_t lock;
} update_data;
static void set_target_state(enum module_update_state state)
{
/* Reset ack counter. */
- atomic_set(&update_data.thread_ack, num_online_cpus());
- /*
- * Ensure thread_ack is updated before the new state.
- * Otherwise, other CPUs may see the new state and ack
- * it before thread_ack is reset. An ack before reset
- * is effectively lost, causing the system to wait
- * forever for thread_ack to become zero.
- */
- smp_wmb();
- WRITE_ONCE(update_data.state, state);
+ update_data.thread_ack = num_online_cpus();
+ update_data.state = state;
}
/* Last one to ack a state moves to the next state. */
static void ack_state(void)
{
- if (atomic_dec_and_test(&update_data.thread_ack))
+ guard(raw_spinlock)(&update_data.lock);
+ update_data.thread_ack--;
+ if (!update_data.thread_ack)
set_target_state(update_data.state + 1);
}
^ permalink raw reply
* Re: SVSM Development Call March 11, 2026
From: Jörg Rödel @ 2026-03-13 10:49 UTC (permalink / raw)
To: coconut-svsm, linux-coco
In-Reply-To: <ete4u42tzmawocptkhibgkv6tgg7rlqcpncdutnk36qtpdejcz@cxhur5ib7dzz>
Meeting minutes are now posted here:
https://github.com/coconut-svsm/governance/pull/99
-Joerg
^ permalink raw reply
* Re: [PATCH v2 10/19] x86, swiotlb: Teach swiotlb to skip "accepted" devices
From: Xu Yilun @ 2026-03-13 10:26 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Dan Williams, linux-coco, linux-pci, gregkh, aik, bhelgaas,
alistair23, lukas, jgg, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Marek Szyprowski, Robin Murphy
In-Reply-To: <yq5ah5qx9t77.fsf@kernel.org>
> > @@ -365,6 +365,7 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
> >
> > io_tlb_default_mem.force_bounce =
> > swiotlb_force_bounce || (flags & SWIOTLB_FORCE);
> > + io_tlb_default_mem.bounce_unaccepted = flags & SWIOTLB_UNACCEPTED;
> >
>
> This should be.
>
> @@ -373,7 +373,7 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
>
> io_tlb_default_mem.force_bounce =
> swiotlb_force_bounce || (flags & SWIOTLB_FORCE);
> - io_tlb_default_mem.bounce_unaccepted = flags & SWIOTLB_UNACCEPTED;
> + io_tlb_default_mem.bounce_unaccepted = !!(flags & SWIOTLB_UNACCEPTED);
Ah yes, I just realized assigning to a 1-bit field would truncate the
assigned value to its LSB...
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Xu Yilun @ 2026-03-13 10:23 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Dan Williams, linux-coco, linux-pci, gregkh, aik, bhelgaas,
alistair23, lukas, jgg, Arnd Bergmann
In-Reply-To: <yq5a4imuanoa.fsf@kernel.org>
> > + if (last_bar < bar) {
> > + /* transition to a new bar */
> > + last_bar = bar;
> > + /*
> > + * The tsm_offset for the first range of the BAR
> > + * corresponds to the BAR base.
> > + */
> > + reporting_bar_base = tsm_offset;
> > + } else if (tsm_offset < last_reporting_end) {
> > + pci_dbg(pdev, "Reporting ranges within BAR not in ascending order\n");
> > + return NULL;
> > + }
> >
> ....
> > + range_off = tsm_offset - reporting_bar_base;
> >
> range_off will always be zero? Should we do
tsm_offset comes from Device Interface Report, MMIO RANGE, First 4k
Page. How do you interpret the exact meaning of this field?
My understanding is, it is the obfuscated host start pfn of this range,
if this range has offset to the BAR start, this field should also be
offsetted.
But if the first range in the BAR should be aligned to BAR, otherwise
there is no way for guest to position the range in the BAR.
So the logic here is:
reporting_bar_base: the first obfuscated pfn for the BAR, the BAR pfn
tsm_offset: the current obfucated pfn for the BAR.
tsm_offset - reporting_bar_base: the offset to the BAR.
>
> range_off = tsm_offset & (pci_resource_len(pdev, bar) - 1);
>
>
> So that we correctly handle if the interface report is reporting a range
> within a bar. The only requirement here is bar address should be aligned
> to its size and mmio_reporting_offset should not add offsets in that range.
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Xu Yilun @ 2026-03-13 10:07 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, bhelgaas,
alistair23, lukas, jgg, Donald Hunter, Jakub Kicinski
In-Reply-To: <20260303000207.1836586-9-dan.j.williams@intel.com>
> +void pci_tsm_init_evidence(struct pci_tsm_evidence *evidence, int slot,
> + enum hash_algo digest_algo)
> +{
> + evidence->slot = slot;
> + evidence->generation = 1;
> + evidence->digest_algo = digest_algo;
> + init_rwsem(&evidence->lock);
IIUC, this function is for link tsm driver, is it? But in the following
patch, devsec tsm would consume pci_tsm_mmio_alloc() which uses
evidence->lock. So my solution is to initialize the lock on tsm
construction.
--------8<--------
diff --git a/drivers/pci/tsm/core.c b/drivers/pci/tsm/core.c
index 9f062218c312..c55deeafe32b 100644
--- a/drivers/pci/tsm/core.c
+++ b/drivers/pci/tsm/core.c
@@ -1175,7 +1175,6 @@ void pci_tsm_init_evidence(struct pci_tsm_evidence *evidence, int slot,
evidence->slot = slot;
evidence->generation = 1;
evidence->digest_algo = digest_algo;
- init_rwsem(&evidence->lock);
}
EXPORT_SYMBOL_GPL(pci_tsm_init_evidence);
@@ -1198,6 +1197,7 @@ int pci_tsm_link_constructor(struct pci_dev *pdev, struct pci_tsm *tsm,
}
tsm->pdev = pdev;
tsm->tsm_dev = tsm_dev;
+ init_rwsem(&tsm->evidence.lock);
return 0;
}
@@ -1221,6 +1221,7 @@ int pci_tsm_devsec_constructor(struct pci_dev *pdev, struct pci_tsm_devsec *tsm,
pci_tsm->tdi = NULL;
pci_tsm->pdev = pdev;
pci_tsm->tsm_dev = tsm_dev;
+ init_rwsem(&pci_tsm->evidence.lock);
return 0;
}
^ permalink raw reply related
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Xu Yilun @ 2026-03-13 9:57 UTC (permalink / raw)
To: dan.j.williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, bhelgaas,
alistair23, lukas, jgg, Arnd Bergmann
In-Reply-To: <69a86891458b8_6423c1009f@dwillia2-mobl4.notmuch>
> > + if (dev_WARN_ONCE(&tsm_dev->dev, !IS_ALIGNED((unsigned long) report_obj->data, 8),
> > + "misaligned report data\n"))
> > + return NULL;
>
> Is this going to cause any implementation to need to copy the buffer
> received from the low-level TSM? If so I would just mark 'struct
TDX Connect needs the copy, the GHCI header is 20 byte size, not
naturally aligned...
> pci_tsm_devif_report' and 'struct pci_tsm_mmio_entry' as __packed and
> drop this check.
..But since this series will persist the report blob long term in
tsm.evidence.obj[PCI_TSM_EVIDENCE_TYPE_REPORT], we anyway need to copy
the blob out from temporary shared GHCI communication buffer. So it is
actually good to me.
^ permalink raw reply
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Nirmoy Das @ 2026-03-13 9:46 UTC (permalink / raw)
To: Salman Nabi, vvidwans, andre.przywara, sudeep.holla, mark.rutland,
lpieralisi
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <20260119122729.287522-2-salman.nabi@arm.com>
Hi Salman and Andre,
We found an bug while testing LFA. See below:
On 19.01.26 14:27, Salman Nabi wrote:
> The Arm Live Firmware Activation (LFA) is a specification [1] to describe
> activating firmware components without a reboot. Those components
> (like TF-A's BL31, EDK-II, TF-RMM, secure paylods) would be updated the
> usual way: via fwupd, FF-A or other secure storage methods, or via some
> IMPDEF Out-Of-Bound method. The user can then activate this new firmware,
> at system runtime, without requiring a reboot.
> The specification covers the SMCCC interface to list and query available
> components and eventually trigger the activation.
>
> Add a new directory under /sys/firmware to present firmware components
> capable of live activation. Each of them is a directory under lfa/,
> and is identified via its GUID. The activation will be triggered by echoing
> "1" into the "activate" file:
> ==========================================
> /sys/firmware/lfa # ls -l . 6c*
> .:
> total 0
> drwxr-xr-x 2 0 0 0 Jan 19 11:33 47d4086d-4cfe-9846-9b95-2950cbbd5a00
> drwxr-xr-x 2 0 0 0 Jan 19 11:33 6c0762a6-12f2-4b56-92cb-ba8f633606d9
> drwxr-xr-x 2 0 0 0 Jan 19 11:33 d6d0eea7-fcea-d54b-9782-9934f234b6e4
>
> 6c0762a6-12f2-4b56-92cb-ba8f633606d9:
> total 0
> --w------- 1 0 0 4096 Jan 19 11:33 activate
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 activation_capable
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 activation_pending
> --w------- 1 0 0 4096 Jan 19 11:33 cancel
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 cpu_rendezvous
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 current_version
> -rw-r--r-- 1 0 0 4096 Jan 19 11:33 force_cpu_rendezvous
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 may_reset_cpu
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 name
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 pending_version
> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 # grep . *
> grep: activate: Permission denied
> activation_capable:1
> activation_pending:1
> grep: cancel: Permission denied
> cpu_rendezvous:1
> current_version:0.0
> force_cpu_rendezvous:1
> may_reset_cpu:0
> name:TF-RMM
> pending_version:0.0
> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 # echo 1 > activate
> [ 2825.797871] Arm LFA: firmware activation succeeded.
> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 #
> ==========================================
>
> [1] https://developer.arm.com/documentation/den0147/latest/
>
> Signed-off-by: Salman Nabi <salman.nabi@arm.com>
> ---
> drivers/firmware/smccc/Kconfig | 8 +
> drivers/firmware/smccc/Makefile | 1 +
> drivers/firmware/smccc/lfa_fw.c | 668 ++++++++++++++++++++++++++++++++
> 3 files changed, 677 insertions(+)
> create mode 100644 drivers/firmware/smccc/lfa_fw.c
>
> diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
> index 15e7466179a6..ff7ca49486b0 100644
> --- a/drivers/firmware/smccc/Kconfig
> +++ b/drivers/firmware/smccc/Kconfig
> @@ -23,3 +23,11 @@ config ARM_SMCCC_SOC_ID
> help
> Include support for the SoC bus on the ARM SMCCC firmware based
> platforms providing some sysfs information about the SoC variant.
> +
> +config ARM_LFA
> + tristate "Arm Live Firmware activation support"
> + depends on HAVE_ARM_SMCCC_DISCOVERY
> + default y
> + help
> + Include support for triggering Live Firmware Activation, which
> + allows to upgrade certain firmware components without a reboot.
> diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile
> index 40d19144a860..a6dd01558a94 100644
> --- a/drivers/firmware/smccc/Makefile
> +++ b/drivers/firmware/smccc/Makefile
> @@ -2,3 +2,4 @@
> #
> obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smccc.o kvm_guest.o
> obj-$(CONFIG_ARM_SMCCC_SOC_ID) += soc_id.o
> +obj-$(CONFIG_ARM_LFA) += lfa_fw.o
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> new file mode 100644
> index 000000000000..ce54049b7190
> --- /dev/null
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -0,0 +1,668 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 Arm Limited
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/kobject.h>
> +#include <linux/module.h>
> +#include <linux/stop_machine.h>
> +#include <linux/string.h>
> +#include <linux/sysfs.h>
> +#include <linux/arm-smccc.h>
> +#include <linux/psci.h>
> +#include <uapi/linux/psci.h>
> +#include <linux/uuid.h>
> +#include <linux/array_size.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "Arm LFA: " fmt
> +
> +/* LFA v1.0b0 specification */
> +#define LFA_1_0_FN_BASE 0xc40002e0
> +#define LFA_1_0_FN(n) (LFA_1_0_FN_BASE + (n))
> +
> +#define LFA_1_0_FN_GET_VERSION LFA_1_0_FN(0)
> +#define LFA_1_0_FN_CHECK_FEATURE LFA_1_0_FN(1)
> +#define LFA_1_0_FN_GET_INFO LFA_1_0_FN(2)
> +#define LFA_1_0_FN_GET_INVENTORY LFA_1_0_FN(3)
> +#define LFA_1_0_FN_PRIME LFA_1_0_FN(4)
> +#define LFA_1_0_FN_ACTIVATE LFA_1_0_FN(5)
> +#define LFA_1_0_FN_CANCEL LFA_1_0_FN(6)
> +
> +/* CALL_AGAIN flags (returned by SMC) */
> +#define LFA_PRIME_CALL_AGAIN BIT(0)
> +#define LFA_ACTIVATE_CALL_AGAIN BIT(0)
> +
> +/* LFA return values */
> +#define LFA_SUCCESS 0
> +#define LFA_NOT_SUPPORTED 1
> +#define LFA_BUSY 2
> +#define LFA_AUTH_ERROR 3
> +#define LFA_NO_MEMORY 4
> +#define LFA_CRITICAL_ERROR 5
> +#define LFA_DEVICE_ERROR 6
> +#define LFA_WRONG_STATE 7
> +#define LFA_INVALID_PARAMETERS 8
> +#define LFA_COMPONENT_WRONG_STATE 9
> +#define LFA_INVALID_ADDRESS 10
> +#define LFA_ACTIVATION_FAILED 11
> +
> +#define LFA_ERROR_STRING(name) \
> + [name] = #name
> +
> +static const char * const lfa_error_strings[] = {
> + LFA_ERROR_STRING(LFA_SUCCESS),
> + LFA_ERROR_STRING(LFA_NOT_SUPPORTED),
> + LFA_ERROR_STRING(LFA_BUSY),
> + LFA_ERROR_STRING(LFA_AUTH_ERROR),
> + LFA_ERROR_STRING(LFA_NO_MEMORY),
> + LFA_ERROR_STRING(LFA_CRITICAL_ERROR),
> + LFA_ERROR_STRING(LFA_DEVICE_ERROR),
> + LFA_ERROR_STRING(LFA_WRONG_STATE),
> + LFA_ERROR_STRING(LFA_INVALID_PARAMETERS),
> + LFA_ERROR_STRING(LFA_COMPONENT_WRONG_STATE),
> + LFA_ERROR_STRING(LFA_INVALID_ADDRESS),
> + LFA_ERROR_STRING(LFA_ACTIVATION_FAILED)
> +};
> +
> +enum image_attr_names {
> + LFA_ATTR_NAME,
> + LFA_ATTR_CURRENT_VERSION,
> + LFA_ATTR_PENDING_VERSION,
> + LFA_ATTR_ACT_CAPABLE,
> + LFA_ATTR_ACT_PENDING,
> + LFA_ATTR_MAY_RESET_CPU,
> + LFA_ATTR_CPU_RENDEZVOUS,
> + LFA_ATTR_FORCE_CPU_RENDEZVOUS,
> + LFA_ATTR_ACTIVATE,
> + LFA_ATTR_CANCEL,
> + LFA_ATTR_NR_IMAGES
> +};
> +
> +struct image_props {
> + struct list_head image_node;
> + const char *image_name;
> + int fw_seq_id;
> + u64 current_version;
> + u64 pending_version;
> + bool activation_capable;
> + bool activation_pending;
> + bool may_reset_cpu;
> + bool cpu_rendezvous;
> + bool cpu_rendezvous_forced;
> + struct kobject *image_dir;
> + struct kobj_attribute image_attrs[LFA_ATTR_NR_IMAGES];
> +};
> +static LIST_HEAD(lfa_fw_images);
> +
> +/* A UUID split over two 64-bit registers */
> +struct uuid_regs {
> + u64 uuid_lo;
> + u64 uuid_hi;
> +};
> +
> +static const struct fw_image_uuid {
> + const char *name;
> + const char *uuid;
> +} fw_images_uuids[] = {
> + {
> + .name = "TF-A BL31 runtime",
> + .uuid = "47d4086d-4cfe-9846-9b95-2950cbbd5a00",
> + },
> + {
> + .name = "BL33 non-secure payload",
> + .uuid = "d6d0eea7-fcea-d54b-9782-9934f234b6e4",
> + },
> + {
> + .name = "TF-RMM",
> + .uuid = "6c0762a6-12f2-4b56-92cb-ba8f633606d9",
> + },
> +};
> +
> +static struct kobject *lfa_dir;
> +static DEFINE_MUTEX(lfa_lock);
> +static struct workqueue_struct *fw_images_update_wq;
> +static struct work_struct fw_images_update_work;
> +
> +static int update_fw_images_tree(void);
> +
> +static void delete_fw_image_node(struct image_props *attrs)
> +{
> + int i;
> +
> + for (i = 0; i < LFA_ATTR_NR_IMAGES; i++)
> + sysfs_remove_file(attrs->image_dir, &attrs->image_attrs[i].attr);
> +
> + kobject_put(attrs->image_dir);
> + list_del(&attrs->image_node);
> + kfree(attrs);
> +}
> +
> +static void remove_invalid_fw_images(struct work_struct *work)
> +{
> + struct image_props *attrs, *tmp;
> +
> + mutex_lock(&lfa_lock);
> +
> + /*
> + * Remove firmware images including directories that are no longer
> + * present in the LFA agent after updating the existing ones.
> + */
> + list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node) {
> + if (attrs->fw_seq_id == -1)
> + delete_fw_image_node(attrs);
> + }
> +
> + mutex_unlock(&lfa_lock);
> +}
> +
> +static void set_image_flags(struct image_props *attrs, int seq_id,
> + u32 image_flags, u64 reg_current_ver,
> + u64 reg_pending_ver)
> +{
> + attrs->fw_seq_id = seq_id;
> + attrs->current_version = reg_current_ver;
> + attrs->pending_version = reg_pending_ver;
> + attrs->activation_capable = !!(image_flags & BIT(0));
> + attrs->activation_pending = !!(image_flags & BIT(1));
> + attrs->may_reset_cpu = !!(image_flags & BIT(2));
> + /* cpu_rendezvous_optional bit has inverse logic in the spec */
> + attrs->cpu_rendezvous = !(image_flags & BIT(3));
> +}
> +
> +static unsigned long get_nr_lfa_components(void)
> +{
> + struct arm_smccc_1_2_regs reg = { 0 };
> +
> + reg.a0 = LFA_1_0_FN_GET_INFO;
> + reg.a1 = 0; /* lfa_info_selector = 0 */
> +
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 != LFA_SUCCESS)
> + return reg.a0;
> +
> + return reg.a1;
> +}
> +
> +static int lfa_cancel(void *data)
> +{
> + struct image_props *attrs = data;
> + struct arm_smccc_1_2_regs reg = { 0 };
> +
> + reg.a0 = LFA_1_0_FN_CANCEL;
> + reg.a1 = attrs->fw_seq_id;
> + arm_smccc_1_2_invoke(®, ®);
> +
> + /*
> + * When firmware activation is called with "skip_cpu_rendezvous=1",
> + * LFA_CANCEL can fail with LFA_BUSY if the activation could not be
> + * cancelled.
> + */
> + if (reg.a0 == LFA_SUCCESS) {
> + pr_info("Activation cancelled for image %s\n",
> + attrs->image_name);
> + } else {
> + pr_err("Firmware activation could not be cancelled: %s\n",
> + lfa_error_strings[-reg.a0]);
> + return -EINVAL;
> + }
> +
> + return reg.a0;
> +}
> +
> +static int call_lfa_activate(void *data)
> +{
> + struct image_props *attrs = data;
> + struct arm_smccc_1_2_regs reg = { 0 };
> +
> + reg.a0 = LFA_1_0_FN_ACTIVATE;
> + reg.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
> + /*
> + * As we do not support updates requiring a CPU reset (yet),
> + * we pass 0 in reg.a3 and reg.a4, holding the entry point and context
> + * ID respectively.
> + * cpu_rendezvous_forced is set by the administrator, via sysfs,
> + * cpu_rendezvous is dictated by each firmware component.
> + */
> + reg.a2 = !(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous);
> +
> + for (;;) {
> + arm_smccc_1_2_invoke(®, ®);
> +
> + if ((long)reg.a0 < 0) {
> + pr_err("ACTIVATE for image %s failed: %s\n",
> + attrs->image_name, lfa_error_strings[-reg.a0]);
> + return reg.a0;
> + }
> + if (!(reg.a1 & LFA_ACTIVATE_CALL_AGAIN))
> + break; /* ACTIVATE successful */
> + }
> +
> + return reg.a0;
> +}
> +
> +static int activate_fw_image(struct image_props *attrs)
> +{
> + int ret;
> +
> + mutex_lock(&lfa_lock);
> + if (attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous)
> + ret = stop_machine(call_lfa_activate, attrs, cpu_online_mask);
> + else
> + ret = call_lfa_activate(attrs);
> +
> + if (ret != 0) {
> + mutex_unlock(&lfa_lock);
> + return lfa_cancel(attrs);
> + }
> +
> + /*
> + * Invalidate fw_seq_ids (-1) for all images as the seq_ids and the
> + * number of firmware images in the LFA agent may change after a
> + * successful activation attempt. Negate all image flags as well.
> + */
> + attrs = NULL;
> + list_for_each_entry(attrs, &lfa_fw_images, image_node) {
> + set_image_flags(attrs, -1, 0b1000, 0, 0);
> + }
> +
> + update_fw_images_tree();
> +
> + /*
> + * Removing non-valid image directories at the end of an activation.
> + * We can't remove the sysfs attributes while in the respective
> + * _store() handler, so have to postpone the list removal to a
> + * workqueue.
> + */
> + INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
This can get invoke multiple times so re-initializing a work item that
may already be queued or running
is unsafe. This should be moved to lfa_init() so it is only called once.
I suggest:
diff --git a/drivers/firmware/smccc/lfa_fw.c
b/drivers/firmware/smccc/lfa_fw.c
index 90727a66e49a5..135358113104c 100644
--- a/drivers/firmware/smccc/lfa_fw.c
+++ b/drivers/firmware/smccc/lfa_fw.c
@@ -653,7 +653,6 @@ static int update_fw_images_tree(void)
* _store() handler, so have to postpone the list removal to a
* workqueue.
*/
- INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
queue_work(fw_images_update_wq, &fw_images_update_work);
return 0;
@@ -680,7 +679,7 @@ static void lfa_notify_handler(acpi_handle handle,
u32 event, void *data)
* of all activable and pending images.
*/
do {
- /* Reset activable image flag */
+ flush_workqueue(fw_images_update_wq);
found_activable_image = false;
list_for_each_entry(attrs, &lfa_fw_images, image_node) {
if (attrs->fw_seq_id == -1)
@@ -782,6 +781,8 @@ static int __init lfa_init(void)
return -ENOMEM;
}
+ INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
+
pr_info("Live Firmware Activation: detected v%ld.%ld\n",
reg.a0 >> 16, reg.a0 & 0xffff);
Regards,
Nirmoy
> + queue_work(fw_images_update_wq, &fw_images_update_work);
> + mutex_unlock(&lfa_lock);
> +
> + return ret;
> +}
> +
> +static int prime_fw_image(struct image_props *attrs)
> +{
> + struct arm_smccc_1_2_regs reg = { 0 };
> + int ret;
> +
> + mutex_lock(&lfa_lock);
> + /* Avoid SMC calls on invalid firmware images */
> + if (attrs->fw_seq_id == -1) {
> + pr_err("Arm LFA: Invalid firmware sequence id\n");
> + mutex_unlock(&lfa_lock);
> +
> + return -ENODEV;
> + }
> +
> + if (attrs->may_reset_cpu) {
> + pr_err("CPU reset not supported by kernel driver\n");
> + mutex_unlock(&lfa_lock);
> +
> + return -EINVAL;
> + }
> +
> + /*
> + * LFA_PRIME/ACTIVATE will return 1 in reg.a1 if the firmware
> + * priming/activation is still in progress. In that case
> + * LFA_PRIME/ACTIVATE will need to be called again.
> + * reg.a1 will become 0 once the prime/activate process completes.
> + */
> + reg.a0 = LFA_1_0_FN_PRIME;
> + reg.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
> + for (;;) {
> + arm_smccc_1_2_invoke(®, ®);
> +
> + if ((long)reg.a0 < 0) {
> + pr_err("LFA_PRIME for image %s failed: %s\n",
> + attrs->image_name, lfa_error_strings[-reg.a0]);
> + mutex_unlock(&lfa_lock);
> +
> + return reg.a0;
> + }
> + if (!(reg.a1 & LFA_PRIME_CALL_AGAIN)) {
> + ret = 0;
> + break; /* PRIME successful */
> + }
> + }
> +
> + mutex_unlock(&lfa_lock);
> + return ret;
> +}
> +
> +static ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_NAME]);
> +
> + return sysfs_emit(buf, "%s\n", attrs->image_name);
> +}
> +
> +static ssize_t activation_capable_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_ACT_CAPABLE]);
> +
> + return sysfs_emit(buf, "%d\n", attrs->activation_capable);
> +}
> +
> +static ssize_t activation_pending_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_ACT_PENDING]);
> + struct arm_smccc_1_2_regs reg = { 0 };
> +
> + /*
> + * Activation pending status can change anytime thus we need to update
> + * and return its current value
> + */
> + reg.a0 = LFA_1_0_FN_GET_INVENTORY;
> + reg.a1 = attrs->fw_seq_id;
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 == LFA_SUCCESS)
> + attrs->activation_pending = !!(reg.a3 & BIT(1));
> +
> + return sysfs_emit(buf, "%d\n", attrs->activation_pending);
> +}
> +
> +static ssize_t may_reset_cpu_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_MAY_RESET_CPU]);
> +
> + return sysfs_emit(buf, "%d\n", attrs->may_reset_cpu);
> +}
> +
> +static ssize_t cpu_rendezvous_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_CPU_RENDEZVOUS]);
> +
> + return sysfs_emit(buf, "%d\n", attrs->cpu_rendezvous);
> +}
> +
> +static ssize_t force_cpu_rendezvous_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_FORCE_CPU_RENDEZVOUS]);
> + int ret;
> +
> + ret = kstrtobool(buf, &attrs->cpu_rendezvous_forced);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static ssize_t force_cpu_rendezvous_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_FORCE_CPU_RENDEZVOUS]);
> +
> + return sysfs_emit(buf, "%d\n", attrs->cpu_rendezvous_forced);
> +}
> +
> +static ssize_t current_version_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_CURRENT_VERSION]);
> + u32 maj, min;
> +
> + maj = attrs->current_version >> 32;
> + min = attrs->current_version & 0xffffffff;
> + return sysfs_emit(buf, "%u.%u\n", maj, min);
> +}
> +
> +static ssize_t pending_version_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_ACT_PENDING]);
> + struct arm_smccc_1_2_regs reg = { 0 };
> + u32 maj, min;
> +
> + /*
> + * Similar to activation pending, this value can change following an
> + * update, we need to retrieve fresh info instead of stale information.
> + */
> + reg.a0 = LFA_1_0_FN_GET_INVENTORY;
> + reg.a1 = attrs->fw_seq_id;
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 == LFA_SUCCESS) {
> + if (reg.a5 != 0 && attrs->activation_pending)
> + {
> + attrs->pending_version = reg.a5;
> + maj = reg.a5 >> 32;
> + min = reg.a5 & 0xffffffff;
> + }
> + }
> +
> + return sysfs_emit(buf, "%u.%u\n", maj, min);
> +}
> +
> +static ssize_t activate_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_ACTIVATE]);
> + int ret;
> +
> + ret = prime_fw_image(attrs);
> + if (ret) {
> + pr_err("Firmware prime failed: %s\n",
> + lfa_error_strings[-ret]);
> + return -ECANCELED;
> + }
> +
> + ret = activate_fw_image(attrs);
> + if (ret) {
> + pr_err("Firmware activation failed: %s\n",
> + lfa_error_strings[-ret]);
> + return -ECANCELED;
> + }
> +
> + pr_info("Firmware activation succeeded\n");
> +
> + return count;
> +}
> +
> +static ssize_t cancel_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_CANCEL]);
> + int ret;
> +
> + ret = lfa_cancel(attrs);
> + if (ret != 0)
> + return ret;
> +
> + return count;
> +}
> +
> +static struct kobj_attribute image_attrs_group[LFA_ATTR_NR_IMAGES] = {
> + [LFA_ATTR_NAME] = __ATTR_RO(name),
> + [LFA_ATTR_CURRENT_VERSION] = __ATTR_RO(current_version),
> + [LFA_ATTR_PENDING_VERSION] = __ATTR_RO(pending_version),
> + [LFA_ATTR_ACT_CAPABLE] = __ATTR_RO(activation_capable),
> + [LFA_ATTR_ACT_PENDING] = __ATTR_RO(activation_pending),
> + [LFA_ATTR_MAY_RESET_CPU] = __ATTR_RO(may_reset_cpu),
> + [LFA_ATTR_CPU_RENDEZVOUS] = __ATTR_RO(cpu_rendezvous),
> + [LFA_ATTR_FORCE_CPU_RENDEZVOUS] = __ATTR_RW(force_cpu_rendezvous),
> + [LFA_ATTR_ACTIVATE] = __ATTR_WO(activate),
> + [LFA_ATTR_CANCEL] = __ATTR_WO(cancel)
> +};
> +
> +static void clean_fw_images_tree(void)
> +{
> + struct image_props *attrs, *tmp;
> +
> + list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node)
> + delete_fw_image_node(attrs);
> +}
> +
> +static int update_fw_image_node(char *fw_uuid, int seq_id,
> + u32 image_flags, u64 reg_current_ver,
> + u64 reg_pending_ver)
> +{
> + const char *image_name = "(unknown)";
> + struct image_props *attrs;
> + int ret;
> +
> + /*
> + * If a fw_image is already in the images list then we just update
> + * its flags and seq_id instead of trying to recreate it.
> + */
> + list_for_each_entry(attrs, &lfa_fw_images, image_node) {
> + if (!strcmp(attrs->image_dir->name, fw_uuid)) {
> + set_image_flags(attrs, seq_id, image_flags,
> + reg_current_ver, reg_pending_ver);
> + return 0;
> + }
> + }
> +
> + attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> + if (!attrs)
> + return -ENOMEM;
> +
> + for (int i = 0; i < ARRAY_SIZE(fw_images_uuids); i++) {
> + if (!strcmp(fw_images_uuids[i].uuid, fw_uuid))
> + image_name = fw_images_uuids[i].name;
> + }
> +
> + attrs->image_dir = kobject_create_and_add(fw_uuid, lfa_dir);
> + if (!attrs->image_dir)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&attrs->image_node);
> + attrs->image_name = image_name;
> + attrs->cpu_rendezvous_forced = 1;
> + set_image_flags(attrs, seq_id, image_flags, reg_current_ver,
> + reg_pending_ver);
> +
> + /*
> + * The attributes for each sysfs file are constant (handler functions,
> + * name and permissions are the same within each directory), but we
> + * need a per-directory copy regardless, to get a unique handle
> + * for each directory, so that container_of can do its magic.
> + * Also this requires an explicit sysfs_attr_init(), since it's a new
> + * copy, to make LOCKDEP happy.
> + */
> + memcpy(attrs->image_attrs, image_attrs_group,
> + sizeof(attrs->image_attrs));
> + for (int i = 0; i < LFA_ATTR_NR_IMAGES; i++) {
> + struct attribute *attr = &attrs->image_attrs[i].attr;
> +
> + sysfs_attr_init(attr);
> + ret = sysfs_create_file(attrs->image_dir, attr);
> + if (ret) {
> + pr_err("creating sysfs file for uuid %s: %d\n",
> + fw_uuid, ret);
> + clean_fw_images_tree();
> +
> + return ret;
> + }
> + }
> + list_add(&attrs->image_node, &lfa_fw_images);
> +
> + return ret;
> +}
> +
> +static int update_fw_images_tree(void)
> +{
> + struct arm_smccc_1_2_regs reg = { 0 };
> + struct uuid_regs image_uuid;
> + char image_id_str[40];
> + int ret, num_of_components;
> +
> + num_of_components = get_nr_lfa_components();
> + if (num_of_components <= 0) {
> + pr_err("Error getting number of LFA components\n");
> + return -ENODEV;
> + }
> +
> + for (int i = 0; i < num_of_components; i++) {
> + reg.a0 = LFA_1_0_FN_GET_INVENTORY;
> + reg.a1 = i; /* fw_seq_id under consideration */
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 == LFA_SUCCESS) {
> + image_uuid.uuid_lo = reg.a1;
> + image_uuid.uuid_hi = reg.a2;
> +
> + snprintf(image_id_str, sizeof(image_id_str), "%pUb",
> + &image_uuid);
> + ret = update_fw_image_node(image_id_str, i,
> + reg.a3, reg.a4, reg.a5);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int __init lfa_init(void)
> +{
> + struct arm_smccc_1_2_regs reg = { 0 };
> + int err;
> +
> + reg.a0 = LFA_1_0_FN_GET_VERSION;
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 == -LFA_NOT_SUPPORTED) {
> + pr_info("Live Firmware activation: no firmware agent found\n");
> + return -ENODEV;
> + }
> +
> + fw_images_update_wq = alloc_workqueue("fw_images_update_wq",
> + WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
> + if (!fw_images_update_wq) {
> + pr_err("Live Firmware Activation: Failed to allocate workqueue.\n");
> +
> + return -ENOMEM;
> + }
> +
> + pr_info("Live Firmware Activation: detected v%ld.%ld\n",
> + reg.a0 >> 16, reg.a0 & 0xffff);
> +
> + lfa_dir = kobject_create_and_add("lfa", firmware_kobj);
> + if (!lfa_dir)
> + return -ENOMEM;
> +
> + mutex_lock(&lfa_lock);
> + err = update_fw_images_tree();
> + if (err != 0)
> + kobject_put(lfa_dir);
> +
> + mutex_unlock(&lfa_lock);
> + return err;
> +}
> +module_init(lfa_init);
> +
> +static void __exit lfa_exit(void)
> +{
> + flush_workqueue(fw_images_update_wq);
> + destroy_workqueue(fw_images_update_wq);
> +
> + mutex_lock(&lfa_lock);
> + clean_fw_images_tree();
> + mutex_unlock(&lfa_lock);
> +
> + kobject_put(lfa_dir);
> +}
> +module_exit(lfa_exit);
> +
> +MODULE_DESCRIPTION("ARM Live Firmware Activation (LFA)");
> +MODULE_LICENSE("GPL");
^ permalink raw reply related
* Re: [PATCH v4 09/24] x86/virt/seamldr: Check update limit before TDX Module updates
From: Chao Gao @ 2026-03-13 8:32 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <63082cd1-15ab-4aaf-83ad-f72d94b9bb8e@intel.com>
On Thu, Mar 12, 2026 at 01:23:04PM -0700, Dave Hansen wrote:
>On 2/12/26 06:35, Chao Gao wrote:
>> Note that userspace should perform this check before updates. Perform this
>> check in kernel as well to make the update process more robust.
>
>How many of these patches are to be "more robust"?
>
>If you don't need it to "turn the lights on", I say kick it out.
Only patch 8 and this patch fall into this category. I'll drop them.
^ permalink raw reply
* Re: [PATCH v4 07/24] coco/tdx-host: Implement firmware upload sysfs ABI for TDX Module updates
From: Chao Gao @ 2026-03-13 8:28 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <5b8d6f33-272b-4879-98ae-11c3cae3ccac@intel.com>
On Thu, Mar 12, 2026 at 01:20:59PM -0700, Dave Hansen wrote:
>On 2/12/26 06:35, Chao Gao wrote:
>> + * @data: Pointer to the TDX module update blob. It should be vmalloc'd
>> + * memory.
>
>Why?!?! What does it matter? Why enforce this?
The firmware upload framework stores firmware from userspace in vmalloc memory.
I added this check because a later patch uses vmalloc_to_pfn() to obtain
physical addresses.
As we discussed offline, I'll remove all is_vmalloc_addr() checks. Since
vmalloc_to_pfn/page() already has a BUG_ON, future implementation changes
of firmware upload framework will produce a call trace anyway. So, no need
to duplicate the debug code.
^ permalink raw reply
* Re: [PATCH v4 08/24] x86/virt/seamldr: Block TDX Module updates if any CPU is offline
From: Chao Gao @ 2026-03-13 8:17 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <7b3babcd-0b09-4d36-b713-3e55ded1696d@intel.com>
On Thu, Mar 12, 2026 at 01:20:27PM -0700, Dave Hansen wrote:
>On 2/12/26 06:35, Chao Gao wrote:
>> P-SEAMLDR requires every CPU to call SEAMLDR.INSTALL during updates. So,
>> every CPU should be online during updates.
>
>Gah, how did another one of these creep in? We've already fixed like a
>half dozen of these.
>
>There needs to be a *LONG* justification why there is no other choice
>here. There are very good reasons to leave CPUs offline forever.
I will drop this patch.
For the record:
This patch was added in v2 after testing revealed that module updates with offline
CPUs would fail and kill all TDs. I attempted to provide graceful handling in the
kernel.
But "all-CPUs-online" is a temporary TDX module limitation that will be
resolved in future releases.
So, adding kernel complexity for this isn't warranted. Admins can verify all
CPUs are online before updating. This is consistent with how this series
already expects users to do compatibility verification.
^ permalink raw reply
* Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-03-13 8:02 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Farrah Chen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin
In-Reply-To: <85e42dab-865b-425c-bd31-fba7ff5d3443@intel.com>
On Thu, Mar 12, 2026 at 01:14:33PM -0700, Dave Hansen wrote:
>On 2/12/26 06:35, Chao Gao wrote:
>> +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
>> +{
>> + /*
>> + * Serialize P-SEAMLDR calls and disable interrupts as the calls
>> + * can be made from IRQ context.
>> + */
>> + guard(raw_spinlock_irqsave)(&seamldr_lock);
>> + return seamcall_prerr(fn, args);
>> +}
>
>What SEAMLDR calls are getting made from IRQ context?
No, I confused IRQ context with interrupt-disabled context.
SEAMLDR calls happen in two scenarios:
1. Userspace reads num_remaining_updates/seamldr version (interrupts enabled)
2. stop_machine() calls SEAMLDR to install updates (interrupts disabled)
Both run in process context, just with different interrupt states.
(I mistakenly thought case 2 was IRQ context)
>
>Why does this need to be raw_?
In RT kernel, a plain spinlock becomes sleeping lock. it cannot be called when
interrupt disabled (in case 2). I verified this by changing the lock to a plain
spinlock, I got this splat regardless of _irqsave version is used or not:
BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 1772, name: migration/192
preempt_count: 1, expected: 0
RCU nest depth: 0, expected: 0
1 lock held by migration/192/1772:
#0: ffffffff834747e0 (seamldr_lock){+.+.}-{3:3}, at: seamldr_call+0x3a/0x1c0
irq event stamp: 1070
hardirqs last enabled at (1069): [<ffffffff828ea7e8>] _raw_spin_unlock_irq+0x28/0x60
hardirqs last disabled at (1070): [<ffffffff814a1ae0>] multi_cpu_stop+0xc0/0x140
softirqs last enabled at (0): [<ffffffff81313dcf>] copy_process+0xaaf/0x22a0
softirqs last disabled at (0): [<0000000000000000>] 0x0
Preemption disabled at:
[<ffffffff814a1397>] cpu_stopper_thread+0x97/0x140
So, I will use:
guard(raw_spinlock)(&seamldr_lock);
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Dan Williams @ 2026-03-13 4:11 UTC (permalink / raw)
To: Greg KH, Dan Williams
Cc: linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu, bhelgaas,
alistair23, lukas, jgg, Christoph Hellwig, Jason Gunthorpe,
Marek Szyprowski, Robin Murphy, Roman Kisel, Samuel Ortiz,
Rafael J. Wysocki, Danilo Krummrich
In-Reply-To: <2026031230-mastiff-create-7593@gregkh>
Greg KH wrote:
> On Mon, Mar 02, 2026 at 04:01:51PM -0800, Dan Williams wrote:
> > An "accepted" device is one that is allowed to access private memory within
> > a Trusted Computing Boundary (TCB). The concept of "acceptance" is distinct
> > from other device properties like usb_device::authorized, or
> > tb_switch::authorized. The entry to the accepted state is a violent
> > operation in which the device will reject MMIO requests that are not
> > encrypted, and the device enters a new IOMMU protection domain to allow it
> > to access addresses that were previously off-limits.
>
> Trying to mix/match "acceptance" with "authorized" is going to be a
> nightmare, what's the combination that can happen here over time?
I do think Linux needs to mix/match these concepts. "Authorization" is a
kernel policy to operate a device at all. "Acceptance" is a mechanism
to operate a device within a hardware TCB boundary.
So, the truth table of combinations would be:
accepted authorized result
0 0 logically and physically disconnected device
0 1 connected device, DMA is bounce buffered
and loses confidentiality, integrity,
and performance
1 0 logically disconnected device, but a
relying party trusts that the device is
not spoofing authorization.
1 1 connected device, DMA is direct and gains
confidentiality, integrity and performance
To say it another way, when the above distinguishes "logically" vs
"physically" disconnected it is whether the device interface can be
verified to not be under adversarial control. An unaccepted device can
do limited damage, but still can bounce buffer secrets out of the TCB if
so directed.
> We need to either "trust" or "not trust" the device, and the bus can
> decide what to do with that value (if anything). The DMA layer can then
> use that value to do:
Trust is separate. For example, there are deployed use cases today where
the device is trusted, but unaccepted. Acceptance support for those
cases is mostly a performance optimization to be able to stop performing
software encryption on top of DMA bounce buffering.
> > Subsystems like the DMA mapping layer, that need to modify their behavior
> > based on the accept state, may only have access to the base 'struct
> > device'.
>
> ^this.
The DMA layer is not operating on a trust concept it is effectively
being told to select an IOMMU.
> > It is also likely that the concept of TCB acceptance grows beyond
> > PCI devices over time. For these reasons, introduce the concept of
> > acceptance in 'struct device_private' which is device common, but only
> > suitable for buses and built-in infrastructure to consume.
>
> Busses are what can control this, but please, let's not make this a
> cc-only type thing. We have the idea of trust starting to propagate
> through a number of different busses, let's get it right here, so we
> don't have to have all of these different bus-specific hacks like we do
> today.
The conflation of "trust" and "acceptance" has been the main stumbling
block of past proposals. As you have said before "kernel drivers trust
their devices". That precedent is not being touched in this proposal.
Instead, give userspace all the tools it needs to deploy policy about
when to operate a device. When it does decide to operate the device give
it the mechanism to add confidentiality, integrity and performance to
that operation.
This is a "CC-only type thing" because only CC partitions the system
into two device domains. One where "trusted unaccepted" devices can
operate without CC protections and "trusted accepted" devices can
operate with CC protections and direct DMA.
>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: Roman Kisel <romank@linux.microsoft.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Samuel Ortiz <sameo@rivosinc.com>
> > Cc: Alexey Kardashevskiy <aik@amd.com>
> > Cc: Xu Yilun <yilun.xu@linux.intel.com>
> > Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Cc: Danilo Krummrich <dakr@kernel.org>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> > drivers/base/Kconfig | 4 +++
> > drivers/base/Makefile | 1 +
> > drivers/base/base.h | 9 +++++++
> > include/linux/device.h | 22 ++++++++++++++++
> > drivers/base/coco.c | 58 ++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 94 insertions(+)
> > create mode 100644 drivers/base/coco.c
> >
> > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> > index 1786d87b29e2..d4743bf978ec 100644
> > --- a/drivers/base/Kconfig
> > +++ b/drivers/base/Kconfig
> > @@ -249,4 +249,8 @@ config FW_DEVLINK_SYNC_STATE_TIMEOUT
> > command line option on every system/board your kernel is expected to
> > work on.
> >
> > +config CONFIDENTIAL_DEVICES
> > + depends on ARCH_HAS_CC_PLATFORM
> > + bool
> > +
> > endmenu
> > diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> > index 8074a10183dc..e11052cd5253 100644
> > --- a/drivers/base/Makefile
> > +++ b/drivers/base/Makefile
> > @@ -27,6 +27,7 @@ obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
> > obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
> > obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
> > obj-$(CONFIG_ACPI) += physical_location.o
> > +obj-$(CONFIG_CONFIDENTIAL_DEVICES) += coco.o
> >
> > obj-y += test/
> >
> > diff --git a/drivers/base/base.h b/drivers/base/base.h
> > index b68355f5d6e3..1ae9a1679504 100644
> > --- a/drivers/base/base.h
> > +++ b/drivers/base/base.h
> > @@ -119,8 +119,13 @@ struct driver_type {
> > * @dead: This device is currently either in the process of or has been
> > * removed from the system. Any asynchronous events scheduled for this
> > * device should exit without taking any action.
> > + * @cc_accepted: track the TEE acceptance state of the device for deferred
> > + * probing, MMIO mapping type, and SWIOTLB bypass for private memory DMA.
> > *
> > * Nothing outside of the driver core should ever touch these fields.
> > + *
> > + * All bitfield flags are manipulated under device_lock() to avoid
> > + * read-modify-write collisions.
> > */
> > struct device_private {
> > struct klist klist_children;
> > @@ -136,6 +141,10 @@ struct device_private {
> > struct driver_type driver_type;
> > #endif
> > u8 dead:1;
> > +#ifdef CONFIG_CONFIDENTIAL_DEVICES
> > + u8 cc_accepted:1;
> > +#endif
>
> Just make this:
> u8 trusted:1;
>
> no need for an #ifdef.
Ok.
>
>
> > +
> > };
> > #define to_device_private_parent(obj) \
> > container_of(obj, struct device_private, knode_parent)
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index 0be95294b6e6..4470365d772b 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -1191,6 +1191,28 @@ static inline bool device_link_test(const struct device_link *link, u32 flags)
> > return !!(link->flags & flags);
> > }
> >
> > +/* Confidential Device state helpers */
> > +#ifdef CONFIG_CONFIDENTIAL_DEVICES
> > +int device_cc_accept(struct device *dev);
> > +int device_cc_reject(struct device *dev);
> > +bool device_cc_accepted(struct device *dev);
> > +#else
> > +static inline int device_cc_accept(struct device *dev)
>
> No __must_hold() usage? That's best to check this at build time, not
> just relying on:
>
> > +{
> > + lockdep_assert_held(&dev->mutex);
>
> runtime checks.
>
> Same for all the calls here.
Ok, will take a look and convert.
> > +/**
> > + * device_cc_accept(): Mark a device as able to access private memory
> > + * @dev: device to accept
> > + *
> > + * Confidential bus drivers use this helper to accept devices. For example, PCI
> > + * has a sysfs ABI to accept devices after relying party attestation.
> > + *
> > + * Given that moving a device into confidential / private operation implicates
> > + * changes to MMIO mapping attributes and DMA mappings, the transition must be
> > + * done while the device is idle (driver detached).
> > + */
> > +int device_cc_accept(struct device *dev)
> > +{
> > + lockdep_assert_held(&dev->mutex);
> > +
> > + if (dev->driver)
> > + return -EBUSY;
>
> So you are saying that once a driver is bound, it is "trusted"? That's
> fine, but maybe you don't want to do that in the core, shouldn't that be
> a bus-specific thing?
No, per above this not about trust. This is about the fact that the
device can not switch between DMA/IOMMU and encrypted MMIO operational
models while it may have active DMA or MMIO mappings. So the check here
is to observe that the mechanism of toggling inside / outside TCB
operation is so violent as to not be something that can change while the
device is being operated by a driver.
>
> this could then be:
> int device_trust(struct device *dev);
> int device_untrust(struct device *dev); /* ugh, bad name, pick something else? */
> bool device_trusted(struct device *dev);
>
> but note, do you ever want to move a device from trusted to untrusted?
> What would cause that?
Moving to unaccepted operation is when any detail that the relying party
used to make "accept" decision changed. For example, the device moves
to the ERROR security state from severe events like PCIe link encryption
loss, to modest events like the host VMM toggled the bus-master-enable
bit in the physical function's command register.
So error recovery takes the device from RUN to ERROR before it can be
transitioned back to LOCK then RUN (accepted). But also a coordinated
firmware update would need to take the device from RUN to UNLOCKED to
LOCKED (revalidate device evidence) back to RUN.
^ permalink raw reply
* Re: [PATCH 4/7] KVM: x86: Add wrapper APIs to reset dirty/available register masks
From: Sean Christopherson @ 2026-03-13 0:38 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Yosry Ahmed, Kiryl Shutsemau, kvm, x86, linux-coco, linux-kernel,
Chang S . Bae
In-Reply-To: <c117141f-61da-4572-98ea-7a8a79eb5bab@redhat.com>
On Wed, Mar 11, 2026, Paolo Bonzini wrote:
> On 3/11/26 14:31, Sean Christopherson wrote:
> > > Not closely following this series and don't know this code well, but
> > > this API is very confusing for me tbh. Especially in comparison with
> > > kvm_reset_dirty_registers().
> > >
> > > Maybe rename this to kvm_clear_available_registers(), and pass in a
> > > "clear_mask", then reverse the polarity:
> > >
> > > vcpu->arch.regs_avail &= ~clear_mask;
> > Oh, yeah, I can do something like that. I originally misread the TDX code and
> > thought it was explicitly setting regs_avail, and so came up with a roundabout
> > name. I didn't revisit the naming or the polarity of the param once I realized
> > all callers could use the same scheme.
> >
> > No small part of me is tempted to turn it into a straigh "set" though, unless I'm
> > missing something, the whole &= business is an implementation quirk.
>
> I like kvm_clear_available_registers() for this + removing the second
> argument completely for kvm_reset_dirty_registers().
Ya, me too. I almost dropped the param for kvm_reset_dirty_registers(), but
wanted symmetry since the names were the same. But I like this a lot more.
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Dave Hansen @ 2026-03-12 20:40 UTC (permalink / raw)
To: Chao Gao, linux-coco, linux-kernel, kvm, x86
Cc: reinette.chatre, ira.weiny, kai.huang, dan.j.williams, yilun.xu,
sagis, vannapurve, paulmck, nik.borisov, zhenzhong.duan, seanjc,
rick.p.edgecombe, kas, dave.hansen, vishal.l.verma, binbin.wu,
tony.lindgren, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin
In-Reply-To: <20260212143606.534586-12-chao.gao@intel.com>
On 2/12/26 06:35, Chao Gao wrote:
> +static void set_target_state(enum tdp_state state)
> +{
> + /* Reset ack counter. */
> + atomic_set(&tdp_data.thread_ack, num_online_cpus());
> + /* Ensure thread_ack is updated before the new state */
> + smp_wmb();
> + WRITE_ONCE(tdp_data.state, state);
> +}
This looks overly complicated.
If it doesn't need to be scalable, just make it stupid and simple. Why
not just protect the whole thing with a spinlock and be done with it?
^ permalink raw reply
* [PATCH v2 16/16] fs/resctrl: Add per-task kmode enable support via rdtgroup
From: Babu Moger @ 2026-03-12 20:37 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <cover.1773347820.git.babu.moger@amd.com>
Introduce support for enabling kmode on a per-task basis through the
resctrl control-group interface.
Add an architecture helper to set the kmode state in the task structure and
extend the rdtgroup task handling path to apply kmode (e.g. PLZA) when
associating a task with a CTRL_MON or MON group.
Proper memory ordering is enforced to ensure that task closid and rmid
updates are visible before determining whether the task is currently
running. If the task is active on a CPU, the relevant MSRs are updated
immediately; otherwise, PLZA state is programmed on the next context
switch.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Few name changes to refer PLZA as kmode.
---
arch/x86/include/asm/resctrl.h | 13 +++++
fs/resctrl/rdtgroup.c | 98 +++++++++++++++++++++++++++++++++-
2 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index ccfd95b98bac..f48d1279e33d 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -238,6 +238,19 @@ static inline void resctrl_arch_set_cpu_kmode(int cpu, u32 closid, u32 rmid, u32
WRITE_ONCE(per_cpu(pqr_state.kmode_rmid, cpu), rmid);
}
+/**
+ * resctrl_arch_set_task_kmode() - Set per-task kernel mode (e.g. PLZA) flag
+ * @tsk: Task to update.
+ * @enable: 1 to enable kmode for this task; 0 to disable.
+ *
+ * When enabled, the task will use the group's CLOSID/RMID for kernel mode
+ * on context switch (see __resctrl_sched_in()).
+ */
+static inline void resctrl_arch_set_task_kmode(struct task_struct *tsk, u32 enable)
+{
+ WRITE_ONCE(tsk->kmode, enable);
+}
+
static inline void resctrl_arch_sched_in(struct task_struct *tsk)
{
if (static_branch_likely(&rdt_enable_key))
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index b41e681f6922..74fc942e6a4e 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -827,6 +827,31 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
return 0;
}
+/**
+ * __rdtgroup_task_kmode() - Enable kernel mode (e.g. PLZA) for a single task
+ * @tsk: Task to enable kmode for.
+ * @rdtgrp: Rdtgroup with kmode enabled (used for context; CLOSID/RMID applied on sched-in).
+ *
+ * Sets t->kmode so that the task uses the group's CLOSID/RMID on context
+ * switch. Memory ordering ensures the store is visible before we check if
+ * the task is current (and thus before any sched-in that may observe it).
+ *
+ * Return: 0.
+ */
+static int __rdtgroup_task_kmode(struct task_struct *tsk, struct rdtgroup *rdtgrp)
+{
+ resctrl_arch_set_task_kmode(tsk, true);
+
+ /*
+ * Order the task's kmode state stores above before the loads in
+ * task_curr(). This pairs with the full barrier between the
+ * rq->curr update and resctrl_arch_sched_in() during context switch.
+ */
+ smp_mb();
+
+ return 0;
+}
+
static bool is_closid_match(struct task_struct *t, struct rdtgroup *r)
{
return (resctrl_arch_alloc_capable() && (r->type == RDTCTRL_GROUP) &&
@@ -916,6 +941,48 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp,
return ret;
}
+/**
+ * rdtgroup_task_kmode() - Enable kernel mode for a task added to a kmode group
+ * @pid: PID of the task (0 for current).
+ * @rdtgrp: Rdtgroup with kmode enabled.
+ * @of: kernfs file (for permission check).
+ *
+ * Called when a task is written to the "tasks" file of a group that has
+ * kernel mode enabled. Enables kmode for that task so it uses the group's
+ * CLOSID/RMID on context switch. If the task is currently running, MSRs are
+ * updated on next sched-in.
+ *
+ * Return: 0 on success, or -ESRCH/-EPERM on error.
+ */
+static int rdtgroup_task_kmode(pid_t pid, struct rdtgroup *rdtgrp,
+ struct kernfs_open_file *of)
+{
+ struct task_struct *tsk;
+ int ret;
+
+ rcu_read_lock();
+ if (pid) {
+ tsk = find_task_by_vpid(pid);
+ if (!tsk) {
+ rcu_read_unlock();
+ rdt_last_cmd_printf("No task %d\n", pid);
+ return -ESRCH;
+ }
+ } else {
+ tsk = current;
+ }
+
+ get_task_struct(tsk);
+ rcu_read_unlock();
+
+ ret = rdtgroup_task_write_permission(tsk, of);
+ if (!ret)
+ ret = __rdtgroup_task_kmode(tsk, rdtgrp);
+
+ put_task_struct(tsk);
+ return ret;
+}
+
static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
char *buf, size_t nbytes, loff_t off)
{
@@ -953,7 +1020,11 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
break;
}
- ret = rdtgroup_move_task(pid, rdtgrp, of);
+ /* Group has kmode: set task kmode; else move task CLOSID/RMID. */
+ if (rdtgrp->kmode)
+ ret = rdtgroup_task_kmode(pid, rdtgrp, of);
+ else
+ ret = rdtgroup_move_task(pid, rdtgrp, of);
if (ret) {
rdt_last_cmd_printf("Error while processing task %d\n", pid);
break;
@@ -1011,6 +1082,28 @@ static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
rcu_read_unlock();
}
+/**
+ * rdt_task_set_kmode() - Set or clear kmode for all tasks in the rdtgroup
+ * @r: Rdtgroup (must have r->kmode set for matching).
+ * @kmode: True to set t->kmode for each matching task; false to clear.
+ *
+ * Walks all tasks that belong to @r (via rdt_task_match) and updates their
+ * per-task kmode flag. Used when enabling or disabling kernel mode for the
+ * group so existing members get the new state.
+ */
+static void rdt_task_set_kmode(struct rdtgroup *r, bool kmode)
+{
+ struct task_struct *p, *t;
+
+ rcu_read_lock();
+ for_each_process_thread(p, t) {
+ if (!rdt_task_match(t, r, r->kmode))
+ continue;
+ resctrl_arch_set_task_kmode(t, kmode);
+ }
+ rcu_read_unlock();
+}
+
static int rdtgroup_tasks_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
@@ -1225,6 +1318,9 @@ static int rdtgroup_config_kmode(struct rdtgroup *rdtgrp, bool enable)
resctrl_arch_set_kmode(&rdtgrp->cpu_mask, &resctrl_kcfg, closid,
rdtgrp->mon.rmid, enable);
+
+ rdt_task_set_kmode(rdtgrp, enable);
+
rdtgrp->kmode = enable;
if (enable)
resctrl_kcfg.k_rdtgrp = rdtgrp;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 15/16] x86/resctrl: Refactor show_rdt_tasks() to support PLZA tasks
From: Babu Moger @ 2026-03-12 20:37 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <cover.1773347820.git.babu.moger@amd.com>
Refactor show_rdt_tasks() to use a new rdt_task_match() helper that checks
t->kmode when kmode (e.g. PLZA) is enabled for a group, falling back to
CLOSID/RMID matching otherwise. This ensures correct task display for
PLZA-enabled groups.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Added more code comments for clarity.
---
fs/resctrl/rdtgroup.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 31479893633a..b41e681f6922 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -966,6 +966,34 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
return ret ?: nbytes;
}
+/**
+ * rdt_task_match() - Decide if a task belongs to an rdtgroup for display
+ * @t: Task to check.
+ * @r: Rdtgroup (for CLOSID/RMID matching when not kmode).
+ * @kmode: True if @r has kernel mode (e.g. PLZA) enabled.
+ *
+ * When @kmode is true, matches tasks that have kernel mode set (they are
+ * associated with this group via PLZA). Otherwise matches by CLOSID or RMID.
+ *
+ * Return: true if @t should be shown as belonging to @r.
+ */
+static inline bool rdt_task_match(struct task_struct *t,
+ struct rdtgroup *r, bool kmode)
+{
+ if (kmode)
+ return t->kmode;
+
+ return is_closid_match(t, r) || is_rmid_match(t, r);
+}
+
+/**
+ * show_rdt_tasks() - List task PIDs that belong to the rdtgroup
+ * @r: Rdtgroup whose tasks to list.
+ * @s: seq_file to write PIDs to.
+ *
+ * Uses rdt_task_match() so that when the group has kernel mode (e.g. PLZA)
+ * enabled, tasks are matched by t->kmode; otherwise by CLOSID/RMID.
+ */
static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
{
struct task_struct *p, *t;
@@ -973,11 +1001,12 @@ static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
rcu_read_lock();
for_each_process_thread(p, t) {
- if (is_closid_match(t, r) || is_rmid_match(t, r)) {
- pid = task_pid_vnr(t);
- if (pid)
- seq_printf(s, "%d\n", pid);
- }
+ if (!rdt_task_match(t, r, r->kmode))
+ continue;
+
+ pid = task_pid_vnr(t);
+ if (pid)
+ seq_printf(s, "%d\n", pid);
}
rcu_read_unlock();
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 14/16] fs/resctrl: Update kmode configuration when cpu_mask changes
From: Babu Moger @ 2026-03-12 20:36 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <cover.1773347820.git.babu.moger@amd.com>
When kernel mode (e.g. PLZA) is active for a resctrl group, per-CPU
state must stay in sync with the group's cpu_mask. If the user
changes the cpus file, we must enable kmode on newly added CPUs and
disable it on CPUs that left the group.
Add cpus_write_kmode(), which calls cpus_ctrl_write_kmode() for
CTRL_MON groups and cpus_mon_write_kmode() for MON groups.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Fixed few typos in commit message.
Added separate functions to handle kmode configuration for CTRL_MON and MON groups.
---
fs/resctrl/rdtgroup.c | 149 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 148 insertions(+), 1 deletion(-)
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 23e610d59111..31479893633a 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -456,6 +456,150 @@ static void cpumask_rdtgrp_clear(struct rdtgroup *r, struct cpumask *m)
cpumask_and(&crgrp->cpu_mask, &r->cpu_mask, &crgrp->cpu_mask);
}
+/**
+ * cpus_mon_write_kmode() - Update per-CPU kmode when a MON group's cpu_mask changes
+ * @rdtgrp: The MON group whose cpu_mask is being updated.
+ * @newmask: The new CPU mask requested by the user.
+ * @tmpmask: Temporary mask for computing CPU set differences.
+ *
+ * When CPUs are dropped from the group, disables kmode on those CPUs and
+ * returns them to the parent. When CPUs are added, removes them from sibling
+ * MON groups and enables kmode on them. Caller must hold rdtgroup_mutex.
+ *
+ * Return: 0 on success, or -EINVAL if newmask contains CPUs outside the parent.
+ */
+static int cpus_mon_write_kmode(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
+ cpumask_var_t tmpmask)
+{
+ struct rdtgroup *prgrp = rdtgrp->mon.parent, *crgrp;
+ struct list_head *head;
+
+ /* Check whether cpus belong to parent ctrl group */
+ cpumask_andnot(tmpmask, newmask, &prgrp->cpu_mask);
+ if (!cpumask_empty(tmpmask)) {
+ rdt_last_cmd_puts("Can only add CPUs to mongroup that belong to parent\n");
+ return -EINVAL;
+ }
+
+ /* Check whether cpus are dropped from this group */
+ cpumask_andnot(tmpmask, &rdtgrp->cpu_mask, newmask);
+ if (!cpumask_empty(tmpmask)) {
+ /* Give any dropped cpus to parent rdtgroup */
+ cpumask_or(&prgrp->cpu_mask, &prgrp->cpu_mask, tmpmask);
+
+ /* Disable kmode on the dropped CPUs */
+ resctrl_arch_set_kmode(tmpmask, &resctrl_kcfg, prgrp->closid,
+ rdtgrp->mon.rmid, false);
+ }
+
+ /*
+ * If we added cpus, remove them from previous group that owned them
+ * and enable kmode on added CPUs.
+ */
+ cpumask_andnot(tmpmask, newmask, &rdtgrp->cpu_mask);
+ if (!cpumask_empty(tmpmask)) {
+ head = &prgrp->mon.crdtgrp_list;
+ list_for_each_entry(crgrp, head, mon.crdtgrp_list) {
+ if (crgrp == rdtgrp)
+ continue;
+ cpumask_andnot(&crgrp->cpu_mask, &crgrp->cpu_mask, tmpmask);
+ }
+ resctrl_arch_set_kmode(tmpmask, &resctrl_kcfg, prgrp->closid,
+ rdtgrp->mon.rmid, true);
+ }
+
+ /* Done pushing/pulling - update this group with new mask */
+ cpumask_copy(&rdtgrp->cpu_mask, newmask);
+
+ return 0;
+}
+
+/**
+ * cpus_ctrl_write_kmode() - Update per-CPU kmode when a CTRL group's cpu_mask changes
+ * @rdtgrp: The CTRL_MON group whose cpu_mask is being updated.
+ * @newmask: The new CPU mask requested by the user.
+ * @tmpmask: Temporary mask for computing CPU set differences.
+ * @tmpmask1: Second temporary mask (e.g. for cpumask_rdtgrp_clear).
+ *
+ * When CPUs are dropped from the group, disables kmode on those CPUs (cannot
+ * drop from default group). When CPUs are added, clears them from child groups
+ * that owned them and enables kmode on them. Updates this group's cpu_mask and
+ * intersects child MON group masks with the new parent mask. Caller must hold
+ * rdtgroup_mutex.
+ *
+ * Return: 0 on success, or -EINVAL if dropping CPUs from the default group.
+ */
+static int cpus_ctrl_write_kmode(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
+ cpumask_var_t tmpmask, cpumask_var_t tmpmask1)
+{
+ struct rdtgroup *crgrp;
+ struct list_head *head;
+
+ /* Check whether cpus are dropped from this group */
+ cpumask_andnot(tmpmask, &rdtgrp->cpu_mask, newmask);
+ if (!cpumask_empty(tmpmask)) {
+ /* Can't drop from default group */
+ if (rdtgrp == &rdtgroup_default) {
+ rdt_last_cmd_puts("Can't drop CPUs from default group\n");
+ return -EINVAL;
+ }
+ /* Disable kmode on the dropped CPUs */
+ resctrl_arch_set_kmode(tmpmask, &resctrl_kcfg, rdtgrp->closid,
+ rdtgrp->mon.rmid, false);
+ }
+
+ /*
+ * If we added cpus, remove them from child groups that owned them
+ * previously.
+ */
+ cpumask_andnot(tmpmask, newmask, &rdtgrp->cpu_mask);
+ if (!cpumask_empty(tmpmask)) {
+ cpumask_rdtgrp_clear(rdtgrp, tmpmask1);
+ /* Enable kmode on the added CPUs */
+ resctrl_arch_set_kmode(tmpmask, &resctrl_kcfg, rdtgrp->closid,
+ rdtgrp->mon.rmid, true);
+ }
+
+ /* Done pushing/pulling - update this group with new mask */
+ cpumask_copy(&rdtgrp->cpu_mask, newmask);
+
+ /* Clear child mon group masks since there is a new parent mask now */
+ head = &rdtgrp->mon.crdtgrp_list;
+ list_for_each_entry(crgrp, head, mon.crdtgrp_list) {
+ cpumask_and(tmpmask, &rdtgrp->cpu_mask, &crgrp->cpu_mask);
+ }
+
+ return 0;
+}
+
+/**
+ * cpus_write_kmode() - Update per-CPU kmode for a group's new cpu_mask
+ * @rdtgrp: The group (CTRL_MON or MON) whose cpu_mask is being updated.
+ * @newmask: The new CPU mask requested by the user.
+ * @tmpmask: Temporary mask for computing CPU set differences.
+ * @tmpmask1: Second temporary mask (only used for CTRL_MON groups).
+ *
+ * Dispatches to cpus_ctrl_write_kmode() or cpus_mon_write_kmode() based on
+ * group type. Used when the group has kmode enabled and the user writes to
+ * the cpus file.
+ *
+ * Return: 0 on success, or -EINVAL on error.
+ */
+static int cpus_write_kmode(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
+ cpumask_var_t tmpmask, cpumask_var_t tmpmask1)
+{
+ int ret;
+
+ if (rdtgrp->type == RDTCTRL_GROUP)
+ ret = cpus_ctrl_write_kmode(rdtgrp, newmask, tmpmask, tmpmask1);
+ else if (rdtgrp->type == RDTMON_GROUP)
+ ret = cpus_mon_write_kmode(rdtgrp, newmask, tmpmask);
+ else
+ ret = -EINVAL;
+
+ return ret;
+}
+
static int cpus_ctrl_write(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
cpumask_var_t tmpmask, cpumask_var_t tmpmask1)
{
@@ -566,7 +710,10 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
goto unlock;
}
- if (rdtgrp->type == RDTCTRL_GROUP)
+ /* Group has kernel mode: update per-CPU kmode state for new mask. */
+ if (rdtgrp->kmode)
+ ret = cpus_write_kmode(rdtgrp, newmask, tmpmask, tmpmask1);
+ else if (rdtgrp->type == RDTCTRL_GROUP)
ret = cpus_ctrl_write(rdtgrp, newmask, tmpmask, tmpmask1);
else if (rdtgrp->type == RDTMON_GROUP)
ret = cpus_mon_write(rdtgrp, newmask, tmpmask);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 13/16] fs/resctrl: Add write interface for kernel_mode_assignment
From: Babu Moger @ 2026-03-12 20:36 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <cover.1773347820.git.babu.moger@amd.com>
Allow enabling kernel mode assignment (PLZA) for resctrl groups via the
kernel_mode_assignment sysfs file. Add a kmode flag to struct rdtgroup to
track the state, enforce that only one group has PLZA at a time, and clear
kmode when groups are removed or during rmdir_all_sub teardown.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: New patch to handle PLZA interfaces with /sys/fs/resctrl/info/ directory.
https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
---
Documentation/filesystems/resctrl.rst | 35 ++++++
fs/resctrl/rdtgroup.c | 148 +++++++++++++++++++++++++-
2 files changed, 182 insertions(+), 1 deletion(-)
diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
index 2107dd4b3649..2b4beedd7207 100644
--- a/Documentation/filesystems/resctrl.rst
+++ b/Documentation/filesystems/resctrl.rst
@@ -548,6 +548,41 @@ conveyed in the error returns from file operations. E.g.
- "global_assign_ctrl_assign_mon": One resource group (CLOSID and RMID)
is assigned for all kernel work.
+"kernel_mode_assignment":
+ In the top level of the "info" directory, "kernel_mode_assignment" shows
+ and (when a global-assign kernel mode is active) sets which resctrl group
+ is used for kernel mode. It is only relevant when "kernel_mode" is not
+ "inherit_ctrl_and_mon".
+
+ Reading the file shows the currently assigned group in the form
+ "CTRL_MON/MON/" with a newline::
+
+ # cat info/kernel_mode_assignment
+ //
+
+ Possible read formats:
+
+ - "//": Default CTRL_MON group is assigned.
+ - "ctrl_name//": A CTRL_MON group named "ctrl_name" is assigned.
+ - "/mon_name/": A MON group named "mon_name" under the default CTRL_MON
+ group is assigned.
+ - "ctrl_name/mon_name/": A MON group named "mon_name" under the CTRL_MON
+ group "ctrl_name" is assigned.
+ - "Kmode is not configured": No group is assigned for kernel mode.
+
+ Writing assigns a group for kernel mode. The write is only allowed when
+ the current kernel mode is not "inherit_ctrl_and_mon". Input format is
+ one or more lines, each of the form "CTRL_MON/MON/" (same as the read
+ format). Examples::
+
+ # echo "//" > info/kernel_mode_assignment
+ # echo "mydir//" > info/kernel_mode_assignment
+ # echo "mydir/mon1/" > info/kernel_mode_assignment
+
+ An empty write (e.g. ``echo >> info/kernel_mode_assignment``) clears the
+ assignment. Only one group can be assigned at a time. Pseudo-locked
+ groups cannot be assigned. Errors are reported in "info/last_cmd_status".
+
Resource alloc and monitor groups
=================================
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index c2d6d1995dff..23e610d59111 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -1156,6 +1156,137 @@ static int resctrl_kernel_mode_assignment_show(struct kernfs_open_file *of,
return 0;
}
+/**
+ * rdtgroup_find_grp_by_name() - Find an rdtgroup by type and parent/child names
+ * @rtype: RDTCTRL_GROUP or RDTMON_GROUP.
+ * @p_grp: Parent CTRL_MON group name, or "" for the default group.
+ * @c_grp: Child MON group name (only used when rtype is RDTMON_GROUP).
+ *
+ * Return: The rdtgroup, or NULL if not found.
+ */
+static struct rdtgroup *rdtgroup_find_grp_by_name(enum rdt_group_type rtype,
+ char *p_grp, char *c_grp)
+{
+ struct rdtgroup *rdtg, *crg;
+
+ if (rtype == RDTCTRL_GROUP && *p_grp == '\0') {
+ return &rdtgroup_default;
+ } else if (rtype == RDTCTRL_GROUP) {
+ list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list)
+ if (rdtg->type == RDTCTRL_GROUP && !strcmp(p_grp, rdtg->kn->name))
+ return rdtg;
+ } else if (rtype == RDTMON_GROUP) {
+ list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list) {
+ if (rdtg->type == RDTCTRL_GROUP && !strcmp(p_grp, rdtg->kn->name)) {
+ list_for_each_entry(crg, &rdtg->mon.crdtgrp_list,
+ mon.crdtgrp_list) {
+ if (!strcmp(c_grp, crg->kn->name))
+ return crg;
+ }
+ }
+ }
+ }
+
+ return NULL;
+}
+
+/**
+ * resctrl_kernel_mode_assignment_write() - Set rdtgroup for kernel mode via info file
+ * @of: kernfs file handle.
+ * @buf: Input: "CTRL_MON/MON/" per line (e.g. "//" for default,
+ * "ctrl1//" or "ctrl1/mon1/"); empty string clears the assignment.
+ * @nbytes: Length of buf.
+ * @off: File offset (unused).
+ *
+ * Only valid when kernel mode is not inherit_ctrl_and_mon. Empty write clears
+ * the current assignment. Parses lines as "parent/child/"; empty child means
+ * CTRL_MON group. Errors are reported in last_cmd_status.
+ *
+ * Return: nbytes on success, or -EINVAL with last_cmd_status set on error.
+ */
+static ssize_t resctrl_kernel_mode_assignment_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ struct rdtgroup *rdtgrp;
+ char *token, *cmon_grp, *mon_grp;
+ enum rdt_group_type rtype;
+ int ret = 0;
+
+ if (nbytes == 0 || buf[nbytes - 1] != '\n')
+ return -EINVAL;
+ buf[nbytes - 1] = '\0';
+ buf = strim(buf);
+
+ mutex_lock(&rdtgroup_mutex);
+ rdt_last_cmd_clear();
+
+ if (resctrl_kcfg.kmode_cur & INHERIT_CTRL_AND_MON) {
+ rdt_last_cmd_puts("Cannot change kmode in inherit_ctrl_and_mon\n");
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ /*
+ * Group can be deleted from Kmode by empty write: e.g.
+ * "echo >> /sys/fs/resctrl/info/kernel_mode_assignment"
+ */
+ if (*buf == '\0') {
+ if (resctrl_kcfg.k_rdtgrp) {
+ ret = rdtgroup_config_kmode(resctrl_kcfg.k_rdtgrp, false);
+ if (ret)
+ rdt_last_cmd_printf("Kernel mode disable failed on group %s\n",
+ rdt_kn_name(resctrl_kcfg.k_rdtgrp->kn));
+ }
+ goto out_unlock;
+ }
+
+ /* Only one group can be assigned for kernel mode at a time. */
+ if (resctrl_kcfg.k_rdtgrp) {
+ rdt_last_cmd_printf("Kernel mode already configured on group %s\n",
+ rdt_kn_name(resctrl_kcfg.k_rdtgrp->kn));
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ while ((token = strsep(&buf, "\n")) != NULL) {
+ /*
+ * Each line has the format "<CTRL_MON group>/<MON group>/".
+ * Extract the CTRL_MON group name.
+ */
+ cmon_grp = strsep(&token, "/");
+
+ /*
+ * Extract the MON_GROUP.
+ * strsep returns empty string for contiguous delimiters.
+ * Empty mon_grp here means it is a RDTCTRL_GROUP.
+ */
+ mon_grp = strsep(&token, "/");
+
+ if (*mon_grp == '\0')
+ rtype = RDTCTRL_GROUP;
+ else
+ rtype = RDTMON_GROUP;
+
+ rdtgrp = rdtgroup_find_grp_by_name(rtype, cmon_grp, mon_grp);
+
+ if (!rdtgrp) {
+ rdt_last_cmd_puts("Not a valid resctrl group\n");
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ if (!rdtgrp->kmode) {
+ ret = rdtgroup_config_kmode(rdtgrp, true);
+ if (ret)
+ break;
+ }
+ }
+
+out_unlock:
+ mutex_unlock(&rdtgroup_mutex);
+ return ret ?: nbytes;
+}
+
void *rdt_kn_parent_priv(struct kernfs_node *kn)
{
/*
@@ -2067,9 +2198,10 @@ static struct rftype res_common_files[] = {
},
{
.name = "kernel_mode_assignment",
- .mode = 0444,
+ .mode = 0644,
.kf_ops = &rdtgroup_kf_single_ops,
.seq_show = resctrl_kernel_mode_assignment_show,
+ .write = resctrl_kernel_mode_assignment_write,
.fflags = RFTYPE_TOP_INFO,
},
{
@@ -3248,6 +3380,10 @@ static void rmdir_all_sub(void)
rdt_move_group_tasks(NULL, &rdtgroup_default, NULL);
list_for_each_entry_safe(rdtgrp, tmp, &rdt_all_groups, rdtgroup_list) {
+ /* Disable Kmode if configured */
+ if (rdtgrp->kmode)
+ rdtgroup_config_kmode(rdtgrp, false);
+
/* Free any child rmids */
free_all_child_rdtgrp(rdtgrp);
@@ -3358,6 +3494,8 @@ static void resctrl_fs_teardown(void)
mon_put_kn_priv();
rdt_pseudo_lock_release();
rdtgroup_default.mode = RDT_MODE_SHAREABLE;
+ resctrl_kcfg.k_rdtgrp = NULL;
+ resctrl_kcfg.kmode_cur = INHERIT_CTRL_AND_MON;
closid_exit();
schemata_list_destroy();
rdtgroup_destroy_root();
@@ -4156,6 +4294,10 @@ static int rdtgroup_rmdir_mon(struct rdtgroup *rdtgrp, cpumask_var_t tmpmask)
u32 closid, rmid;
int cpu;
+ /* Disable Kmode if configured */
+ if (rdtgrp->kmode)
+ rdtgroup_config_kmode(rdtgrp, false);
+
/* Give any tasks back to the parent group */
rdt_move_group_tasks(rdtgrp, prdtgrp, tmpmask);
@@ -4206,6 +4348,10 @@ static int rdtgroup_rmdir_ctrl(struct rdtgroup *rdtgrp, cpumask_var_t tmpmask)
u32 closid, rmid;
int cpu;
+ /* Disable Kmode if configured */
+ if (rdtgrp->kmode)
+ rdtgroup_config_kmode(rdtgrp, false);
+
/* Give any tasks back to the default group */
rdt_move_group_tasks(rdtgrp, &rdtgroup_default, tmpmask);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 12/16] fs/resctrl: Add info/kernel_mode_assignment to show kernel-mode rdtgroup
From: Babu Moger @ 2026-03-12 20:36 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <cover.1773347820.git.babu.moger@amd.com>
Add the interface info/kernel_mode_assignment file to show the rdtgroup
enabled for kernel mode (e.g. PLZA).
The assigned rdtgroup list is printed in following format:
"<CTRL_MON group>/<MON group>/"
Format for specific type of groups:
* Default CTRL_MON group:
"//"
* Non-default CTRL_MON group:
"<CTRL_MON group>//"
* Child MON group of default CTRL_MON group:
"/<MON group>/"
* Child MON group of non-default CTRL_MON group:
"<CTRL_MON group>/<MON group>/"
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: New patch to handle PLZA interfaces with /sys/fs/resctrl/info/ directory.
https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
---
fs/resctrl/rdtgroup.c | 46 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 6cd928fabaa2..c2d6d1995dff 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -1117,6 +1117,45 @@ static ssize_t resctrl_kernel_mode_write(struct kernfs_open_file *of,
return ret ?: nbytes;
}
+/**
+ * resctrl_kernel_mode_assignment_show() - Show rdtgroup assigned to kernel mode
+ * @of: kernfs file handle.
+ * @s: seq_file to write to.
+ * @v: unused.
+ *
+ * Prints the rdtgroup (resctrl_kcfg.k_rdtgrp) used for kernel work when a
+ * kernel mode is active (e.g. PLZA).
+ * Format: "CTRL_MON/MON/\n"
+ * "//" for default CTRL_MON,
+ * "ctrl_name//" for a CTRL_MON group,
+ * "/mon_name/" for a MON group under default,
+ * "ctrl_name/mon_name/" otherwise.
+ *
+ * Prints "Kmode is not configured" if no rdtgroup is assigned.
+ */
+static int resctrl_kernel_mode_assignment_show(struct kernfs_open_file *of,
+ struct seq_file *s, void *v)
+{
+ mutex_lock(&rdtgroup_mutex);
+ if (!resctrl_kcfg.k_rdtgrp) {
+ seq_puts(s, "Kmode is not configured");
+ } else if (resctrl_kcfg.k_rdtgrp == &rdtgroup_default) {
+ seq_puts(s, "//");
+ } else if (resctrl_kcfg.k_rdtgrp->type == RDTCTRL_GROUP) {
+ seq_printf(s, "%s//", rdt_kn_name(resctrl_kcfg.k_rdtgrp->kn));
+ } else if (resctrl_kcfg.k_rdtgrp->type == RDTMON_GROUP) {
+ if (resctrl_kcfg.k_rdtgrp->mon.parent == &rdtgroup_default)
+ seq_printf(s, "/%s/", rdt_kn_name(resctrl_kcfg.k_rdtgrp->kn));
+ else
+ seq_printf(s, "%s/%s/",
+ rdt_kn_name(resctrl_kcfg.k_rdtgrp->mon.parent->kn),
+ rdt_kn_name(resctrl_kcfg.k_rdtgrp->kn));
+ }
+ seq_puts(s, "\n");
+ mutex_unlock(&rdtgroup_mutex);
+ return 0;
+}
+
void *rdt_kn_parent_priv(struct kernfs_node *kn)
{
/*
@@ -2026,6 +2065,13 @@ static struct rftype res_common_files[] = {
.write = resctrl_kernel_mode_write,
.fflags = RFTYPE_TOP_INFO,
},
+ {
+ .name = "kernel_mode_assignment",
+ .mode = 0444,
+ .kf_ops = &rdtgroup_kf_single_ops,
+ .seq_show = resctrl_kernel_mode_assignment_show,
+ .fflags = RFTYPE_TOP_INFO,
+ },
{
.name = "mbm_assign_on_mkdir",
.mode = 0644,
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox