* Re: [PATCH v2 2/7] KVM: x86: Extract VMXON and EFER.SVME enablement to kernel
From: Sean Christopherson @ 2025-12-09 20:01 UTC (permalink / raw)
To: dan.j.williams
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Chao Gao
In-Reply-To: <69352b2239a33_1b2e100d2@dwillia2-mobl4.notmuch>
On Sat, Dec 06, 2025, dan.j.williams@intel.com wrote:
> Sean Christopherson wrote:
> > @@ -694,9 +696,6 @@ static void drop_user_return_notifiers(void)
> > kvm_on_user_return(&msrs->urn);
> > }
> >
> > -__visible bool kvm_rebooting;
> > -EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_rebooting);
>
> ...a short stay for this symbol in kvm/x86.c? It raises my curiosity why
> patch1 is separate.
Because it affects non-x86 architectures. It should be a complete nop, but I
wanted to isolate what I could.
> Patch1 looked like the start of a series of incremental conversions, patch2
> is a combo move. I am ok either way, just questioning consistency. I.e. if
> combo move then patch1 folds in here, if incremental, perhaps split out other
> combo conversions like emergency_disable_virtualization_cpu()? The aspect of
> "this got moved twice in the same patchset" is what poked me.
Yeah, I got lazy to a large extent. I'm not super optimistic that we won't end
up with one big "move all this stuff" patch, but I agree it doesn't need to be
_this_ big.
> [..]
> > diff --git a/arch/x86/virt/hw.c b/arch/x86/virt/hw.c
> > new file mode 100644
> > index 000000000000..986e780cf438
> > --- /dev/null
> > +++ b/arch/x86/virt/hw.c
> > @@ -0,0 +1,340 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +#include <linux/cpu.h>
> > +#include <linux/cpumask.h>
> > +#include <linux/errno.h>
> > +#include <linux/kvm_types.h>
> > +#include <linux/list.h>
> > +#include <linux/percpu.h>
> > +
> > +#include <asm/perf_event.h>
> > +#include <asm/processor.h>
> > +#include <asm/virt.h>
> > +#include <asm/vmx.h>
> > +
> > +static int x86_virt_feature __ro_after_init;
> > +
> > +__visible bool virt_rebooting;
> > +EXPORT_SYMBOL_GPL(virt_rebooting);
> > +
> > +static DEFINE_PER_CPU(int, virtualization_nr_users);
> > +
> > +static cpu_emergency_virt_cb __rcu *kvm_emergency_callback;
>
> Hmm, why kvm_ and not virt_?
I was trying to capture that this callback can _only_ be used by KVM, because
KVM is the only in-tree hypervisor. That's also why the exports are only for
KVM (and will use EXPORT_SYMBOL_FOR_KVM() when I post the next version).
> [..]
> > +#if IS_ENABLED(CONFIG_KVM_INTEL)
> > +static DEFINE_PER_CPU(struct vmcs *, root_vmcs);
>
> Perhaps introduce a CONFIG_INTEL_VMX for this? For example, KVM need not
> be enabled if all one wants to do is use TDX to setup PCIe Link
> Encryption. ...or were you expecting?
>
> #if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(...<other VMX users>...)
I don't think we need anything at this time. INTEL_TDX_HOST depends on KVM_INTEL,
and so without a user that needs VMXON without KVM_INTEL, I think we're good as-is.
config INTEL_TDX_HOST
bool "Intel Trust Domain Extensions (TDX) host support"
depends on CPU_SUP_INTEL
depends on X86_64
depends on KVM_INTEL
^ permalink raw reply
* Re: [f2fs-dev] [PATCH kvm-next V11 0/7] Add NUMA mempolicy support for KVM guest-memfd
From: patchwork-bot+f2fs @ 2025-12-09 17:18 UTC (permalink / raw)
To: Garg, Shivank
Cc: willy, akpm, david, pbonzini, shuah, seanjc, vbabka, jgowans,
mhocko, jack, kvm, dhavale, linux-btrfs, aik, papaluri, kalyazin,
peterx, linux-mm, clm, ddutile, linux-kselftest, shdhiman, gshan,
ying.huang, ira.weiny, roypat, matthew.brost, linux-coco,
zbestahu, lorenzo.stoakes, linux-bcachefs, apopple, jmorris, hch,
chao.gao, cgzones, ziy, rientjes, yuzhao, xiang, nikunj, gourry,
serge, thomas.lendacky, ashish.kalra, chao.p.peng, yan.y.zhao,
byungchul, michael.day, Neeraj.Upadhyay, michael.roth, bfoster,
bharata, josef, Liam.Howlett, ackerleytng, dsterba, viro,
jefflexu, jaegeuk, dan.j.williams, surenb, tabba, paul,
joshua.hahnjy, brauner, quic_eberman, rakie.kim, pvorel,
linux-erofs, kent.overstreet, linux-kernel, linux-f2fs-devel,
pankaj.gupta, linux-security-module, lihongbo22, amit,
linux-fsdevel, vannapurve, suzuki.poulose, rppt, jgg
In-Reply-To: <20250827175247.83322-2-shivankg@amd.com>
Hello:
This series was applied to jaegeuk/f2fs.git (dev)
by Sean Christopherson <seanjc@google.com>:
On Wed, 27 Aug 2025 17:52:41 +0000 you wrote:
> This series introduces NUMA-aware memory placement support for KVM guests
> with guest_memfd memory backends. It builds upon Fuad Tabba's work (V17)
> that enabled host-mapping for guest_memfd memory [1] and can be applied
> directly applied on KVM tree [2] (branch kvm-next, base commit: a6ad5413,
> Merge branch 'guest-memfd-mmap' into HEAD)
>
> == Background ==
> KVM's guest-memfd memory backend currently lacks support for NUMA policy
> enforcement, causing guest memory allocations to be distributed across host
> nodes according to kernel's default behavior, irrespective of any policy
> specified by the VMM. This limitation arises because conventional userspace
> NUMA control mechanisms like mbind(2) don't work since the memory isn't
> directly mapped to userspace when allocations occur.
> Fuad's work [1] provides the necessary mmap capability, and this series
> leverages it to enable mbind(2).
>
> [...]
Here is the summary with links:
- [f2fs-dev,kvm-next,V11,1/7] mm/filemap: Add NUMA mempolicy support to filemap_alloc_folio()
(no matching commit)
- [f2fs-dev,kvm-next,V11,2/7] mm/filemap: Extend __filemap_get_folio() to support NUMA memory policies
https://git.kernel.org/jaegeuk/f2fs/c/16a542e22339
- [f2fs-dev,kvm-next,V11,3/7] mm/mempolicy: Export memory policy symbols
https://git.kernel.org/jaegeuk/f2fs/c/f634f10809ec
- [f2fs-dev,kvm-next,V11,4/7] KVM: guest_memfd: Use guest mem inodes instead of anonymous inodes
(no matching commit)
- [f2fs-dev,kvm-next,V11,5/7] KVM: guest_memfd: Add slab-allocated inode cache
(no matching commit)
- [f2fs-dev,kvm-next,V11,6/7] KVM: guest_memfd: Enforce NUMA mempolicy using shared policy
(no matching commit)
- [f2fs-dev,kvm-next,V11,7/7] KVM: guest_memfd: selftests: Add tests for mmap and NUMA policy support
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [RFC PATCH 4/5] firmware: smccc: register as platform driver
From: Sudeep Holla @ 2025-12-09 11:47 UTC (permalink / raw)
To: Vedashree Vidwans
Cc: salman.nabi, andre.przywara, lpieralisi, mark.rutland, ardb,
chao.gao, linux-arm-kernel, linux-coco, linux-kernel, sdonthineni,
vsethi, Sudeep Holla, vwadekar
In-Reply-To: <20251208221319.1524888-5-vvidwans@nvidia.com>
On Mon, Dec 08, 2025 at 10:13:14PM +0000, Vedashree Vidwans wrote:
> - Update driver to be in-built kernel module. This will ensure driver is
> installed in kernel and would not require any user intervention.
> - Register the LFA driver as a platform driver corresponding to
> 'armhf000' device. The driver will be invoked when the device is
> detected on a platform.
> - Add functionality to register LFA interrupt in the driver probe().
> This LFA IRQ number will be retrived from the LFA device node.
> - On IRQ, driver will query FW component details and trigger activation
> of capable and pending FW component. The driver will loop to update FW
> component details after every successful FW component activation.
> - Mutex synchronization is implemented to avoid concurrent LFA updates
> through interrupt and sysfs interfaces.
>
> Device node snippet from LFA spec[1]:
> fwu0 {
> compatible = "arm,armhf000";
> memory-region = <&fwu_payload>;
> interrupt-parent = <&ic>;
> interrupts = <0 100 1>; // SPI, Interrupt #100, Edge Rising
> };
>
This will be gone in the latest beta of LFA, so please discuss and get
an agreement for the LFA device tree bindings.
We don't just use ACPI HID as devicetree compatibles. There are more
aligned with ACPI CID IIUC but I don't expect you to use ACPI CID just to
match DT compatible as ACPI HID will be defined for LFA.
> [1] https://developer.arm.com/documentation/den0147/latest/
>
> Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
> ---
> drivers/firmware/smccc/Kconfig | 3 +-
> drivers/firmware/smccc/lfa_fw.c | 124 +++++++++++++++++++++++++++++++-
> 2 files changed, 125 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
> index 48b98c14f770..c21be43fbfed 100644
> --- a/drivers/firmware/smccc/Kconfig
> +++ b/drivers/firmware/smccc/Kconfig
> @@ -25,8 +25,9 @@ config ARM_SMCCC_SOC_ID
> platforms providing some sysfs information about the SoC variant.
>
> config ARM_LFA
> - tristate "Arm Live Firmware activation support"
> + bool "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/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index 0e420cefa260..24916fc53420 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -19,7 +19,12 @@
> #include <linux/nmi.h>
> #include <linux/ktime.h>
> #include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/acpi.h>
> +#include <linux/interrupt.h>
> +#include <linux/mutex.h>
>
> +#define DRIVER_NAME "ARM_LFA"
> #define LFA_ERROR_STRING(name) \
> [name] = #name
> #undef pr_fmt
> @@ -129,6 +134,7 @@ static const struct fw_image_uuid {
> };
>
> static struct kobject *lfa_dir;
> +static DEFINE_MUTEX(lfa_lock);
>
> static int get_nr_lfa_components(void)
> {
> @@ -374,17 +380,23 @@ static ssize_t activate_store(struct kobject *kobj, struct kobj_attribute *attr,
> image_attrs[LFA_ATTR_ACTIVATE]);
> int ret;
>
> + if (!mutex_trylock(&lfa_lock)) {
> + pr_err("Mutex locked, try again");
> + return -EAGAIN;
> + }
> +
> ret = activate_fw_image(attrs);
> if (ret) {
> pr_err("Firmware activation failed: %s\n",
> lfa_error_strings[-ret]);
> -
> + mutex_unlock(&lfa_lock);
> return -ECANCELED;
> }
>
> pr_info("Firmware activation succeeded\n");
>
> /* TODO: refresh image flags here*/
> + mutex_unlock(&lfa_lock);
> return count;
> }
>
> @@ -510,6 +522,106 @@ static int create_fw_images_tree(void)
> return 0;
> }
>
> +static irqreturn_t lfa_irq_thread(int irq, void *data)
> +{
> + struct image_props *attrs = NULL;
> + int ret;
> + int num_of_components, curr_component;
> +
> + mutex_lock(&lfa_lock);
> +
> + /*
> + * As per LFA spec, after activation of a component, the caller
> + * is expected to re-enumerate the component states (using
> + * LFA_GET_INFO then LFA_GET_INVENTORY).
> + * Hence we need an unconditional loop.
> + */
> +
> + do {
> + /* TODO: refresh image flags here */
> + /* If refresh fails goto exit_unlock */
> +
> + /* Initialize counters to track list traversal */
> + num_of_components = get_nr_lfa_components();
> + curr_component = 0;
> +
> + /* Execute PRIME and ACTIVATE for activable FW component */
> + list_for_each_entry(attrs, &lfa_fw_images, image_node) {
> + curr_component++;
> + if ((!attrs->activation_capable) || (!attrs->activation_pending)) {
> + /* LFA not applicable for this FW component */
> + continue;
> + }
> +
> + ret = activate_fw_image(attrs);
> + if (ret) {
> + pr_err("Firmware %s activation failed: %s\n",
> + attrs->image_name, lfa_error_strings[-ret]);
> + goto exit_unlock;
> + }
> +
> + pr_info("Firmware %s activation succeeded", attrs->image_name);
> + /* Refresh FW component details */
> + break;
> + }
> + } while (curr_component < num_of_components);
> +
> + /* TODO: refresh image flags here */
> + /* If refresh fails goto exit_unlock */
> +
> +exit_unlock:
> + mutex_unlock(&lfa_lock);
> + return IRQ_HANDLED;
> +}
> +
> +static int __init lfa_probe(struct platform_device *pdev)
> +{
> + int err;
> + unsigned int irq;
> +
> + err = platform_get_irq_byname_optional(pdev, "fw-store-updated-interrupt");
Nice, "fw-store-updated-interrupt" is not even mentioned in the example DT
node above, let alone proper DT bindings.
--
Regards,
Sudeep
^ permalink raw reply
* Re: [RFC PATCH 1/5] firmware: smccc: LFA: use smcc 1.2
From: Sudeep Holla @ 2025-12-09 11:42 UTC (permalink / raw)
To: Vedashree Vidwans
Cc: salman.nabi, andre.przywara, lpieralisi, mark.rutland, ardb,
chao.gao, linux-arm-kernel, linux-coco, linux-kernel, sdonthineni,
vsethi, Sudeep Holla, vwadekar
In-Reply-To: <20251208221319.1524888-2-vvidwans@nvidia.com>
On Mon, Dec 08, 2025 at 10:13:11PM +0000, Vedashree Vidwans wrote:
> Update driver to use SMCCC 1.2+ version as mentioned in the LFA spec.
>
I would prefer if you work with Salman Nabi and get this incorporated
in the original patch by providing this as a review feedback.
There is no point in having this independent of the original patch as it
is not yet merged.
--
Regards,
Sudeep
^ permalink raw reply
* Re: [RFC PATCH 0/5] Arm LFA: Improvements and interrupt support
From: Sudeep Holla @ 2025-12-09 11:39 UTC (permalink / raw)
To: Vedashree Vidwans
Cc: salman.nabi, andre.przywara, lpieralisi, mark.rutland, ardb,
chao.gao, linux-arm-kernel, linux-coco, linux-kernel, sdonthineni,
vsethi, Sudeep Holla, vwadekar
In-Reply-To: <20251208221319.1524888-1-vvidwans@nvidia.com>
On Mon, Dec 08, 2025 at 10:13:10PM +0000, Vedashree Vidwans wrote:
> Hello,
>
> The patches update the proposed Arm Live Firmware Activation (LFA)
> kernel driver [1] to incorporate review feedback [2] and refine the
> activation flow while remaining aligned with the LFA specification
> DEN0147 [3] and the SMCCC 1.2 calling convention. The series keeps
> the existing functionality but restructures and extends it to improve
> robustness, reviewability, and future extensibility.
>
> The SMCCC usage in the driver is updated to consistently use the
> SMCCC 1.2 register-based calling convention, consolidating arguments
> and results into a single struct to reduce stack usage and simplify
> the SMC interface. The patches also split the original changes into
> focused pieces and document the device node bindings in the commit
> messages, making it easier to follow and validate the implementation
> against the specification.
>
> The kernel driver is registered as a platform driver in accordence to
> the LFA device defined by the specification [3]. The driver now extends
> interface for interrupt-based enablement of LFA. During LFA, the
> interrupt
> thread refreshes firmware component details after each activation step
> and iterates over all activable components until no further activation
> is pending, matching the spec’s allowance for component detail changes
> after activation. This ensures that sysfs exposure of LFA components
> remains consistent with the authoritative information provided by the
> secure firmware.
>
> The handling of CPU rendezvous is adjusted so that the kernel now
> honors the rendezvous policy chosen by the firmware, instead of
> unconditionally forcing a rendezvous. This reflects experience with
> existing firmware deployments where mandatory rendezvous is not
> required, while still allowing the firmware to request it when
> needed.
>
> Thank you,
> Veda
>
> [1] https://lore.kernel.org/lkml/20250926123145.268728-1-salman.nabi@arm.com/
> [2] https://lkml.org/lkml/2025/10/8/980
> [3] https://developer.arm.com/documentation/den0147/latest/
>
> Vedashree Vidwans (5):
> firmware: smccc: LFA: use smcc 1.2
> firmware: smccc: LFA: refactor
> firmware: smccc: add timeout, touch wdt
> firmware: smccc: register as platform driver
> firmware: smccc: lfa: refresh fw details
>
> drivers/firmware/smccc/Kconfig | 3 +-
> drivers/firmware/smccc/lfa_fw.c | 478 +++++++++++++++++++++++++-------
Same comment as before[1], looks like the feedback got ignored or missed.
--
Regards,
Sudeep
[1] https://lore.kernel.org/all/20251009-calculating-glorious-oriole-ceccf6@sudeepholla
^ permalink raw reply
* Re: [PATCH v2 1/7] KVM: x86: Move kvm_rebooting to x86
From: Chao Gao @ 2025-12-09 7:46 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Dan Williams
In-Reply-To: <20251206011054.494190-2-seanjc@google.com>
On Fri, Dec 05, 2025 at 05:10:48PM -0800, Sean Christopherson wrote:
>Move kvm_rebooting, which is only read by x86, to KVM x86 so that it can
>be moved again to core x86 code. Add a "shutdown" arch hook to facilate
>setting the flag in KVM x86.
>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
^ permalink raw reply
* Re: [PATCH v2 7/7] KVM: Bury kvm_{en,dis}able_virtualization() in kvm_main.c once more
From: Chao Gao @ 2025-12-09 7:37 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Dan Williams
In-Reply-To: <20251206011054.494190-8-seanjc@google.com>
On Fri, Dec 05, 2025 at 05:10:54PM -0800, Sean Christopherson wrote:
>Now that TDX handles doing VMXON without KVM's involvement, bury the
>top-level APIs to enable and disable virtualization back in kvm_main.c.
>
>No functional change intended.
>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
^ permalink raw reply
* Re: [PATCH v2 6/7] x86/virt/tdx: Use ida_is_empty() to detect if any TDs may be running
From: Chao Gao @ 2025-12-09 7:33 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Dan Williams
In-Reply-To: <20251206011054.494190-7-seanjc@google.com>
On Fri, Dec 05, 2025 at 05:10:53PM -0800, Sean Christopherson wrote:
>Drop nr_configured_hkid and instead use ida_is_empty() to detect if any
>HKIDs have been allocated/configured.
>
>Suggested-by: Dan Williams <dan.j.williams@intel.com>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
^ permalink raw reply
* Re: [PATCH v2 4/7] x86/virt/tdx: Tag a pile of functions as __init, and globals as __ro_after_init
From: Chao Gao @ 2025-12-09 7:26 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Dan Williams
In-Reply-To: <20251206011054.494190-5-seanjc@google.com>
On Fri, Dec 05, 2025 at 05:10:51PM -0800, Sean Christopherson wrote:
>Now that TDX-Module initialization is done during subsys init, tag all
>related functions as __init, and relevant data as __ro_after_init.
>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
Two nits:
1. do_global_key_config() and config_global_keyid() can be tagged as __init.
2. Double space after the __init tag of free_pamt()
>-static void free_pamt(unsigned long pamt_base, unsigned long pamt_size)
>+static __init void free_pamt(unsigned long pamt_base, unsigned long pamt_size)
^^
> {
> free_contig_range(pamt_base >> PAGE_SHIFT, pamt_size >> PAGE_SHIFT);
> }
>
^ permalink raw reply
* Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init
From: Chao Gao @ 2025-12-09 7:06 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Dan Williams
In-Reply-To: <20251206011054.494190-4-seanjc@google.com>
On Fri, Dec 05, 2025 at 05:10:50PM -0800, Sean Christopherson wrote:
>Now that VMXON can be done without bouncing through KVM, do TDX-Module
>initialization during subsys init (specifically before module_init() so
>that it runs before KVM when both are built-in). Aside from the obvious
>benefits of separating core TDX code from KVM, this will allow tagging a
>pile of TDX functions and globals as being __init and __ro_after_init.
>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
>---
> Documentation/arch/x86/tdx.rst | 26 -----
> arch/x86/include/asm/tdx.h | 4 -
> arch/x86/kvm/vmx/tdx.c | 169 ++++++--------------------------
> arch/x86/virt/vmx/tdx/tdx.c | 170 ++++++++++++++++++---------------
> arch/x86/virt/vmx/tdx/tdx.h | 8 --
> 5 files changed, 124 insertions(+), 253 deletions(-)
>
>diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
>index 61670e7df2f7..2e0a15d6f7d1 100644
>--- a/Documentation/arch/x86/tdx.rst
>+++ b/Documentation/arch/x86/tdx.rst
>@@ -60,32 +60,6 @@ Besides initializing the TDX module, a per-cpu initialization SEAMCALL
> must be done on one cpu before any other SEAMCALLs can be made on that
> cpu.
>
>-The kernel provides two functions, tdx_enable() and tdx_cpu_enable() to
>-allow the user of TDX to enable the TDX module and enable TDX on local
>-cpu respectively.
>-
>-Making SEAMCALL requires VMXON has been done on that CPU. Currently only
>-KVM implements VMXON. For now both tdx_enable() and tdx_cpu_enable()
>-don't do VMXON internally (not trivial), but depends on the caller to
>-guarantee that.
>-
>-To enable TDX, the caller of TDX should: 1) temporarily disable CPU
>-hotplug; 2) do VMXON and tdx_enable_cpu() on all online cpus; 3) call
>-tdx_enable(). For example::
>-
>- cpus_read_lock();
>- on_each_cpu(vmxon_and_tdx_cpu_enable());
>- ret = tdx_enable();
>- cpus_read_unlock();
>- if (ret)
>- goto no_tdx;
>- // TDX is ready to use
>-
>-And the caller of TDX must guarantee the tdx_cpu_enable() has been
>-successfully done on any cpu before it wants to run any other SEAMCALL.
>-A typical usage is do both VMXON and tdx_cpu_enable() in CPU hotplug
>-online callback, and refuse to online if tdx_cpu_enable() fails.
>-
With this patch applied, other parts of the document will be stale and need
updates, i.e.,:
diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
index 2e0a15d6f7d1..3d5bc68e3bcb 100644
--- a/Documentation/arch/x86/tdx.rst
+++ b/Documentation/arch/x86/tdx.rst
@@ -66,12 +66,12 @@ If the TDX module is initialized successfully, dmesg shows something
like below::
[..] virt/tdx: 262668 KBs allocated for PAMT
- [..] virt/tdx: module initialized
+ [..] virt/tdx: TDX-Module initialized
If the TDX module failed to initialize, dmesg also shows it failed to
initialize::
- [..] virt/tdx: module initialization failed ...
+ [..] virt/tdx: TDX-Module initialization failed ...
TDX Interaction to Other Kernel Components
------------------------------------------
@@ -102,11 +102,6 @@ removal but depends on the BIOS to behave correctly.
CPU Hotplug
~~~~~~~~~~~
-TDX module requires the per-cpu initialization SEAMCALL must be done on
-one cpu before any other SEAMCALLs can be made on that cpu. The kernel
-provides tdx_cpu_enable() to let the user of TDX to do it when the user
-wants to use a new cpu for TDX task.
-
TDX doesn't support physical (ACPI) CPU hotplug. During machine boot,
TDX verifies all boot-time present logical CPUs are TDX compatible before
enabling TDX. A non-buggy BIOS should never support hot-add/removal of
> static int __init __tdx_bringup(void)
> {
> const struct tdx_sys_info_td_conf *td_conf;
>@@ -3417,34 +3362,18 @@ static int __init __tdx_bringup(void)
> }
> }
>
>- /*
>- * Enabling TDX requires enabling hardware virtualization first,
>- * as making SEAMCALLs requires CPU being in post-VMXON state.
>- */
>- r = kvm_enable_virtualization();
>- if (r)
>- return r;
>-
>- cpus_read_lock();
>- r = __do_tdx_bringup();
>- cpus_read_unlock();
>-
>- if (r)
>- goto tdx_bringup_err;
>-
>- r = -EINVAL;
> /* Get TDX global information for later use */
> tdx_sysinfo = tdx_get_sysinfo();
>- if (WARN_ON_ONCE(!tdx_sysinfo))
>- goto get_sysinfo_err;
>+ if (!tdx_sysinfo)
>+ return -EINVAL;
...
>- /*
>- * Ideally KVM should probe whether TDX module has been loaded
>- * first and then try to bring it up. But TDX needs to use SEAMCALL
>- * to probe whether the module is loaded (there is no CPUID or MSR
>- * for that), and making SEAMCALL requires enabling virtualization
>- * first, just like the rest steps of bringing up TDX module.
>- *
>- * So, for simplicity do everything in __tdx_bringup(); the first
>- * SEAMCALL will return -ENODEV when the module is not loaded. The
>- * only complication is having to make sure that initialization
>- * SEAMCALLs don't return TDX_SEAMCALL_VMFAILINVALID in other
>- * cases.
>- */
> r = __tdx_bringup();
>- if (r) {
>- /*
>- * Disable TDX only but don't fail to load module if the TDX
>- * module could not be loaded. No need to print message saying
>- * "module is not loaded" because it was printed when the first
>- * SEAMCALL failed. Don't bother unwinding the S-EPT hooks or
>- * vm_size, as kvm_x86_ops have already been finalized (and are
>- * intentionally not exported). The S-EPT code is unreachable,
>- * and allocating a few more bytes per VM in a should-be-rare
>- * failure scenario is a non-issue.
>- */
>- if (r == -ENODEV)
>- goto success_disable_tdx;
Previously, loading kvm-intel.ko (with tdx=1) would succeed even if there was
no TDX module loaded by BIOS. IIUC, the behavior changes here; the lack of TDX
module becomes fatal and kvm-intel.ko loading would fail.
Is this intentional?
>-
>+ if (r)
> enable_tdx = 0;
>- }
>
> return r;
>
>@@ -3596,6 +3480,15 @@ int __init tdx_bringup(void)
> return 0;
> }
^ permalink raw reply related
* Re: [PATCH v2 2/7] KVM: x86: Extract VMXON and EFER.SVME enablement to kernel
From: Chao Gao @ 2025-12-09 5:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Dan Williams
In-Reply-To: <20251206011054.494190-3-seanjc@google.com>
>--- /dev/null
>+++ b/arch/x86/include/asm/virt.h
>@@ -0,0 +1,26 @@
>+/* SPDX-License-Identifier: GPL-2.0-only */
>+#ifndef _ASM_X86_VIRT_H
>+#define _ASM_X86_VIRT_H
>+
>+#include <asm/reboot.h>
asm/reboot.h isn't used.
>+
>+typedef void (cpu_emergency_virt_cb)(void);
>+
>+#if IS_ENABLED(CONFIG_KVM_X86)
>+extern bool virt_rebooting;
>+
>+void __init x86_virt_init(void);
>+
>+int x86_virt_get_cpu(int feat);
>+void x86_virt_put_cpu(int feat);
>+
>+int x86_virt_emergency_disable_virtualization_cpu(void);
>+
>+void x86_virt_register_emergency_callback(cpu_emergency_virt_cb *callback);
>+void x86_virt_unregister_emergency_callback(cpu_emergency_virt_cb *callback);
>+#else
>+static __always_inline void x86_virt_init(void) {}
Why does this need to be "__always_inline" rather than just "inline"?
> static void emergency_reboot_disable_virtualization(void)
> {
> local_irq_disable();
>@@ -587,16 +543,11 @@ static void emergency_reboot_disable_virtualization(void)
> * We can't take any locks and we may be on an inconsistent state, so
> * use NMIs as IPIs to tell the other CPUs to disable VMX/SVM and halt.
> *
>- * Do the NMI shootdown even if virtualization is off on _this_ CPU, as
>- * other CPUs may have virtualization enabled.
>+ * Safely force _this_ CPU out of VMX/SVM operation, and if necessary,
>+ * blast NMIs to force other CPUs out of VMX/SVM as well.k
^ stray "k".
I don't understand the "if necessary" part. My understanding is this code
issues NMIs if CPUs support VMX or SVM. If so, I think the code snippet below
would be more readable:
if (cpu_feature_enabled(X86_FEATURE_VMX) ||
cpu_feature_enabled(X86_FEATURE_SVM)) {
x86_virt_emergency_disable_virtualization_cpu();
nmi_shootdown_cpus_on_restart();
}
Then x86_virt_emergency_disable_virtualization_cpu() wouldn't need to return
anything. And readers wouldn't need to trace down the function to understand
when NMIs are "necessary" and when they are not.
> */
>- if (rcu_access_pointer(cpu_emergency_virt_callback)) {
>- /* Safely force _this_ CPU out of VMX/SVM operation. */
>- cpu_emergency_disable_virtualization();
>-
>- /* Disable VMX/SVM and halt on other CPUs. */
>+ if (!x86_virt_emergency_disable_virtualization_cpu())
> nmi_shootdown_cpus_on_restart();
>- }
> }
<snip>
>+#define x86_virt_call(fn) \
>+({ \
>+ int __r; \
>+ \
>+ if (IS_ENABLED(CONFIG_KVM_INTEL) && \
>+ cpu_feature_enabled(X86_FEATURE_VMX)) \
>+ __r = x86_vmx_##fn(); \
>+ else if (IS_ENABLED(CONFIG_KVM_AMD) && \
>+ cpu_feature_enabled(X86_FEATURE_SVM)) \
>+ __r = x86_svm_##fn(); \
>+ else \
>+ __r = -EOPNOTSUPP; \
>+ \
>+ __r; \
>+})
>+
>+int x86_virt_get_cpu(int feat)
>+{
>+ int r;
>+
>+ if (!x86_virt_feature || x86_virt_feature != feat)
>+ return -EOPNOTSUPP;
>+
>+ if (this_cpu_inc_return(virtualization_nr_users) > 1)
>+ return 0;
Should we assert that preemption is disabled? Calling this API when preemption
is enabled is wrong.
Maybe use __this_cpu_inc_return(), which already verifies preemption status.
<snip>
>+int x86_virt_emergency_disable_virtualization_cpu(void)
>+{
>+ if (!x86_virt_feature)
>+ return -EOPNOTSUPP;
>+
>+ /*
>+ * IRQs must be disabled as virtualization is enabled in hardware via
>+ * function call IPIs, i.e. IRQs need to be disabled to guarantee
>+ * virtualization stays disabled.
>+ */
>+ lockdep_assert_irqs_disabled();
>+
>+ /*
>+ * Do the NMI shootdown even if virtualization is off on _this_ CPU, as
>+ * other CPUs may have virtualization enabled.
>+ *
>+ * TODO: Track whether or not virtualization might be enabled on other
>+ * CPUs? May not be worth avoiding the NMI shootdown...
>+ */
This comment is misplaced. NMIs are issued by the caller.
>+ (void)x86_virt_call(emergency_disable_virtualization_cpu);
>+ return 0;
>+}
^ permalink raw reply
* Re: [PATCH v2 7/7] KVM: Bury kvm_{en,dis}able_virtualization() in kvm_main.c once more
From: dan.j.williams @ 2025-12-09 4:20 UTC (permalink / raw)
To: Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Sean Christopherson, Paolo Bonzini
Cc: linux-kernel, linux-coco, kvm, Chao Gao, Dan Williams
In-Reply-To: <20251206011054.494190-8-seanjc@google.com>
Sean Christopherson wrote:
> Now that TDX handles doing VMXON without KVM's involvement, bury the
> top-level APIs to enable and disable virtualization back in kvm_main.c.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Looks good,
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply
* Re: [PATCH v2 6/7] x86/virt/tdx: Use ida_is_empty() to detect if any TDs may be running
From: dan.j.williams @ 2025-12-09 4:19 UTC (permalink / raw)
To: Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Sean Christopherson, Paolo Bonzini
Cc: linux-kernel, linux-coco, kvm, Chao Gao, Dan Williams
In-Reply-To: <20251206011054.494190-7-seanjc@google.com>
Sean Christopherson wrote:
> Drop nr_configured_hkid and instead use ida_is_empty() to detect if any
> HKIDs have been allocated/configured.
>
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply
* Re: [PATCH v2 5/7] x86/virt/tdx: KVM: Consolidate TDX CPU hotplug handling
From: dan.j.williams @ 2025-12-09 4:19 UTC (permalink / raw)
To: Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Sean Christopherson, Paolo Bonzini
Cc: linux-kernel, linux-coco, kvm, Chao Gao, Dan Williams
In-Reply-To: <20251206011054.494190-6-seanjc@google.com>
Sean Christopherson wrote:
> From: Chao Gao <chao.gao@intel.com>
>
> The core kernel registers a CPU hotplug callback to do VMX and TDX init
> and deinit while KVM registers a separate CPU offline callback to block
> offlining the last online CPU in a socket.
>
> Splitting TDX-related CPU hotplug handling across two components is odd
> and adds unnecessary complexity.
>
> Consolidate TDX-related CPU hotplug handling by integrating KVM's
> tdx_offline_cpu() to the one in the core kernel.
>
> Also move nr_configured_hkid to the core kernel because tdx_offline_cpu()
> references it. Since HKID allocation and free are handled in the core
> kernel, it's more natural to track used HKIDs there.
Looks good,
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply
* Re: [PATCH v2 4/7] x86/virt/tdx: Tag a pile of functions as __init, and globals as __ro_after_init
From: dan.j.williams @ 2025-12-09 4:17 UTC (permalink / raw)
To: Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Sean Christopherson, Paolo Bonzini
Cc: linux-kernel, linux-coco, kvm, Chao Gao, Dan Williams
In-Reply-To: <20251206011054.494190-5-seanjc@google.com>
Sean Christopherson wrote:
> Now that TDX-Module initialization is done during subsys init, tag all
> related functions as __init, and relevant data as __ro_after_init.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Looks good,
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply
* Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init
From: dan.j.williams @ 2025-12-09 1:34 UTC (permalink / raw)
To: Sean Christopherson, dan.j.williams
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Chao Gao
In-Reply-To: <aTdchSmOBo60vbZT@google.com>
Sean Christopherson wrote:
[..]
> > With or without the additional tdx_{on,off}line_cpu fixups:
>
> I think the fixups you're looking for are in patch 5, "/virt/tdx: KVM: Consolidate
> TDX CPU hotplug handling", or did I misunderstand?
Oh, yes, all addressed there. Should have finished going through it
before replying.
^ permalink raw reply
* Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init
From: Sean Christopherson @ 2025-12-08 23:17 UTC (permalink / raw)
To: dan.j.williams
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Chao Gao
In-Reply-To: <69352bd044fdb_1b2e10033@dwillia2-mobl4.notmuch>
On Sat, Dec 06, 2025, dan.j.williams@intel.com wrote:
> Sean Christopherson wrote:
> Given this routine now has nothing to do...
>
> > + * TDX-specific cpuhp callback to disallow offlining the last CPU in a
> > + * packing while KVM is running one or more TDs. Reclaiming HKIDs
> > + * requires doing PAGE.WBINVD on every package, i.e. offlining all CPUs
> > + * of a package would prevent reclaiming the HKID.
> > */
> > + r = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "kvm/cpu/tdx:online",
> > + tdx_online_cpu, tdx_offline_cpu);
>
> ...the @startup param can be NULL. That also saves some grep pain no
> more multiple implementations of a "tdx_online_cpu".
>
> Along those lines, should tdx_offline_cpu() become
> kvm_tdx_offline_cpu()?
>
> [..]
> > /*
> > * Add a memory region as a TDX memory block. The caller must make sure
> > @@ -1156,67 +1194,50 @@ static int init_tdx_module(void)
> > goto out_put_tdxmem;
> > }
> >
> > -static int __tdx_enable(void)
> > +static int tdx_enable(void)
>
> Almost commented about this being able to be __init now, but then I see
> you have a combo patch for that later.
>
> With or without the additional tdx_{on,off}line_cpu fixups:
I think the fixups you're looking for are in patch 5, "/virt/tdx: KVM: Consolidate
TDX CPU hotplug handling", or did I misunderstand?
^ permalink raw reply
* [RFC PATCH 5/5] firmware: smccc: lfa: refresh fw details
From: Vedashree Vidwans @ 2025-12-08 22:13 UTC (permalink / raw)
To: salman.nabi, sudeep.holla, andre.przywara, lpieralisi,
mark.rutland
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar, Vedashree Vidwans
In-Reply-To: <20251208221319.1524888-1-vvidwans@nvidia.com>
FW image details are querried with a SMC call. Currently, these FW
details are added as nodes in a linked list. This patch updates the
FW node creation and update functions. Now the linked list is updated
based on the current value of num_lfa_components.
As per spec [1], FW inventory is updated after each activation.
[1] https://developer.arm.com/documentation/den0147/latest/
Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
---
drivers/firmware/smccc/lfa_fw.c | 148 +++++++++++++++++++++-----------
1 file changed, 100 insertions(+), 48 deletions(-)
diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
index 24916fc53420..334090708405 100644
--- a/drivers/firmware/smccc/lfa_fw.c
+++ b/drivers/firmware/smccc/lfa_fw.c
@@ -133,6 +133,7 @@ static const struct fw_image_uuid {
},
};
+static int update_fw_images_tree(void);
static struct kobject *lfa_dir;
static DEFINE_MUTEX(lfa_lock);
@@ -282,17 +283,6 @@ static int activate_fw_image(struct image_props *attrs)
return ret;
}
-static void set_image_flags(struct image_props *attrs, int seq_id,
- u32 image_flags)
-{
- attrs->fw_seq_id = seq_id;
- 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 ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
@@ -395,7 +385,9 @@ static ssize_t activate_store(struct kobject *kobj, struct kobj_attribute *attr,
pr_info("Firmware activation succeeded\n");
- /* TODO: refresh image flags here*/
+ ret = update_fw_images_tree();
+ if (ret)
+ pr_err("Failed to update FW images tree");
mutex_unlock(&lfa_lock);
return count;
}
@@ -425,20 +417,41 @@ static struct kobj_attribute image_attrs_group[LFA_ATTR_NR_IMAGES] = {
[LFA_ATTR_CANCEL] = __ATTR_WO(cancel)
};
+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);
+ attrs->image_dir = NULL;
+ list_del(&attrs->image_node);
+ kfree(attrs);
+}
+
static void clean_fw_images_tree(void)
{
struct image_props *attrs, *tmp;
- list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node) {
- kobject_put(attrs->image_dir);
- list_del(&attrs->image_node);
- kfree(attrs);
- }
+ list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node)
+ delete_fw_image_node(attrs);
+}
+
+static void update_fw_image_node(struct image_props *attrs, int seq_id,
+ char *fw_uuid, u32 image_flags)
+{
+ attrs->fw_seq_id = seq_id;
+ attrs->image_name = fw_uuid;
+ 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 int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
+static int create_fw_image_node(int seq_id, char *fw_uuid, u32 image_flags)
{
- const char *image_name = "(unknown)";
struct image_props *attrs;
int ret;
@@ -446,21 +459,12 @@ static int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
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;
- else
- image_name = fw_uuid;
- }
-
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);
+ /* Update FW attributes */
+ update_fw_image_node(attrs, seq_id, fw_uuid, image_flags);
/*
* The attributes for each sysfs file are constant (handler functions,
@@ -485,17 +489,19 @@ static int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
return ret;
}
}
- list_add(&attrs->image_node, &lfa_fw_images);
+ list_add_tail(&attrs->image_node, &lfa_fw_images);
return ret;
}
-static int create_fw_images_tree(void)
+static int update_fw_images_tree(void)
{
struct arm_smccc_1_2_regs reg = { 0 };
struct uuid_regs image_uuid;
+ struct image_props *attrs, *tmp;
char image_id_str[40];
int ret, num_of_components;
+ int node_idx = 0;
num_of_components = get_nr_lfa_components();
if (num_of_components <= 0) {
@@ -503,22 +509,67 @@ static int create_fw_images_tree(void)
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) {
+ /*
+ * Pass 1:
+ * For nodes < num_of_components, update fw_image_node
+ * For nodes >= num_of_components, delete
+ */
+ list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node) {
+ if (attrs->fw_seq_id < num_of_components) {
+ /* Update this FW image node */
+
+ /* Get FW details */
+ reg.a0 = LFA_1_0_FN_GET_INVENTORY;
+ reg.a1 = attrs->fw_seq_id;
+ arm_smccc_1_2_invoke(®, ®);
+
+ if (reg.a0 != LFA_SUCCESS)
+ return -EINVAL;
+
+ /* Build image name with UUID */
image_uuid.uuid_lo = reg.a1;
image_uuid.uuid_hi = reg.a2;
+ snprintf(image_id_str, sizeof(image_id_str), "%pUb", &image_uuid);
- snprintf(image_id_str, sizeof(image_id_str), "%pUb",
- &image_uuid);
- ret = create_fw_inventory(image_id_str, i, reg.a3);
- if (ret)
- return ret;
+ if (strcmp(attrs->image_name, image_id_str)) {
+ /* UUID doesn't match previous UUID for given FW, not expected */
+ pr_err("FW seq id %u: Previous UUID %s doesn't match current %s",
+ attrs->fw_seq_id, attrs->image_name, image_id_str);
+ return -EINVAL;
+ }
+
+ /* Update FW attributes */
+ update_fw_image_node(attrs, attrs->fw_seq_id, image_id_str, reg.a3);
+ node_idx++;
+ } else {
+ /* This node is beyond valid FW components list */
+ delete_fw_image_node(attrs);
}
}
+ /*
+ * Pass 2:
+ * If current FW components number is more than previous list, add new component nodes.
+ */
+ for (node_idx; node_idx < num_of_components; node_idx++) {
+ /* Get FW details */
+ reg.a0 = LFA_1_0_FN_GET_INVENTORY;
+ reg.a1 = node_idx;
+ arm_smccc_1_2_invoke(®, ®);
+
+ if (reg.a0 != LFA_SUCCESS)
+ return -EINVAL;
+
+ /* Build image name with UUID */
+ image_uuid.uuid_lo = reg.a1;
+ image_uuid.uuid_hi = reg.a2;
+ snprintf(image_id_str, sizeof(image_id_str), "%pUb", &image_uuid);
+
+ ret = create_fw_image_node(node_idx, image_id_str, reg.a3);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
@@ -538,8 +589,9 @@ static irqreturn_t lfa_irq_thread(int irq, void *data)
*/
do {
- /* TODO: refresh image flags here */
- /* If refresh fails goto exit_unlock */
+ ret = update_fw_images_tree();
+ if (ret)
+ goto exit_unlock;
/* Initialize counters to track list traversal */
num_of_components = get_nr_lfa_components();
@@ -561,13 +613,13 @@ static irqreturn_t lfa_irq_thread(int irq, void *data)
}
pr_info("Firmware %s activation succeeded", attrs->image_name);
- /* Refresh FW component details */
break;
}
} while (curr_component < num_of_components);
- /* TODO: refresh image flags here */
- /* If refresh fails goto exit_unlock */
+ ret = update_fw_images_tree();
+ if (ret)
+ goto exit_unlock;
exit_unlock:
mutex_unlock(&lfa_lock);
@@ -657,7 +709,7 @@ static int __init lfa_init(void)
return -ENOMEM;
mutex_lock(&lfa_lock);
- err = create_fw_images_tree();
+ err = update_fw_images_tree();
if (err != 0)
kobject_put(lfa_dir);
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 4/5] firmware: smccc: register as platform driver
From: Vedashree Vidwans @ 2025-12-08 22:13 UTC (permalink / raw)
To: salman.nabi, sudeep.holla, andre.przywara, lpieralisi,
mark.rutland
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar, Vedashree Vidwans
In-Reply-To: <20251208221319.1524888-1-vvidwans@nvidia.com>
- Update driver to be in-built kernel module. This will ensure driver is
installed in kernel and would not require any user intervention.
- Register the LFA driver as a platform driver corresponding to
'armhf000' device. The driver will be invoked when the device is
detected on a platform.
- Add functionality to register LFA interrupt in the driver probe().
This LFA IRQ number will be retrived from the LFA device node.
- On IRQ, driver will query FW component details and trigger activation
of capable and pending FW component. The driver will loop to update FW
component details after every successful FW component activation.
- Mutex synchronization is implemented to avoid concurrent LFA updates
through interrupt and sysfs interfaces.
Device node snippet from LFA spec[1]:
fwu0 {
compatible = "arm,armhf000";
memory-region = <&fwu_payload>;
interrupt-parent = <&ic>;
interrupts = <0 100 1>; // SPI, Interrupt #100, Edge Rising
};
[1] https://developer.arm.com/documentation/den0147/latest/
Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
---
drivers/firmware/smccc/Kconfig | 3 +-
drivers/firmware/smccc/lfa_fw.c | 124 +++++++++++++++++++++++++++++++-
2 files changed, 125 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
index 48b98c14f770..c21be43fbfed 100644
--- a/drivers/firmware/smccc/Kconfig
+++ b/drivers/firmware/smccc/Kconfig
@@ -25,8 +25,9 @@ config ARM_SMCCC_SOC_ID
platforms providing some sysfs information about the SoC variant.
config ARM_LFA
- tristate "Arm Live Firmware activation support"
+ bool "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/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
index 0e420cefa260..24916fc53420 100644
--- a/drivers/firmware/smccc/lfa_fw.c
+++ b/drivers/firmware/smccc/lfa_fw.c
@@ -19,7 +19,12 @@
#include <linux/nmi.h>
#include <linux/ktime.h>
#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/acpi.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#define DRIVER_NAME "ARM_LFA"
#define LFA_ERROR_STRING(name) \
[name] = #name
#undef pr_fmt
@@ -129,6 +134,7 @@ static const struct fw_image_uuid {
};
static struct kobject *lfa_dir;
+static DEFINE_MUTEX(lfa_lock);
static int get_nr_lfa_components(void)
{
@@ -374,17 +380,23 @@ static ssize_t activate_store(struct kobject *kobj, struct kobj_attribute *attr,
image_attrs[LFA_ATTR_ACTIVATE]);
int ret;
+ if (!mutex_trylock(&lfa_lock)) {
+ pr_err("Mutex locked, try again");
+ return -EAGAIN;
+ }
+
ret = activate_fw_image(attrs);
if (ret) {
pr_err("Firmware activation failed: %s\n",
lfa_error_strings[-ret]);
-
+ mutex_unlock(&lfa_lock);
return -ECANCELED;
}
pr_info("Firmware activation succeeded\n");
/* TODO: refresh image flags here*/
+ mutex_unlock(&lfa_lock);
return count;
}
@@ -510,6 +522,106 @@ static int create_fw_images_tree(void)
return 0;
}
+static irqreturn_t lfa_irq_thread(int irq, void *data)
+{
+ struct image_props *attrs = NULL;
+ int ret;
+ int num_of_components, curr_component;
+
+ mutex_lock(&lfa_lock);
+
+ /*
+ * As per LFA spec, after activation of a component, the caller
+ * is expected to re-enumerate the component states (using
+ * LFA_GET_INFO then LFA_GET_INVENTORY).
+ * Hence we need an unconditional loop.
+ */
+
+ do {
+ /* TODO: refresh image flags here */
+ /* If refresh fails goto exit_unlock */
+
+ /* Initialize counters to track list traversal */
+ num_of_components = get_nr_lfa_components();
+ curr_component = 0;
+
+ /* Execute PRIME and ACTIVATE for activable FW component */
+ list_for_each_entry(attrs, &lfa_fw_images, image_node) {
+ curr_component++;
+ if ((!attrs->activation_capable) || (!attrs->activation_pending)) {
+ /* LFA not applicable for this FW component */
+ continue;
+ }
+
+ ret = activate_fw_image(attrs);
+ if (ret) {
+ pr_err("Firmware %s activation failed: %s\n",
+ attrs->image_name, lfa_error_strings[-ret]);
+ goto exit_unlock;
+ }
+
+ pr_info("Firmware %s activation succeeded", attrs->image_name);
+ /* Refresh FW component details */
+ break;
+ }
+ } while (curr_component < num_of_components);
+
+ /* TODO: refresh image flags here */
+ /* If refresh fails goto exit_unlock */
+
+exit_unlock:
+ mutex_unlock(&lfa_lock);
+ return IRQ_HANDLED;
+}
+
+static int __init lfa_probe(struct platform_device *pdev)
+{
+ int err;
+ unsigned int irq;
+
+ err = platform_get_irq_byname_optional(pdev, "fw-store-updated-interrupt");
+ if (err < 0)
+ err = platform_get_irq(pdev, 0);
+ if (err < 0) {
+ pr_err("Interrupt not found, functionality will be unavailable.");
+
+ /* Bail out without failing the driver. */
+ return 0;
+ }
+ irq = err;
+
+ err = request_threaded_irq(irq, NULL, lfa_irq_thread, IRQF_ONESHOT, DRIVER_NAME, NULL);
+ if (err != 0) {
+ pr_err("Interrupt setup failed, functionality will be unavailable.");
+
+ /* Bail out without failing the driver. */
+ return 0;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id lfa_of_ids[] = {
+ { .compatible = "arm,armhf000", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, lfa_of_ids);
+
+static const struct acpi_device_id lfa_acpi_ids[] = {
+ {"ARMHF000"},
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, lfa_acpi_ids);
+
+static struct platform_driver lfa_driver = {
+ .probe = lfa_probe,
+ .driver = {
+ .name = DRIVER_NAME,
+ .of_match_table = lfa_of_ids,
+ .acpi_match_table = ACPI_PTR(lfa_acpi_ids),
+ },
+};
+
static int __init lfa_init(void)
{
struct arm_smccc_1_2_regs reg = { 0 };
@@ -536,22 +648,32 @@ static int __init lfa_init(void)
pr_info("Arm Live Firmware Activation (LFA): detected v%ld.%ld\n",
reg.a0 >> 16, reg.a0 & 0xffff);
+ err = platform_driver_register(&lfa_driver);
+ if (err < 0)
+ pr_err("Platform driver register failed");
+
lfa_dir = kobject_create_and_add("lfa", firmware_kobj);
if (!lfa_dir)
return -ENOMEM;
+ mutex_lock(&lfa_lock);
err = create_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)
{
+ mutex_lock(&lfa_lock);
clean_fw_images_tree();
+ mutex_unlock(&lfa_lock);
+
kobject_put(lfa_dir);
+ platform_driver_unregister(&lfa_driver);
}
module_exit(lfa_exit);
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 2/5] firmware: smccc: LFA: refactor
From: Vedashree Vidwans @ 2025-12-08 22:13 UTC (permalink / raw)
To: salman.nabi, sudeep.holla, andre.przywara, lpieralisi,
mark.rutland
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar, Vedashree Vidwans
In-Reply-To: <20251208221319.1524888-1-vvidwans@nvidia.com>
- Refactor LFA CANCEL logic into independent lfa_cancel() function.
- Use FW UUID as image_name for images not known by the driver.
- Move may_reset_cpu check to activate_fw_image(). This keeps all the
functionality within a function.
Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
---
drivers/firmware/smccc/lfa_fw.c | 64 ++++++++++++++++++++-------------
1 file changed, 40 insertions(+), 24 deletions(-)
diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
index bdde14b66606..df8b65324413 100644
--- a/drivers/firmware/smccc/lfa_fw.c
+++ b/drivers/firmware/smccc/lfa_fw.c
@@ -129,6 +129,31 @@ static int get_nr_lfa_components(void)
return reg.a1;
}
+static int lfa_cancel(struct image_props *attrs)
+{
+ 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;
@@ -160,6 +185,11 @@ static int activate_fw_image(struct image_props *attrs)
struct arm_smccc_1_2_regs res = { 0 };
int ret;
+ if (attrs->may_reset_cpu) {
+ pr_err("CPU reset not supported by kernel driver\n");
+ return -EINVAL;
+ }
+
/*
* LFA_PRIME/ACTIVATE will return 1 in res.a1 if the firmware
* priming/activation is still in progress. In that case
@@ -284,12 +314,6 @@ static ssize_t activate_store(struct kobject *kobj, struct kobj_attribute *attr,
image_attrs[LFA_ATTR_ACTIVATE]);
int ret;
- if (attrs->may_reset_cpu) {
- pr_err("Firmware component requires unsupported CPU reset\n");
-
- return -EINVAL;
- }
-
ret = activate_fw_image(attrs);
if (ret) {
pr_err("Firmware activation failed: %s\n",
@@ -309,25 +333,11 @@ static ssize_t cancel_store(struct kobject *kobj, struct kobj_attribute *attr,
{
struct image_props *attrs = container_of(attr, struct image_props,
image_attrs[LFA_ATTR_CANCEL]);
- 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(®, ®);
+ int ret;
- /*
- * 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;
- }
+ ret = lfa_cancel(attrs);
+ if (ret != 0)
+ return ret;
return count;
}
@@ -367,6 +377,8 @@ static int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
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;
+ else
+ image_name = fw_uuid;
}
attrs->image_dir = kobject_create_and_add(fw_uuid, lfa_dir);
@@ -414,6 +426,10 @@ static int create_fw_images_tree(void)
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");
+ return -ENODEV;
+ }
for (int i = 0; i < num_of_components; i++) {
reg.a0 = LFA_1_0_FN_GET_INVENTORY;
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 3/5] firmware: smccc: add timeout, touch wdt
From: Vedashree Vidwans @ 2025-12-08 22:13 UTC (permalink / raw)
To: salman.nabi, sudeep.holla, andre.przywara, lpieralisi,
mark.rutland
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar, Vedashree Vidwans
In-Reply-To: <20251208221319.1524888-1-vvidwans@nvidia.com>
Enhance PRIME/ACTIVATION functions to touch watchdog and implement
timeout mechanism. This update ensures that any potential hangs are
detected promptly and that the LFA process is allocated sufficient
execution time before the watchdog timer expires. These changes improve
overall system reliability by reducing the risk of undetected process
stalls and unexpected watchdog resets.
Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
---
drivers/firmware/smccc/lfa_fw.c | 74 +++++++++++++++++++++++++++++----
1 file changed, 67 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
index df8b65324413..0e420cefa260 100644
--- a/drivers/firmware/smccc/lfa_fw.c
+++ b/drivers/firmware/smccc/lfa_fw.c
@@ -16,6 +16,9 @@
#include <linux/uuid.h>
#include <linux/array_size.h>
#include <linux/list.h>
+#include <linux/nmi.h>
+#include <linux/ktime.h>
+#include <linux/delay.h>
#define LFA_ERROR_STRING(name) \
[name] = #name
@@ -34,6 +37,18 @@
#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)
+
+/* Prime loop limits, TODO: tune after testing */
+#define LFA_PRIME_BUDGET_US 30000000 /* 30s cap */
+#define LFA_PRIME_POLL_DELAY_US 10 /* 10us between polls */
+
+/* Activation loop limits, TODO: tune after testing */
+#define LFA_ACTIVATE_BUDGET_US 20000000 /* 20s cap */
+#define LFA_ACTIVATE_POLL_DELAY_US 10 /* 10us between polls */
+
/* LFA return values */
#define LFA_SUCCESS 0
#define LFA_NOT_SUPPORTED 1
@@ -159,6 +174,8 @@ static int call_lfa_activate(void *data)
struct image_props *attrs = data;
struct arm_smccc_1_2_regs args = { 0 };
struct arm_smccc_1_2_regs res = { 0 };
+ ktime_t end = ktime_add_us(ktime_get(), LFA_ACTIVATE_BUDGET_US);
+ int ret;
args.a0 = LFA_1_0_FN_ACTIVATE;
args.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
@@ -172,9 +189,34 @@ static int call_lfa_activate(void *data)
*/
args.a2 = !(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous);
- do {
+ for (;;) {
+ /* Touch watchdog, ACTIVATE shouldn't take longer than watchdog_thresh */
+ touch_nmi_watchdog();
arm_smccc_1_2_invoke(&args, &res);
- } while (res.a0 == 0 && res.a1 == 1);
+
+ if ((long)res.a0 < 0) {
+ pr_err("ACTIVATE for image %s failed: %s",
+ attrs->image_name, lfa_error_strings[-res.a0]);
+ return res.a0;
+ }
+
+ /* SMC returned with success */
+ if (!(res.a1 & LFA_ACTIVATE_CALL_AGAIN))
+ break; /* ACTIVATE successful */
+
+ /* SMC returned with call_again flag set */
+ if (ktime_before(ktime_get(), end)) {
+ udelay(LFA_ACTIVATE_POLL_DELAY_US);
+ continue;
+ }
+
+ pr_err("ACTIVATE timed out for image %s", attrs->image_name);
+ ret = lfa_cancel(attrs);
+ if (ret == 0)
+ return -ETIMEDOUT;
+ else
+ return ret;
+ }
return res.a0;
}
@@ -183,6 +225,7 @@ static int activate_fw_image(struct image_props *attrs)
{
struct arm_smccc_1_2_regs args = { 0 };
struct arm_smccc_1_2_regs res = { 0 };
+ ktime_t end = ktime_add_us(ktime_get(), LFA_PRIME_BUDGET_US);
int ret;
if (attrs->may_reset_cpu) {
@@ -198,15 +241,32 @@ static int activate_fw_image(struct image_props *attrs)
*/
args.a0 = LFA_1_0_FN_PRIME;
args.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
- do {
+ for (;;) {
+ /* Touch watchdog, PRIME shouldn't take longer than watchdog_thresh */
+ touch_nmi_watchdog();
arm_smccc_1_2_invoke(&args, &res);
- if (res.a0 != LFA_SUCCESS) {
- pr_err("LFA_PRIME failed: %s\n",
- lfa_error_strings[-res.a0]);
+ if ((long)res.a0 < 0) {
+ pr_err("LFA_PRIME for image %s failed: %s\n",
+ attrs->image_name, lfa_error_strings[-res.a0]);
return res.a0;
}
- } while (res.a1 == 1);
+ if (!(res.a1 & LFA_PRIME_CALL_AGAIN))
+ break; /* PRIME successful */
+
+ /* SMC returned with call_again flag set */
+ if (ktime_before(ktime_get(), end)) {
+ udelay(LFA_PRIME_POLL_DELAY_US);
+ continue;
+ }
+
+ pr_err("PRIME timed out for image %s", attrs->image_name);
+ ret = lfa_cancel(attrs);
+ if (ret == 0)
+ return -ETIMEDOUT;
+ else
+ return ret;
+ }
if (attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous)
ret = stop_machine(call_lfa_activate, attrs, cpu_online_mask);
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 1/5] firmware: smccc: LFA: use smcc 1.2
From: Vedashree Vidwans @ 2025-12-08 22:13 UTC (permalink / raw)
To: salman.nabi, sudeep.holla, andre.przywara, lpieralisi,
mark.rutland
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar, Vedashree Vidwans
In-Reply-To: <20251208221319.1524888-1-vvidwans@nvidia.com>
Update driver to use SMCCC 1.2+ version as mentioned in the LFA spec.
Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
---
drivers/firmware/smccc/lfa_fw.c | 102 ++++++++++++++++++++------------
1 file changed, 65 insertions(+), 37 deletions(-)
diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
index 1f333237271d..bdde14b66606 100644
--- a/drivers/firmware/smccc/lfa_fw.c
+++ b/drivers/firmware/smccc/lfa_fw.c
@@ -117,32 +117,38 @@ static struct kobject *lfa_dir;
static int get_nr_lfa_components(void)
{
- struct arm_smccc_res res = { 0 };
+ struct arm_smccc_1_2_regs reg = { 0 };
- arm_smccc_1_1_invoke(LFA_1_0_FN_GET_INFO, 0x0, &res);
- if (res.a0 != LFA_SUCCESS)
- return res.a0;
+ reg.a0 = LFA_1_0_FN_GET_INFO;
+ reg.a1 = 0; /* lfa_info_selector = 0 */
- return res.a1;
+ arm_smccc_1_2_invoke(®, ®);
+ if (reg.a0 != LFA_SUCCESS)
+ return reg.a0;
+
+ return reg.a1;
}
static int call_lfa_activate(void *data)
{
struct image_props *attrs = data;
- struct arm_smccc_res res = { 0 };
+ struct arm_smccc_1_2_regs args = { 0 };
+ struct arm_smccc_1_2_regs res = { 0 };
+
+ args.a0 = LFA_1_0_FN_ACTIVATE;
+ args.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 args.a3 and args.a4, holding the entry point and context
+ * ID respectively.
+ * We want to force CPU rendezvous if either cpu_rendezvous or
+ * cpu_rendezvous_forced is set. The flag value is flipped as
+ * it is called skip_cpu_rendezvous in the spec.
+ */
+ args.a2 = !(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous);
do {
- /*
- * As we do not support updates requiring a CPU reset (yet),
- * we pass 0 in x3 and x4, holding the entry point and context
- * ID respectively.
- * We want to force CPU rendezvous if either cpu_rendezvous or
- * cpu_rendezvous_forced is set. The flag value is flipped as
- * it is called skip_cpu_rendezvous in the spec.
- */
- arm_smccc_1_1_invoke(LFA_1_0_FN_ACTIVATE, attrs->fw_seq_id,
- !(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous),
- 0, 0, &res);
+ arm_smccc_1_2_invoke(&args, &res);
} while (res.a0 == 0 && res.a1 == 1);
return res.a0;
@@ -150,7 +156,8 @@ static int call_lfa_activate(void *data)
static int activate_fw_image(struct image_props *attrs)
{
- struct arm_smccc_res res = { 0 };
+ struct arm_smccc_1_2_regs args = { 0 };
+ struct arm_smccc_1_2_regs res = { 0 };
int ret;
/*
@@ -159,8 +166,10 @@ static int activate_fw_image(struct image_props *attrs)
* LFA_PRIME/ACTIVATE will need to be called again.
* res.a1 will become 0 once the prime/activate process completes.
*/
+ args.a0 = LFA_1_0_FN_PRIME;
+ args.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
do {
- arm_smccc_1_1_invoke(LFA_1_0_FN_PRIME, attrs->fw_seq_id, &res);
+ arm_smccc_1_2_invoke(&args, &res);
if (res.a0 != LFA_SUCCESS) {
pr_err("LFA_PRIME failed: %s\n",
lfa_error_strings[-res.a0]);
@@ -211,15 +220,17 @@ static ssize_t activation_pending_show(struct kobject *kobj,
{
struct image_props *attrs = container_of(attr, struct image_props,
image_attrs[LFA_ATTR_ACT_PENDING]);
- struct arm_smccc_res res = { 0 };
+ struct arm_smccc_1_2_regs reg = { 0 };
/*
* Activation pending status can change anytime thus we need to update
* and return its current value
*/
- arm_smccc_1_1_invoke(LFA_1_0_FN_GET_INVENTORY, attrs->fw_seq_id, &res);
- if (res.a0 == LFA_SUCCESS)
- attrs->activation_pending = !!(res.a3 & BIT(1));
+ 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);
}
@@ -298,21 +309,23 @@ static ssize_t cancel_store(struct kobject *kobj, struct kobj_attribute *attr,
{
struct image_props *attrs = container_of(attr, struct image_props,
image_attrs[LFA_ATTR_CANCEL]);
- struct arm_smccc_res res = { 0 };
+ struct arm_smccc_1_2_regs reg = { 0 };
- arm_smccc_1_1_invoke(LFA_1_0_FN_CANCEL, attrs->fw_seq_id, &res);
+ 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 (res.a0 == LFA_SUCCESS) {
+ 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[-res.a0]);
+ lfa_error_strings[-reg.a0]);
return -EINVAL;
}
@@ -395,21 +408,24 @@ static int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
static int create_fw_images_tree(void)
{
- struct arm_smccc_res res = { 0 };
+ 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();
+
for (int i = 0; i < num_of_components; i++) {
- arm_smccc_1_1_invoke(LFA_1_0_FN_GET_INVENTORY, i, &res);
- if (res.a0 == LFA_SUCCESS) {
- image_uuid.uuid_lo = res.a1;
- image_uuid.uuid_hi = res.a2;
+ 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 = create_fw_inventory(image_id_str, i, res.a3);
+ ret = create_fw_inventory(image_id_str, i, reg.a3);
if (ret)
return ret;
}
@@ -420,17 +436,29 @@ static int create_fw_images_tree(void)
static int __init lfa_init(void)
{
- struct arm_smccc_res res = { 0 };
+ struct arm_smccc_1_2_regs reg = { 0 };
int err;
- arm_smccc_1_1_invoke(LFA_1_0_FN_GET_VERSION, &res);
- if (res.a0 == -LFA_NOT_SUPPORTED) {
+ /* LFA requires SMCCC version >= 1.2 */
+ if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_2) {
+ pr_err("Not supported with SMCCC version %u", arm_smccc_get_version());
+ return -ENODEV;
+ }
+
+ if (arm_smccc_1_1_get_conduit() == SMCCC_CONDUIT_NONE) {
+ pr_err("Invalid SMCCC conduit");
+ return -ENODEV;
+ }
+
+ reg.a0 = LFA_1_0_FN_GET_VERSION;
+ arm_smccc_1_2_invoke(®, ®);
+ if (reg.a0 == -LFA_NOT_SUPPORTED) {
pr_err("Arm Live Firmware activation(LFA): no firmware agent found\n");
return -ENODEV;
}
pr_info("Arm Live Firmware Activation (LFA): detected v%ld.%ld\n",
- res.a0 >> 16, res.a0 & 0xffff);
+ reg.a0 >> 16, reg.a0 & 0xffff);
lfa_dir = kobject_create_and_add("lfa", firmware_kobj);
if (!lfa_dir)
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 0/5] Arm LFA: Improvements and interrupt support
From: Vedashree Vidwans @ 2025-12-08 22:13 UTC (permalink / raw)
To: salman.nabi, sudeep.holla, andre.przywara, lpieralisi,
mark.rutland
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar, Vedashree Vidwans
Hello,
The patches update the proposed Arm Live Firmware Activation (LFA)
kernel driver [1] to incorporate review feedback [2] and refine the
activation flow while remaining aligned with the LFA specification
DEN0147 [3] and the SMCCC 1.2 calling convention. The series keeps
the existing functionality but restructures and extends it to improve
robustness, reviewability, and future extensibility.
The SMCCC usage in the driver is updated to consistently use the
SMCCC 1.2 register-based calling convention, consolidating arguments
and results into a single struct to reduce stack usage and simplify
the SMC interface. The patches also split the original changes into
focused pieces and document the device node bindings in the commit
messages, making it easier to follow and validate the implementation
against the specification.
The kernel driver is registered as a platform driver in accordence to
the LFA device defined by the specification [3]. The driver now extends
interface for interrupt-based enablement of LFA. During LFA, the
interrupt
thread refreshes firmware component details after each activation step
and iterates over all activable components until no further activation
is pending, matching the spec’s allowance for component detail changes
after activation. This ensures that sysfs exposure of LFA components
remains consistent with the authoritative information provided by the
secure firmware.
The handling of CPU rendezvous is adjusted so that the kernel now
honors the rendezvous policy chosen by the firmware, instead of
unconditionally forcing a rendezvous. This reflects experience with
existing firmware deployments where mandatory rendezvous is not
required, while still allowing the firmware to request it when
needed.
Thank you,
Veda
[1] https://lore.kernel.org/lkml/20250926123145.268728-1-salman.nabi@arm.com/
[2] https://lkml.org/lkml/2025/10/8/980
[3] https://developer.arm.com/documentation/den0147/latest/
Vedashree Vidwans (5):
firmware: smccc: LFA: use smcc 1.2
firmware: smccc: LFA: refactor
firmware: smccc: add timeout, touch wdt
firmware: smccc: register as platform driver
firmware: smccc: lfa: refresh fw details
drivers/firmware/smccc/Kconfig | 3 +-
drivers/firmware/smccc/lfa_fw.c | 478 +++++++++++++++++++++++++-------
2 files changed, 380 insertions(+), 101 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Edgecombe, Rick P @ 2025-12-08 20:27 UTC (permalink / raw)
To: Zhao, Yan Y
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Wu, Binbin, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, Yamahata, Isaku,
kirill.shutemov@linux.intel.com, tglx@linutronix.de,
Annapurve, Vishal, Gao, Chao, bp@alien8.de, x86@kernel.org
In-Reply-To: <aTaXQSu0j/9Y0bsY@yzhao56-desk.sh.intel.com>
On Mon, 2025-12-08 at 17:15 +0800, Yan Zhao wrote:
> On failure to allocate the 2nd page, instead of zeroing out the rest of the
> array and having the caller invoke free_pamt_array() to free the 1st page,
> could we free the 1st page here directly?
>
> Upon alloc_pamt_array() error, the pages shouldn't have been passed to the TDX
> module, so there's no need to invoke tdx_quirk_reset_paddr() as in
> free_pamt_array(), right?
We don't want to optimize the error path. So I think less code is better than
making it slightly faster in an ultra rare case.
>
> It's also somewhat strange to have the caller to invoke free_pamt_array() after
> alloc_pamt_array() fails.
Yea, I agree this code is a bit odd. Based on the discussion with Nikolay and
Dave, this can be less variable. It should get simplified as part of that.
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Vlastimil Babka @ 2025-12-08 11:07 UTC (permalink / raw)
To: Michael Roth, FirstName LastName
Cc: ackerleytng, aik, ashish.kalra, david, ira.weiny, kvm,
liam.merwick, linux-coco, linux-kernel, linux-mm, pbonzini,
seanjc, thomas.lendacky, yan.y.zhao
In-Reply-To: <20251203231208.vsoibqhlosy2cjxs@amd.com>
On 12/4/25 00:12, Michael Roth wrote:
> On Wed, Dec 03, 2025 at 08:59:10PM +0000, FirstName LastName wrote:
>>
>> e.g. 4K page based population logic will keep things simple and can be
>> further simplified if we can add PAGE_ALIGNED(params.uaddr) restriction.
>
> I'm still hesitant to pull the trigger on retroactively enforcing
> page-aligned uaddr for SNP, but if the maintainers are good with it then
> no objection from me.
IMHO it would be for the best. If there are no known users that would break,
it's worth trying. The "do not break userspace" rule isn't about eliminating
any theoretical possibility, but indeed about known breakages (and reacting
appropriately to reports about previously unknown breakages). Perhaps any
such users would be also willing to adjust and not demand a revert.
^ permalink raw reply
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