* Re: [PATCH v4 18/24] iommu/arm-smmu-v3: Introduce master->ats_broken flag
From: Nicolin Chen @ 2026-06-05 21:56 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Helgaas,
Rafael J . Wysocki, Len Brown, Pranjal Shrivastava, Mostafa Saleh,
Lu Baolu, Kevin Tian, linux-arm-kernel, iommu, linux-kernel,
linux-acpi, linux-pci, vsethi, Shuai Xue
In-Reply-To: <20260605194259.GE1962447@nvidia.com>
Thanks for the reply.
This is indeed a very complex and sophisticated topic..
On Fri, Jun 05, 2026 at 04:42:59PM -0300, Jason Gunthorpe wrote:
> I don't see any of these options as appealing. We have to maintain a
> few key invariants, and I think it cannot be done without a way to
> find all the domains that are using the STE.
>
> One way or another you have to be using the invs list rw locks to
> synchronize the EATS state changes.
>
> It is okayish to be sloppy when turning EATS off, but when turning it
> back on we do need to cycle through every invs list and toggle its
> lock to ensure that the invalidations are synchronized before
> EATS=enable happens.
I think the core guarantees that "cycle through every invs list"
happens: a PCI reset calls reset_prepare() blocking all the RID
and PASID domains and removing ATS entries from every invs list,
and then calls reset_done() that re-attach RID/PASID domains so
freshly new ATS entries will be installed before EATS=enable.
So, I think the enable path is not an issue, though the disable
path or the invalidation path would need "a way to find all the
domains that are using the STE".
> Given you must have a way to go from STE -> master -> all invs lists
> I'm not sure either option really makes such a large difference.
>
> If so then adjusting the invs to disable the ATS is pretty simple, run
> over the xarray and set them all off. Yes you could find the master
> through a SID lookup with some locking adjustment.
> >
> > (1) Per-invs marker: INV_TYPE_ATS_BROKEN + master_domains
> > disable_ats() in the timeout path walks master->master_domains
> > and flips matching ATS invs entries to the BROKEN type.
> >
> > + invs walker is free (one case label in the existing type switch).
> > + No lock or pointer deref in the invs walker.
> > + No master pointer stored in invs; no lifetime concern.
> >
> > - disable_ats() walks every (master, domain) and marks each invs
> > set; the list needs locking usable from atomic.
>
> This doesn't seem so bad
Yea, the only thing is that the disable path has to deal with a
complexity from going through a per-device domain list. Maybe it
can reuse iommu_group->pasid_array by taking xa_lock?
> > (2) Per-master flag + streams_lock
> > invs walker resolves SID -> master via streams_lock and reads
> > master->ats_broken.
> >
> > + Single source of truth on the master.
> > + disable_ats() is one WRITE_ONCE.
> > + atc_inv_master early-skips via one READ_ONCE.
> > + attach gates ats_enabled on the flag; a concurrent quarantine
> > race can be closed by a short post-attach re-check in commit()
> > + No master pointer in invs; no lifetime concern.
> >
> > - invs walker pays streams_lock + rb_find(SID) per ATS entry on
> > every invalidation. Measurable on ATS-heavy workloads.
>
> Doesn't consider how to enable
The enable side is core-driven: when reset_done() re-attaches
the device from blocked_domain back to its RID/PASID domains,
the new attach_dev callback (old_domain == blocked_domain) can
clear the per-master flag. If the device is still broken, then
arm_smmu_atc_inv_master() at the end of attach_commit() times
out and re-triggers quarantine.
The flaw lives in the invalidation path as it must translate
every SID to master using streams_lock + rb_find(SID) per ATS
entry, which make it very less attractive.
> > (3) Per-master flag + inv->master pointer (v4)
> > invs entry carries a master pointer; the invs walker reads
> > cur->master->ats_broken directly.
> >
> > + invs walker is one READ_ONCE through a cached pointer.
> > + disable_ats is one WRITE_ONCE.
> > + atc_inv_master early-skip via one READ_ONCE.
> > + attach gate + post-attach re-check, same as (2).
> >
> > - invs holds a master ptr, so release_device must synchronize_rcu()
> > before freeing the master to drain walkers under rcu_read_lock().
> > We dropped this from v4 for that reason.
>
> synchronize_rcu is not right because you have to have gone through the
> rwlock so there can be no readers.
Ah, I think you are right! When release_device() is invoked, the
device must be already in the release (blocked) domain. So there
should be no domain->invs in the system holding its ATS entries.
And the enable part would work as (2).
In this case, (3) seems the best? It's fast on every aspect.
And I think it would fit we plan to generalize the invs design:
struct inv {
struct arm_smmu_device *smmu; // => struct iommu_device *iommu;
struct arm_smmu_master *master; // => void *priv;
// (dev->iommu->priv)
Thanks
Nicolin
^ permalink raw reply
* Re: [PATCH 4/4] arm64: route crash_smp_send_stop() last resort through SDEI
From: Kiryl Shutsemau @ 2026-06-05 21:46 UTC (permalink / raw)
To: Doug Anderson
Cc: Catalin Marinas, Will Deacon, James Morse, Mark Rutland,
Marc Zyngier, Petr Mladek, Thomas Gleixner, Andrew Morton,
Baoquan He, Puranjay Mohan, Usama Arif, Breno Leitao,
Julien Thierry, Lecopzer Chen, Sumit Garg, kernel-team, kexec,
linux-arm-kernel, linux-kernel
In-Reply-To: <CAD=FV=X5++c-6Wd6babajiPbn07cfPcG0uW3ZeepznXgSVO2+w@mail.gmail.com>
On Fri, Jun 05, 2026 at 01:42:57PM -0700, Doug Anderson wrote:
> > + sdei_nmi_crash_smp_send_stop();
>
> It feels weird to me that you're adding SDEI for "crash stop" but not
> for regular "stop". It feels like you should modify smp_send_stop() to
> fall back to SDEI if sending the NMI failed, instead of adding this
> separate path.
Fair. A wedged CPU ignores the reboot-path stop just the same, and the
escalation logic already lives in smp.c, so I'll restructure in v2.
One thing to sort out there: this patch parks the stopped CPU inside
its SDEI handler without completing the event, which is fine for the
crash case (nothing expects the CPU back before reset), but a generic
stop path probably wants SDEI_EVENT_COMPLETE_AND_RESUME into a parking
stub instead, so that e.g. a regular kexec can bring all CPUs back up
in the new kernel. I'll look into that as part of the rework.
> > + cpu_park_loop();
> > + /* unreachable */
>
> Any chance we could avoid duplicating stuff from ipi_cpu_crash_stop()?
Yes -- falls out of the above. I will look into this.
Maybe pull the save/offline/park body into a shared helper that both the
IPI handler and the SDEI handler call.
> > +bool sdei_nmi_crash_smp_send_stop(void)
> > +{
> > + unsigned int this_cpu, cpu, remaining;
> > + unsigned long timeout;
> > + cpumask_t mask;
>
> The above will probably get you a yell. Putting "cpumask_t" on the
> stack is a no-no since it can be quite large under certain CONFIG
> options. This is why it's nearly always defined as "static".
Doh! Will make it static in v2 -- safe here since the path is serialized
by the crash_stop guard.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 2/4] drivers/firmware: add SDEI cross-CPU NMI service for arm64
From: Kiryl Shutsemau @ 2026-06-05 21:29 UTC (permalink / raw)
To: Doug Anderson
Cc: Catalin Marinas, Will Deacon, James Morse, Mark Rutland,
Marc Zyngier, Petr Mladek, Thomas Gleixner, Andrew Morton,
Baoquan He, Puranjay Mohan, Usama Arif, Breno Leitao,
Julien Thierry, Lecopzer Chen, Sumit Garg, kernel-team, kexec,
linux-arm-kernel, linux-kernel
In-Reply-To: <CAD=FV=XMqFVnri1aVGbFJhN6Ts3SeJUzEZrfN0Pqp9WeOzE=OA@mail.gmail.com>
On Fri, Jun 05, 2026 at 01:54:00PM -0700, Doug Anderson wrote:
> Hi,
>
> On Wed, Jun 3, 2026 at 7:36 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
> >
> > @@ -928,11 +929,19 @@ static void arm64_backtrace_ipi(cpumask_t *mask)
> > void arch_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu)
> > {
> > /*
> > + * Prefer the SDEI cross-CPU NMI provider when active: firmware
> > + * dispatches the event out of EL3 and reaches CPUs that have
> > + * interrupts locally masked, without the per-IRQ-mask cost that
> > + * pseudo-NMI pays for the same reach. The plain IPI path below
> > + * can't reach such a CPU unless pseudo-NMI is enabled.
> > + *
> > * NOTE: though nmi_trigger_cpumask_backtrace() has "nmi_" in the name,
> > * nothing about it truly needs to be implemented using an NMI, it's
> > * just that it's _allowed_ to work with NMIs. If ipi_should_be_nmi()
> > * returned false our backtrace attempt will just use a regular IPI.
> > */
> > + if (sdei_nmi_trigger_cpumask_backtrace(mask, exclude_cpu))
> > + return;
> > nmi_trigger_cpumask_backtrace(mask, exclude_cpu, arm64_backtrace_ipi);
>
> nit: instead of one comment block, I would have broken it up in two. Like:
>
> /*
> * Prefer the SDEI ...
> */
> if (sdei_nmi_trigger_cpumask_backtrace(mask, exclude_cpu))
> return;
>
> /*
> * NOTE: though ...
> */
> nmi_trigger_cpumask_backtrace(...);
Makes sense.
> > }
> >
> > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > index bbd2155d8483..6501087ff90d 100644
> > --- a/drivers/firmware/Kconfig
> > +++ b/drivers/firmware/Kconfig
> > @@ -36,6 +36,25 @@ config ARM_SDE_INTERFACE
> > standard for registering callbacks from the platform firmware
> > into the OS. This is typically used to implement RAS notifications.
> >
> > +config ARM_SDEI_NMI
> > + bool "SDEI-based cross-CPU NMI service (arm64)"
> > + depends on ARM64 && ARM_SDE_INTERFACE
> > + help
> > + Provides SDEI-based cross-CPU NMI delivery for hooks that need
> > + to reach interrupt-masked CPUs on silicon that lacks FEAT_NMI:
> > +
> > + - arch_trigger_cpumask_backtrace() (sysrq-l, RCU stalls,
> > + hardlockup_all_cpu_backtrace, soft-lockup secondary dumps,
> > + hung-task auxiliary dumps)
> > +
> > + The driver registers a handler for the SDEI software-signalled
> > + event (event 0) and reaches a target CPU by signalling it with
> > + SDEI_EVENT_SIGNAL. Firmware delivers the event out of EL3
> > + regardless of the target's PSTATE.DAIF -- forced delivery into a
> > + CPU wedged with interrupts locally masked.
> > +
> > + If unsure, say N.
>
> Is there some downside to this? It seems like anyone who has the SDE
> interface would want this. Not sure why you'd suggest people say "N".
No real downside -- without the software-signalled event the driver
stays inert, and there is no cost until an event actually fires.
The "say N" is caution, not a technical limit: so far this has run on
QEMU (TF-A) and one hardware platform, and the interesting paths depend
on each vendor's SDEI implementation at EL3. I'm not sure vendors would
care to run SDEI_EVENT_SIGNAL validation. Maybe we want to see more
data points first?
But maybe I am too cautious. Happy to flip the recommendation (or add
default y) in v2 if that the consensus.
> Other than the nit, this looks reasoanble to me, though I'm a complete
> noob when it comes to SDEI...
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
Thanks!
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 3/4] arm64: wire SDEI NMI into the hardlockup watchdog
From: Kiryl Shutsemau @ 2026-06-05 21:11 UTC (permalink / raw)
To: Doug Anderson
Cc: Catalin Marinas, Will Deacon, James Morse, Mark Rutland,
Marc Zyngier, Petr Mladek, Thomas Gleixner, Andrew Morton,
Baoquan He, Puranjay Mohan, Usama Arif, Breno Leitao,
Julien Thierry, Lecopzer Chen, Sumit Garg, kernel-team, kexec,
linux-arm-kernel, linux-kernel
In-Reply-To: <CAD=FV=U4eJ__dQc1e8CGgj5sMDNrD1MgEEy9Cgj9M5n-WmYAXA@mail.gmail.com>
On Fri, Jun 05, 2026 at 01:03:05PM -0700, Doug Anderson wrote:
> Hi,
>
> On Wed, Jun 3, 2026 at 7:36 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
> >
> > From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
> >
> > Select HAVE_HARDLOCKUP_DETECTOR_ARCH so the framework takes its backend
> > from this driver. A per-CPU hrtimer checks its buddy's heartbeat and
> > signals event 0 at a stalled CPU, which runs watchdog_hardlockup_check()
> > NMI-like.
> >
> > The source is chosen at boot: SDEI if firmware provides it, otherwise a
> > perf-NMI counter (pseudo-NMI) fallback -- one image covers both.
> >
> > Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> > ---
> > arch/arm64/Kconfig | 1 +
> > drivers/firmware/Kconfig | 3 +
> > drivers/firmware/sdei_nmi.c | 247 +++++++++++++++++++++++++++++++++++-
> > 3 files changed, 248 insertions(+), 3 deletions(-)
>
> I'm a little confused about this patch. We already have a buddy
> hardlockup detector using the hrtimer, and it's even been improved
> recently to trigger in a smaller time bound. It looks as if you're
> duplicating bits of the perf and buddy detector here?
>
> I don't think you need this patch at all. The existing buddy detector
> + patches #1 and #2 in your series should be sufficient.
You're mostly right.
Buddy + #2 covers the console case (the remote branch triggers the
culprit's backtrace, which #2 makes deliverable), and #4 gets the wedged
CPU's registers into the vmcore.
The one thing this patch adds that a config can't is boot-time source
selection: PERF-compiled kernels have no detector on a pseudo_nmi=0
boot, and PREFER_BUDDY costs the pseudo-NMI machines perf
self-detection. But that's arguably out of scope for the patchset.
I'll drop this patch in v2 and run PREFER_BUDDY here. If a runtime
perf->buddy fallback ever materializes, the gap closes entirely.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH bpf-next v2 8/8] selftests/bpf: add tests to validate KASAN on JIT programs
From: Alexis Lothoré @ 2026-06-05 20:55 UTC (permalink / raw)
To: Yonghong Song, Alexis Lothoré, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu, Jiri Olsa,
John Fastabend, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Shuah Khan, Maxime Coquelin,
Alexandre Torgue, Ihor Solodrai
Cc: ebpf, Bastien Curutchet, Thomas Petazzoni, bpf, linux-kernel,
linux-kselftest, linux-stm32, linux-arm-kernel
In-Reply-To: <f73d0971-0544-4a92-bde7-b2fbfcdaf28b@linux.dev>
On Fri Jun 5, 2026 at 7:20 PM CEST, Yonghong Song wrote:
[...]
>> Are you seeing any kasan report when you manually check your kernel
>> logs, or not at all ? If not at all, are you using the "CI" defconfig ?
>
> I do see one report:
>
> [ 79.503059] ==================================================================
> [ 79.503715] BUG: KASAN: slab-use-after-free in bpf_prog_bb753b2ee1f69aa0_st_not_on_stack+0x115/0x160
> [ 79.503715] Write of size 1 at addr ff11000117210a20 by task test_progs/2153
>
> [ 79.503715] CPU: 6 UID: 0 PID: 2153 Comm: test_progs Tainted: G OE 7.1.0-rc5-gd552a156c2fa #1926 PREEMPT(full)
> [ 79.503715] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> [ 79.503715] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> [ 79.503715] Call Trace:
> [ 79.503715] <TASK>
> [ 79.503715] dump_stack_lvl+0x6d/0xa0
> [ 79.503715] print_address_description+0x77/0x200
> [ 79.503715] print_report+0x58/0x70
> [ 79.503715] ? bpf_prog_bb753b2ee1f69aa0_st_not_on_stack+0x115/0x160
> [ 79.503715] kasan_report+0xa2/0xe0
> [ 79.503715] ? bpf_prog_bb753b2ee1f69aa0_st_not_on_stack+0x115/0x160
> [ 79.503715] ? bpf_test_run+0x208/0x770
> [ 79.503715] bpf_prog_bb753b2ee1f69aa0_st_not_on_stack+0x115/0x160
> [ 79.503715] bpf_test_run+0x472/0x770
> [ 79.503715] ? srso_alias_return_thunk+0x5/0xfbef5
> [ 79.503715] ? __lock_acquire+0xe4a/0x2a10
> [ 79.503715] ? __pfx___css_rstat_updated+0x10/0x10
> [ 79.503715] ? __lock_acquire+0xe4a/0x2a10
> [ 79.503715] ? __pfx_bpf_test_run+0x10/0x10
> [ 79.503715] ? srso_alias_return_thunk+0x5/0xfbef5
> [ 79.503715] ? lock_acquire+0xfd/0x2b0
> [ 79.503715] ? srso_alias_return_thunk+0x5/0xfbef5
> [ 79.503715] ? srso_alias_return_thunk+0x5/0xfbef5
> [ 79.503715] ? rcu_is_watching+0x1f/0xa0
> [ 79.503715] ? srso_alias_return_thunk+0x5/0xfbef5
> [ 79.503715] ? __kasan_krealloc+0xe9/0x110
> [ 79.503715] ? eth_type_trans+0x4b9/0x5f0
> [ 79.503715] bpf_prog_test_run_skb+0xddf/0x22f0
> [ 79.503715] ? __fget_files+0x29/0x350
> [ 79.503715] ? srso_alias_return_thunk+0x5/0xfbef5
> [ 79.503715] ? __fget_files+0x29/0x350
> [ 79.503715] bpf_prog_test_run+0x1cc/0x2d0
> [ 79.503715] __sys_bpf+0x740/0xa30
> [ 79.503715] ? __pfx___sys_bpf+0x10/0x10
> [ 79.503715] ? _prb_read_valid+0x334/0x770
> [ 79.503715] ? handle_mm_fault+0x91b/0xc00
> [ 79.503715] __x64_sys_bpf+0xba/0xd0
> [ 79.503715] do_syscall_64+0xee/0x400
> [ 79.503715] ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 79.503715] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 79.503715] RIP: 0033:0x7f92d8cfe1ad
> [ 79.503715] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 8
> [ 79.503715] RSP: 002b:00007ffe4237fee8 EFLAGS: 00000206 ORIG_RAX: 0000000000000141
> [ 79.503715] RAX: ffffffffffffffda RBX: 00007ffe423807b8 RCX: 00007f92d8cfe1ad
> [ 79.503715] RDX: 0000000000000050 RSI: 00007ffe4237ff70 RDI: 000000000000000a
> [ 79.503715] RBP: 00007ffe4237ff10 R08: 0000000000000000 R09: 0000000000000050
> [ 79.503715] R10: 0000000000000064 R11: 0000000000000206 R12: 0000000000000000
> [ 79.503715] R13: 00007ffe423807d8 R14: 00007f92d8eb9000 R15: 00005585778dd150
> [ 79.503715] </TASK>
>
> [ 79.503715] Allocated by task 2153:
> [ 79.503715] kasan_save_track+0x2f/0x70
> [ 79.503715] __kasan_kmalloc+0x72/0x90
> [ 79.503715] __kmalloc_node_noprof+0x34c/0x730
> [ 79.503715] bpf_map_area_alloc+0x4a/0x110
> [ 79.503715] array_map_alloc+0x19e/0x580
> [ 79.503715] map_create+0x8b2/0x1500
> [ 79.503715] __sys_bpf+0x7ea/0xa30
> [ 79.503715] __x64_sys_bpf+0xba/0xd0
> [ 79.503715] do_syscall_64+0xee/0x400
> [ 79.503715] entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> [ 79.503715] The buggy address belongs to the object at ff11000117210800
> which belongs to the cache kmalloc-cg-1k of size 1024
> [ 79.503715] The buggy address is located 0 bytes to the right of
> freed 544-byte region [ff11000117210800, ff11000117210a20)
>
> [ 79.503715] The buggy address belongs to the physical page:
> [ 79.503715] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x117210
> [ 79.503715] head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> [ 79.503715] memcg:ff11000117210411
> [ 79.503715] flags: 0x200000000000040(head|node=0|zone=2)
> [ 79.503715] page_type: f5(slab)
> [ 79.503715] raw: 0200000000000040 ff11000100072000 dead000000000100 dead000000000122
> [ 79.503715] raw: 0000000000000000 0000080000100010 00000000f5000000 ff11000117210411
> [ 79.503715] head: 0200000000000040 ff11000100072000 dead000000000100 dead000000000122
> [ 79.503715] head: 0000000000000000 0000080000100010 00000000f5000000 ff11000117210411
> [ 79.503715] head: 0200000000000003 fffffffffffffe01 00000000ffffffff 00000000ffffffff
> [ 79.503715] head: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000008
> [ 79.503715] page dumped because: kasan: bad access detected
>
> [ 79.503715] Memory state around the buggy address:
> [ 79.503715] ff11000117210900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 79.503715] ff11000117210980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 79.503715] >ff11000117210a00: 00 00 00 00 fb fb fc fc fc fc fc fc fc fc fc fc
> [ 79.503715] ^
> [ 79.503715] ff11000117210a80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ 79.503715] ff11000117210b00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ 79.503715] ==================================================================
>
>
> But when I am running another same test './test_progs -t kasan', there is no kasan reports.
Ok, I guess you are missing kasan_multi_shot on your kernel command
line: without this option, only the first report is generated, then
KASAN does not emit additional report until you restart your kernel.
Could you please try adding it and running the tests again ?
Thanks,
Alexis
>>
>> cat tools/testing/selftests/bpf/{config,config.vm,config.x86_64} > .config && make olddefconfig
>>
>> If not, would you mind sharing your defconfig ?
>
> Attached.
>
>>
>> Thanks,
>>
>> Alexis
--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 2/4] drivers/firmware: add SDEI cross-CPU NMI service for arm64
From: Doug Anderson @ 2026-06-05 20:54 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Catalin Marinas, Will Deacon, James Morse, Mark Rutland,
Marc Zyngier, Petr Mladek, Thomas Gleixner, Andrew Morton,
Baoquan He, Puranjay Mohan, Usama Arif, Breno Leitao,
Julien Thierry, Lecopzer Chen, Sumit Garg, kernel-team, kexec,
linux-arm-kernel, linux-kernel, Kiryl Shutsemau (Meta)
In-Reply-To: <145b9e98b12a7d314fc4a203075f65c3a0c3a913.1780496779.git.kas@kernel.org>
Hi,
On Wed, Jun 3, 2026 at 7:36 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
>
> @@ -928,11 +929,19 @@ static void arm64_backtrace_ipi(cpumask_t *mask)
> void arch_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu)
> {
> /*
> + * Prefer the SDEI cross-CPU NMI provider when active: firmware
> + * dispatches the event out of EL3 and reaches CPUs that have
> + * interrupts locally masked, without the per-IRQ-mask cost that
> + * pseudo-NMI pays for the same reach. The plain IPI path below
> + * can't reach such a CPU unless pseudo-NMI is enabled.
> + *
> * NOTE: though nmi_trigger_cpumask_backtrace() has "nmi_" in the name,
> * nothing about it truly needs to be implemented using an NMI, it's
> * just that it's _allowed_ to work with NMIs. If ipi_should_be_nmi()
> * returned false our backtrace attempt will just use a regular IPI.
> */
> + if (sdei_nmi_trigger_cpumask_backtrace(mask, exclude_cpu))
> + return;
> nmi_trigger_cpumask_backtrace(mask, exclude_cpu, arm64_backtrace_ipi);
nit: instead of one comment block, I would have broken it up in two. Like:
/*
* Prefer the SDEI ...
*/
if (sdei_nmi_trigger_cpumask_backtrace(mask, exclude_cpu))
return;
/*
* NOTE: though ...
*/
nmi_trigger_cpumask_backtrace(...);
> }
>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index bbd2155d8483..6501087ff90d 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -36,6 +36,25 @@ config ARM_SDE_INTERFACE
> standard for registering callbacks from the platform firmware
> into the OS. This is typically used to implement RAS notifications.
>
> +config ARM_SDEI_NMI
> + bool "SDEI-based cross-CPU NMI service (arm64)"
> + depends on ARM64 && ARM_SDE_INTERFACE
> + help
> + Provides SDEI-based cross-CPU NMI delivery for hooks that need
> + to reach interrupt-masked CPUs on silicon that lacks FEAT_NMI:
> +
> + - arch_trigger_cpumask_backtrace() (sysrq-l, RCU stalls,
> + hardlockup_all_cpu_backtrace, soft-lockup secondary dumps,
> + hung-task auxiliary dumps)
> +
> + The driver registers a handler for the SDEI software-signalled
> + event (event 0) and reaches a target CPU by signalling it with
> + SDEI_EVENT_SIGNAL. Firmware delivers the event out of EL3
> + regardless of the target's PSTATE.DAIF -- forced delivery into a
> + CPU wedged with interrupts locally masked.
> +
> + If unsure, say N.
Is there some downside to this? It seems like anyone who has the SDE
interface would want this. Not sure why you'd suggest people say "N".
Other than the nit, this looks reasoanble to me, though I'm a complete
noob when it comes to SDEI...
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* Re: [PATCH 1/4] firmware: arm_sdei: add SDEI_EVENT_SIGNAL support
From: Doug Anderson @ 2026-06-05 20:46 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Catalin Marinas, Will Deacon, James Morse, Mark Rutland,
Marc Zyngier, Petr Mladek, Thomas Gleixner, Andrew Morton,
Baoquan He, Puranjay Mohan, Usama Arif, Breno Leitao,
Julien Thierry, Lecopzer Chen, Sumit Garg, kernel-team, kexec,
linux-arm-kernel, linux-kernel, Kiryl Shutsemau (Meta)
In-Reply-To: <ba8074cdb9ca5a471162cbc15f775c1567a3992a.1780496779.git.kas@kernel.org>
Hi,
On Wed, Jun 3, 2026 at 7:36 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
>
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
>
> Add sdei_event_signal(), a thin wrapper over the SDEI_EVENT_SIGNAL call
> (DEN0054) that makes the software-signalled event (event 0) pending on a
> target PE -- delivered NMI-like even when that PE has interrupts masked.
> It takes no locks, so it is safe to call from NMI / crash context.
>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> ---
> drivers/firmware/arm_sdei.c | 12 ++++++++++++
> include/linux/arm_sdei.h | 6 ++++++
> include/uapi/linux/arm_sdei.h | 1 +
> 3 files changed, 19 insertions(+)
I'd never looked at SDEI before this (so my review is probably not
terribly strong), but this looks reasonable to me.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* Re: [PATCH 4/4] arm64: route crash_smp_send_stop() last resort through SDEI
From: Doug Anderson @ 2026-06-05 20:42 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Catalin Marinas, Will Deacon, James Morse, Mark Rutland,
Marc Zyngier, Petr Mladek, Thomas Gleixner, Andrew Morton,
Baoquan He, Puranjay Mohan, Usama Arif, Breno Leitao,
Julien Thierry, Lecopzer Chen, Sumit Garg, kernel-team, kexec,
linux-arm-kernel, linux-kernel, Kiryl Shutsemau (Meta)
In-Reply-To: <54cb99db3c981dc39eb3031aff5caeaadb09e8b9.1780496779.git.kas@kernel.org>
Hi,
On Wed, Jun 3, 2026 at 7:36 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
>
> @@ -1288,8 +1288,32 @@ void crash_smp_send_stop(void)
> return;
> crash_stop = 1;
>
> + /*
> + * Stop the normal way first: IPI_CPU_STOP escalating to a pseudo-NMI
> + * IPI. Every CPU that responds saves its state via crash_save_cpu()
> + * and parks in cpu_park_loop() with its online bit cleared -- the
> + * standard kdump stop, identical to a kernel without SDEI. Crucially
> + * those CPUs stay in a clean, potentially-reusable state.
> + */
> smp_send_stop();
>
> + /*
> + * Whatever is still online didn't respond -- typically a CPU wedged
> + * with interrupts masked. The plain IPI can't reach it, and a fleet
> + * that declines the pseudo-NMI hot-path cost has no NMI IPI to
> + * escalate to. Hit only the survivors with the SDEI cross-CPU NMI
> + * (no-op if SDEI isn't active, or if everything already stopped):
> + * firmware delivers out of EL3 regardless of PSTATE.DAIF, and the
> + * handler captures crash_save_cpu() state from the wedged context
> + * before parking the CPU.
> + *
> + * SDEI is deliberately last: an SDEI-stopped CPU never completes its
> + * event (it parks inside the handler, so EL3 retains its dispatch
> + * slot until reset), which is strictly less recoverable than a normal
> + * stop. We pay that only for CPUs that left no other way to reach them.
> + */
> + sdei_nmi_crash_smp_send_stop();
It feels weird to me that you're adding SDEI for "crash stop" but not
for regular "stop". It feels like you should modify smp_send_stop() to
fall back to SDEI if sending the NMI failed, instead of adding this
separate path.
> static int sdei_nmi_handler(u32 event, struct pt_regs *regs, void *arg)
> {
> + int cpu = smp_processor_id();
> +
> + if (READ_ONCE(*this_cpu_ptr(&sdei_nmi_crash_stop_requested))) {
> + WRITE_ONCE(*this_cpu_ptr(&sdei_nmi_crash_stop_requested), 0);
> +
> + /*
> + * Capture the wedged context for kdump while pt_regs still
> + * points at the interrupted PC. This is the main motivation
> + * for using SDEI here: the plain IPI stop path can't reach an
> + * interrupt-masked CPU (and the fleet declines pseudo-NMI to
> + * keep the IRQ-mask hot path cheap), so crash_save_cpu() for
> + * that CPU would otherwise record nothing useful.
> + */
> + crash_save_cpu(regs, cpu);
> + set_cpu_online(cpu, false);
> +
> + /* publish the crash state/offline before the requester sees the ack */
> + smp_wmb();
> + WRITE_ONCE(*this_cpu_ptr(&sdei_nmi_crash_stop_acked), 1);
> +
> + /*
> + * Park forever from within the SDEI handler. We deliberately
> + * do NOT issue SDEI_EVENT_COMPLETE: the framework's return
> + * path restores firmware's saved interrupted context, which
> + * would land the CPU back wherever it was running (often
> + * do_idle, which then notices cpu_is_offline=true and BUGs
> + * at cpuhp_report_idle_dead). Returning the modified pt_regs
> + * doesn't help -- arch/arm64/kernel/sdei.c::do_sdei_event
> + * only honours a PC override via its IRQ-state heuristic
> + * and otherwise hands EL3 its own saved-context slot back.
> + *
> + * Trade-off: EL3 firmware retains ~one saved-context slot
> + * per parked CPU until the next hardware reset (~hundreds of
> + * bytes per CPU). The CPU itself is parked in cpu_park_loop
> + * exactly as if IPI_CPU_STOP had stopped it; recoverability
> + * is unchanged versus the existing path (neither is
> + * recoverable without hardware reset, since PSCI sees the
> + * CPU as ALREADY_ON in both cases).
> + */
> + cpu_park_loop();
> + /* unreachable */
Any chance we could avoid duplicating stuff from ipi_cpu_crash_stop()?
> +bool sdei_nmi_crash_smp_send_stop(void)
> +{
> + unsigned int this_cpu, cpu, remaining;
> + unsigned long timeout;
> + cpumask_t mask;
The above will probably get you a yell. Putting "cpumask_t" on the
stack is a no-no since it can be quite large under certain CONFIG
options. This is why it's nearly always defined as "static".
-Doug
^ permalink raw reply
* [PATCH 2/3] soc: samsung: exynos-pmu: fix use-after-free of interrupt generator node
From: Alexey Klimov @ 2026-06-05 20:18 UTC (permalink / raw)
To: Krzysztof Kozlowski, Alim Akhtar, Peter Griffin
Cc: Sam Protsenko, linux-samsung-soc, linux-arm-kernel, linux-kernel,
stable, Sashiko
In-Reply-To: <20260605-exynos-pmu-cpuhp-idle-fixes-v1-0-0cd05c81a82d@linaro.org>
The setup_cpuhp_and_cpuidle() parses the device tree node for the
interrupt generation block via of_parse_phandle() and decrements its
reference count using of_node_put() immediately after fetching the resource
address. However, later the intr_gen_node pointer is passed into
of_syscon_register_regmap().
Fix this by moving the of_node_put() invocation to after the
of_syscon_register_regmap() call, and adding it to correct error paths.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
Fixes: 78b72897a5c8 ("soc: samsung: exynos-pmu: Enable CPU Idle for gs101")
Cc: stable@vger.kernel.org
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
drivers/soc/samsung/exynos-pmu.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 6e635872247a..9636287f6794 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -428,23 +428,30 @@ static int setup_cpuhp_and_cpuidle(struct device *dev)
* syscon provided regmap.
*/
ret = of_address_to_resource(intr_gen_node, 0, &intrgen_res);
- of_node_put(intr_gen_node);
+ if (ret) {
+ of_node_put(intr_gen_node);
+ return ret;
+ }
virt_addr = devm_ioremap(dev, intrgen_res.start,
resource_size(&intrgen_res));
- if (!virt_addr)
+ if (!virt_addr) {
+ of_node_put(intr_gen_node);
return -ENOMEM;
+ }
pmu_context->pmuintrgen = devm_regmap_init_mmio(dev, virt_addr,
®map_pmu_intr);
if (IS_ERR(pmu_context->pmuintrgen)) {
dev_err(dev, "failed to initialize pmu-intr-gen regmap\n");
+ of_node_put(intr_gen_node);
return PTR_ERR(pmu_context->pmuintrgen);
}
/* register custom mmio regmap with syscon */
ret = of_syscon_register_regmap(intr_gen_node,
pmu_context->pmuintrgen);
+ of_node_put(intr_gen_node);
if (ret)
return ret;
--
2.51.0
^ permalink raw reply related
* [PATCH 3/3] soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup
From: Alexey Klimov @ 2026-06-05 20:18 UTC (permalink / raw)
To: Krzysztof Kozlowski, Alim Akhtar, Peter Griffin
Cc: Sam Protsenko, linux-samsung-soc, linux-arm-kernel, linux-kernel,
stable, Sashiko
In-Reply-To: <20260605-exynos-pmu-cpuhp-idle-fixes-v1-0-0cd05c81a82d@linaro.org>
The setup_cpuhp_and_cpuidle() initialisation sequence currently ignores
the return values of cpuhp_setup_state(), cpu_pm_register_notifier(), and
register_reboot_notifier(). If any of these registrations fail during
probe() routine, the driver returns 0, leaving the driver partially
configured.
Furthermore, if anything after setup_cpuhp_and_cpuidle() fails in probe()
routine, for instance devm_mfd_add_devices(), the probe() lacks an error
path and leaves notifiers and cpu hotplug states registered.
Introduce variables for the cpu hotplug state IDs in exynos_pmu_context
struct, that should be initialised to CPUHP_INVALID by default. Check all
return codes in setup_cpuhp_and_cpuidle(), and add an error path to remove
registered states on failure. Finally, add destroy_cpuhp_and_cpuidle()
helper to safely tear down notifiers and cpu hotplug states.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
Fixes: 78b72897a5c8 ("soc: samsung: exynos-pmu: Enable CPU Idle for gs101")
Cc: stable@vger.kernel.org
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
drivers/soc/samsung/exynos-pmu.c | 57 ++++++++++++++++++++++++++++++++++------
1 file changed, 49 insertions(+), 8 deletions(-)
diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 9636287f6794..846313a28e9a 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -38,6 +38,8 @@ struct exynos_pmu_context {
unsigned long *in_cpuhp;
bool sys_insuspend;
bool sys_inreboot;
+ int cpuhp_prepare_state;
+ int cpuhp_online_state;
};
void __iomem *pmu_base_addr;
@@ -404,6 +406,17 @@ static struct notifier_block exynos_cpupm_reboot_nb = {
.notifier_call = exynos_cpupm_reboot_notifier,
};
+static void destroy_cpuhp_and_cpuidle(void)
+{
+ cpu_pm_unregister_notifier(&gs101_cpu_pm_notifier);
+ unregister_reboot_notifier(&exynos_cpupm_reboot_nb);
+
+ if (pmu_context->cpuhp_prepare_state != CPUHP_INVALID)
+ cpuhp_remove_state(pmu_context->cpuhp_prepare_state);
+ if (pmu_context->cpuhp_online_state != CPUHP_INVALID)
+ cpuhp_remove_state(pmu_context->cpuhp_online_state);
+}
+
static int setup_cpuhp_and_cpuidle(struct device *dev)
{
struct device_node *intr_gen_node;
@@ -465,16 +478,42 @@ static int setup_cpuhp_and_cpuidle(struct device *dev)
gs101_cpuhp_pmu_online(cpu);
/* register CPU hotplug callbacks */
- cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "soc/exynos-pmu:prepare",
- gs101_cpuhp_pmu_online, NULL);
+ pmu_context->cpuhp_prepare_state = CPUHP_INVALID;
+ pmu_context->cpuhp_online_state = CPUHP_INVALID;
- cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/exynos-pmu:online",
- NULL, gs101_cpuhp_pmu_offline);
+ ret = cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "soc/exynos-pmu:prepare",
+ gs101_cpuhp_pmu_online, NULL);
+ if (ret < 0)
+ return ret;
+
+ pmu_context->cpuhp_prepare_state = ret;
+
+ ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/exynos-pmu:online",
+ NULL, gs101_cpuhp_pmu_offline);
+ if (ret < 0)
+ goto clean_cpuhp_states;
+
+ pmu_context->cpuhp_online_state = ret;
/* register CPU PM notifiers for cpuidle */
- cpu_pm_register_notifier(&gs101_cpu_pm_notifier);
- register_reboot_notifier(&exynos_cpupm_reboot_nb);
- return 0;
+ ret = cpu_pm_register_notifier(&gs101_cpu_pm_notifier);
+ if (ret)
+ goto clean_cpuhp_states;
+
+ ret = register_reboot_notifier(&exynos_cpupm_reboot_nb);
+ if (!ret)
+ /* Success */
+ return ret;
+
+ cpu_pm_unregister_notifier(&gs101_cpu_pm_notifier);
+
+clean_cpuhp_states:
+ if (pmu_context->cpuhp_prepare_state != CPUHP_INVALID)
+ cpuhp_remove_state(pmu_context->cpuhp_prepare_state);
+ if (pmu_context->cpuhp_online_state != CPUHP_INVALID)
+ cpuhp_remove_state(pmu_context->cpuhp_online_state);
+
+ return ret;
}
static int exynos_pmu_probe(struct platform_device *pdev)
@@ -548,8 +587,10 @@ static int exynos_pmu_probe(struct platform_device *pdev)
ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, exynos_pmu_devs,
ARRAY_SIZE(exynos_pmu_devs), NULL, 0, NULL);
- if (ret)
+ if (ret) {
+ destroy_cpuhp_and_cpuidle();
return ret;
+ }
if (devm_of_platform_populate(dev))
dev_err(dev, "Error populating children, reboot and poweroff might not work properly\n");
--
2.51.0
^ permalink raw reply related
* [PATCH 0/3] Exynos PMU fixes for cpu hotplug and cpuidle routines
From: Alexey Klimov @ 2026-06-05 20:18 UTC (permalink / raw)
To: Krzysztof Kozlowski, Alim Akhtar, Peter Griffin
Cc: Sam Protsenko, linux-samsung-soc, linux-arm-kernel, linux-kernel,
stable, Sashiko
This was reported by Sashiko here:
https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
and was mainly introduced by enabling cpu hotplug
support and cpuidle for gs101-based SoCs.
One patch removes strange usage of smp_processor_id() and
other patches deal with a few missing error paths issues
here and there in setup_cpuhp_and_cpuidle() and around.
Tested on gs101-raven device, I don't see any regressions
but testing from others will be appreciated.
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
Alexey Klimov (3):
soc: samsung: exynos-pmu: use target cpu ID in hotplug callbacks
soc: samsung: exynos-pmu: fix use-after-free of interrupt generator node
soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup
drivers/soc/samsung/exynos-pmu.c | 75 ++++++++++++++++++++++++++++++++--------
1 file changed, 60 insertions(+), 15 deletions(-)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260605-exynos-pmu-cpuhp-idle-fixes-32f5ed7c969f
Best regards,
--
Alexey Klimov <alexey.klimov@linaro.org>
^ permalink raw reply
* [PATCH 1/3] soc: samsung: exynos-pmu: use target cpu ID in hotplug callbacks
From: Alexey Klimov @ 2026-06-05 20:18 UTC (permalink / raw)
To: Krzysztof Kozlowski, Alim Akhtar, Peter Griffin
Cc: Sam Protsenko, linux-samsung-soc, linux-arm-kernel, linux-kernel,
stable, Sashiko
In-Reply-To: <20260605-exynos-pmu-cpuhp-idle-fixes-v1-0-0cd05c81a82d@linaro.org>
The CPU hotplug state callbacks __gs101_cpu_pmu_online() and
__gs101_cpu_pmu_offline() currently partially use smp_processor_id() to
determine the target register offset for the CPU inform hints. This may
be fine for cpuidle flow but broken for cpu hotplug where the target
cpu is passed as an argument and could be different from cpu where
that is executing (e.g. CPU 0 offlining CPU 1), meaning that
smp_processor_id() returns the id of local CPU but hotplug flow
deals with another CPU core undergoing the transition.
This causes the pmu driver to write power down and power on configuration
hints to the wrong hardware registers, messing up the power state of active
cores and failing to configure the target core. Fix this by removing the
cpuhint variable entirely and utilizing the target 'cpu' argument passed
to the callbacks by the hotplug core infrastructure.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
Fixes: 598995027b91 ("soc: samsung: exynos-pmu: enable CPU hotplug support for gs101")
Cc: stable@vger.kernel.org
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
drivers/soc/samsung/exynos-pmu.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index d58376c38179..6e635872247a 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -235,11 +235,10 @@ EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap_by_phandle);
static int __gs101_cpu_pmu_online(unsigned int cpu)
__must_hold(&pmu_context->cpupm_lock)
{
- unsigned int cpuhint = smp_processor_id();
u32 reg, mask;
/* clear cpu inform hint */
- regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpuhint),
+ regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu),
CPU_INFORM_CLEAR);
mask = BIT(cpu);
@@ -296,12 +295,10 @@ static int gs101_cpuhp_pmu_online(unsigned int cpu)
static int __gs101_cpu_pmu_offline(unsigned int cpu)
__must_hold(&pmu_context->cpupm_lock)
{
- unsigned int cpuhint = smp_processor_id();
u32 reg, mask;
/* set cpu inform hint */
- regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpuhint),
- CPU_INFORM_C2);
+ regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu), CPU_INFORM_C2);
mask = BIT(cpu);
regmap_update_bits(pmu_context->pmuintrgen, GS101_GRP2_INTR_BID_ENABLE,
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 3/4] arm64: wire SDEI NMI into the hardlockup watchdog
From: Doug Anderson @ 2026-06-05 20:03 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Catalin Marinas, Will Deacon, James Morse, Mark Rutland,
Marc Zyngier, Petr Mladek, Thomas Gleixner, Andrew Morton,
Baoquan He, Puranjay Mohan, Usama Arif, Breno Leitao,
Julien Thierry, Lecopzer Chen, Sumit Garg, kernel-team, kexec,
linux-arm-kernel, linux-kernel, Kiryl Shutsemau (Meta)
In-Reply-To: <6172eafcb9de6e626c0f1c36426d67e1e562ed32.1780496779.git.kas@kernel.org>
Hi,
On Wed, Jun 3, 2026 at 7:36 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
>
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
>
> Select HAVE_HARDLOCKUP_DETECTOR_ARCH so the framework takes its backend
> from this driver. A per-CPU hrtimer checks its buddy's heartbeat and
> signals event 0 at a stalled CPU, which runs watchdog_hardlockup_check()
> NMI-like.
>
> The source is chosen at boot: SDEI if firmware provides it, otherwise a
> perf-NMI counter (pseudo-NMI) fallback -- one image covers both.
>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> ---
> arch/arm64/Kconfig | 1 +
> drivers/firmware/Kconfig | 3 +
> drivers/firmware/sdei_nmi.c | 247 +++++++++++++++++++++++++++++++++++-
> 3 files changed, 248 insertions(+), 3 deletions(-)
I'm a little confused about this patch. We already have a buddy
hardlockup detector using the hrtimer, and it's even been improved
recently to trigger in a smaller time bound. It looks as if you're
duplicating bits of the perf and buddy detector here?
I don't think you need this patch at all. The existing buddy detector
+ patches #1 and #2 in your series should be sufficient.
Did I misunderstand?
-Doug
^ permalink raw reply
* Re: [PATCH v4 18/24] iommu/arm-smmu-v3: Introduce master->ats_broken flag
From: Jason Gunthorpe @ 2026-06-05 19:42 UTC (permalink / raw)
To: Nicolin Chen
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Helgaas,
Rafael J . Wysocki, Len Brown, Pranjal Shrivastava, Mostafa Saleh,
Lu Baolu, Kevin Tian, linux-arm-kernel, iommu, linux-kernel,
linux-acpi, linux-pci, vsethi, Shuai Xue
In-Reply-To: <ah5txKCvspL8zMeG@Asurada-Nvidia>
On Mon, Jun 01, 2026 at 10:44:36PM -0700, Nicolin Chen wrote:
> On Mon, Jun 01, 2026 at 09:15:47PM -0300, Jason Gunthorpe wrote:
> > On Mon, Jun 01, 2026 at 01:41:26PM -0700, Nicolin Chen wrote:
> > > On Mon, Jun 01, 2026 at 09:32:31AM -0300, Jason Gunthorpe wrote:
> > > > On Fri, May 29, 2026 at 06:27:40PM -0700, Nicolin Chen wrote:
> > > > > On Tue, May 19, 2026 at 09:06:58AM -0300, Jason Gunthorpe wrote:
> > > > > > On Mon, May 18, 2026 at 08:39:01PM -0700, Nicolin Chen wrote:
> > > > > So I've tried INV_TYPE_ATS_BROKEN: during per-domain invalidation,
> > > > > each batch is built from domain->invs so it can carry the "invs";
> > > > > if the batch times out, we can immediately mutate its ATS entries.
> > > > >
> > > > > But I realized a limitation. E.g., if a device attaches to two SVA
> > > > > domains on two SSIDs. An invalidation timing out on one of the SVA
> > > > > domains could mark INV_TYPE_ATS_BROKEN in its own invs, but not in
> > > > > the other SVA domain's invs?
> > > >
> > > > You'd have to mark all the S1's sharing the STE.
> > >
> > > That would be a bit convoluted as we would have to go through all
> > > other domains' invs arrays.
> >
> > Ok, that is certainly an annoying problem.
> >
> > I don't have a better idea than storing the master unfortunately
> >
> > But I think the locking for that is going to be tricky, I'm not sure it does
> > actually fully work..
>
> Yes, there can be a race that sets STE.EATS back while per-master
> flag is set, which would skip the ATC_INV in commit(), so no more
> ATC_INV timeout that resets STE.EATS=0. To close it, we can force
> STE.EATS=0 at the end of commit() when state->ats_enabled and the
> per-master flag are both set, which is only possible in a race.
I don't see any of these options as appealing. We have to maintain a
few key invariants, and I think it cannot be done without a way to
find all the domains that are using the STE.
One way or another you have to be using the invs list rw locks to
synchronize the EATS state changes.
It is okayish to be sloppy when turning EATS off, but when turning it
back on we do need to cycle through every invs list and toggle its
lock to ensure that the invalidations are synchronized before
EATS=enable happens.
Given you must have a way to go from STE -> master -> all invs lists
I'm not sure either option really makes such a large difference.
If so then adjusting the invs to disable the ATS is pretty simple, run
over the xarray and set them all off. Yes you could find the master
through a SID lookup with some locking adjustment.
>
> (1) Per-invs marker: INV_TYPE_ATS_BROKEN + master_domains
> disable_ats() in the timeout path walks master->master_domains
> and flips matching ATS invs entries to the BROKEN type.
>
> + invs walker is free (one case label in the existing type switch).
> + No lock or pointer deref in the invs walker.
> + No master pointer stored in invs; no lifetime concern.
>
> - disable_ats() walks every (master, domain) and marks each invs
> set; the list needs locking usable from atomic.
This doesn't seem so bad
> (2) Per-master flag + streams_lock
> invs walker resolves SID -> master via streams_lock and reads
> master->ats_broken.
>
> + Single source of truth on the master.
> + disable_ats() is one WRITE_ONCE.
> + atc_inv_master early-skips via one READ_ONCE.
> + attach gates ats_enabled on the flag; a concurrent quarantine
> race can be closed by a short post-attach re-check in commit()
> + No master pointer in invs; no lifetime concern.
>
> - invs walker pays streams_lock + rb_find(SID) per ATS entry on
> every invalidation. Measurable on ATS-heavy workloads.
Doesn't consider how to enable
> (3) Per-master flag + inv->master pointer (v4)
> invs entry carries a master pointer; the invs walker reads
> cur->master->ats_broken directly.
>
> + invs walker is one READ_ONCE through a cached pointer.
> + disable_ats is one WRITE_ONCE.
> + atc_inv_master early-skip via one READ_ONCE.
> + attach gate + post-attach re-check, same as (2).
>
> - invs holds a master ptr, so release_device must synchronize_rcu()
> before freeing the master to drain walkers under rcu_read_lock().
> We dropped this from v4 for that reason.
synchronize_rcu is not right because you have to have gone through the
rwlock so there can be no readers.
Jason
^ permalink raw reply
* [PATCH v2 1/3] dt-bindings: arm: sunxi: Add NetCube Systems OpenNMC (dobermann)
From: Lukas Schmid @ 2026-06-05 19:13 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Maxime Ripard
Cc: Lukas Schmid, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-riscv
In-Reply-To: <20260605191322.1920944-1-lukas.schmid@netcube.li>
The OpenNMC is an open replacement for APC SmartSlot management cards
based on the Nagami System-on-Module.
Signed-off-by: Lukas Schmid <lukas.schmid@netcube.li>
---
Documentation/devicetree/bindings/arm/sunxi.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index e6443c266fa1..077b65507645 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -598,6 +598,7 @@ properties:
- description: NetCube Systems Nagami SoM based boards
items:
- enum:
+ - netcube,dobermann
- netcube,nagami-basic-carrier
- netcube,nagami-keypad-carrier
- const: netcube,nagami
--
2.47.3
^ permalink raw reply related
* [PATCH v2 3/3] ARM: dts: sunxi: add support for NetCube Systems OpenNMC (dobermann)
From: Lukas Schmid @ 2026-06-05 19:13 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Maxime Ripard
Cc: Lukas Schmid, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-riscv
In-Reply-To: <20260605191322.1920944-1-lukas.schmid@netcube.li>
NetCube Systems OpenNMC is an open replacement for APC SmartSlot Management
Cards. It is based on the Nagami System-on-Module. It breaks out the
following interfaces:
- 10/100 Mbps Ethernet
- USB Type-C OTG using a TUSB320 (usb0)
- USB Type-C Console Port using a CH340 (uart3)
- USB Type-A Host with internal CH334 USB-Hub (usb1)
- MicroSD Slot with Card-Detect (mmc0)
- WiFi/Bluetooth using the modules built-in ESP32
- SmartSlot serial interface (uart4)
- DS3232 RTC with CR1220 Battery Backup
- Extension connector providing SPI,I2C,USB,CAN,UART for future use.
Signed-off-by: Lukas Schmid <lukas.schmid@netcube.li>
---
.../sun8i-t113s-netcube-dobermann.dts | 149 ++++++++++++++++++
1 file changed, 149 insertions(+)
create mode 100644 arch/arm/boot/dts/allwinner/sun8i-t113s-netcube-dobermann.dts
diff --git a/arch/arm/boot/dts/allwinner/sun8i-t113s-netcube-dobermann.dts b/arch/arm/boot/dts/allwinner/sun8i-t113s-netcube-dobermann.dts
new file mode 100644
index 000000000000..d7765caffe2a
--- /dev/null
+++ b/arch/arm/boot/dts/allwinner/sun8i-t113s-netcube-dobermann.dts
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2026 Lukas Schmid <lukas.schmid@netcube.li>
+ */
+
+/dts-v1/;
+#include "sun8i-t113s-netcube-nagami.dtsi"
+
+#include <dt-bindings/leds/common.h>
+
+/ {
+ model = "NetCube Systems OpenNMC (dobermann)";
+ compatible = "netcube,dobermann", "netcube,nagami",
+ "allwinner,sun8i-t113s";
+
+ aliases {
+ serial2 = &uart4; // UART on SmartSlot
+ rtc0 = &ds3232;
+ rtc1 = &rtc; // not battery backed
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ led_heartbeat_green: led-heartbeat-green {
+ gpios = <&pio 6 14 GPIO_ACTIVE_HIGH>; /* PG14 */
+ linux,default-trigger = "heartbeat";
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_HEARTBEAT;
+ };
+ };
+};
+
+&ehci0 {
+ status = "okay";
+};
+
+&ehci1 {
+ status = "okay";
+};
+
+&i2c2 {
+ status = "okay";
+
+ tusb320: typec@60 {
+ compatible = "ti,tusb320";
+ reg = <0x60>;
+ interrupts-extended = <&pio 3 22 IRQ_TYPE_LEVEL_LOW>; /* PD22 */
+ };
+
+ ds3232: rtc@68 {
+ compatible = "dallas,ds3232";
+ reg = <0x68>;
+ };
+};
+
+/* microSD Card Slot on the board */
+&mmc0 {
+ vmmc-supply = <®_vcc3v3>;
+ disable-wp;
+ bus-width = <4>;
+ cd-gpios = <&pio 6 15 GPIO_ACTIVE_LOW>; /* PG15 */
+ status = "okay";
+};
+
+&ohci0 {
+ status = "okay";
+};
+
+&ohci1 {
+ status = "okay";
+};
+
+&pio {
+ gpio-line-names = "", "", "", "", // PA
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "SMART_TX", "SMART_RX", // PB
+ "EXT_IO3", "EXT_IO2", "CONSOLE_TX", "CONSOLE_RX",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "eMMC_CLK", "eMMC_CMD", // PC
+ "eMMC_D2", "eMMC_D1", "eMMC_D0", "eMMC_D3",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "", // PD
+ "", "", "", "",
+ "", "USB_SEC_EN", "EXT_SPI_nCS", "EXT_SPI_SCK",
+ "EXT_SPI_MOSI", "EXT_SPI_MISO", "EXT_IO5", "EXT_IO4",
+ "SMART_SEL", "", "", "",
+ "I2C2_SCL", "I2C2_SDA", "TUSB320_nINT", "",
+ "", "", "", "",
+ "", "", "", "",
+ "ETH_CRSDV", "ETH_RXD0", "ETH_RXD1", "ETH_TXCK", // PE
+ "ETH_TXD0", "ETH_TXD1", "ETH_TXEN", "",
+ "ETH_MDC", "ETH_MDIO", "I2C3_nINT", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "uSD_D1", "uSD_D0", "uSD_CLK", "uSD_CMD", // PF
+ "uSD_D3", "uSD_D2", "TUSB320_ID", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "ESP_CLK", "ESP_CMD", "ESP_D0", "ESP_D1", // PG
+ "ESP_D2", "ESP_D3", "ESP_TXD", "ESP_RXD",
+ "ESP_nBOOT", "ESP_nRST", "I2C3_SCL", "I2C3_SDA",
+ "EXT_IO1", "EXT_IO0", "LED_HEARTBEAT", "SD_DETECT",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "",
+ "", "", "", "";
+};
+
+/* SmartSlot serial */
+&uart4 {
+ pinctrl-0 = <&uart4_pb_pins>;
+ pinctrl-names = "default";
+ status = "okay";
+};
+
+&usb_otg {
+ extcon = <&tusb320 0>;
+ dr_mode = "otg";
+ status = "okay";
+};
+
+&usbphy {
+ usb0_id_det-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
+ status = "okay";
+};
--
2.47.3
^ permalink raw reply related
* [PATCH v2 2/3] riscv: dts: allwinner: d1s-t113: Add uart4 pinctrl required by NetCube Systems OpenNMC
From: Lukas Schmid @ 2026-06-05 19:13 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Maxime Ripard
Cc: Lukas Schmid, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-riscv
In-Reply-To: <20260605191322.1920944-1-lukas.schmid@netcube.li>
Added the "uart4_pb_pins" pinctrl used by the OpenNMC
Signed-off-by: Lukas Schmid <lukas.schmid@netcube.li>
---
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
index 82cc85acccb1..00fddedfa36f 100644
--- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
@@ -191,6 +191,12 @@ uart3_pb_pins: uart3-pb-pins {
pins = "PB6", "PB7";
function = "uart3";
};
+
+ /omit-if-no-ref/
+ uart4_pb_pins: uart4-pb-pins {
+ pins = "PB2", "PB3";
+ function = "uart4";
+ };
};
ccu: clock-controller@2001000 {
--
2.47.3
^ permalink raw reply related
* [PATCH v2 0/3] Add support for NetCube Systems OpenNMC (dobermann)
From: Lukas Schmid @ 2026-06-05 19:13 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Maxime Ripard
Cc: Lukas Schmid, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-riscv
This series adds support for the NetCube Systems OpenNMC
Changes in v2:
- fixed ordering of compatible enum
- fixed gpio line names
Signed-off-by: Lukas Schmid <lukas.schmid@netcube.li>
---
Lukas Schmid (3):
dt-bindings: arm: sunxi: Add NetCube Systems OpenNMC (dobermann)
riscv: dts: allwinner: d1s-t113: Add uart4 pinctrl required by NetCube
Systems OpenNMC
ARM: dts: sunxi: add support for NetCube Systems OpenNMC (dobermann)
.../devicetree/bindings/arm/sunxi.yaml | 1 +
.../sun8i-t113s-netcube-dobermann.dts | 149 ++++++++++++++++++
.../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 6 +
3 files changed, 156 insertions(+)
create mode 100644 arch/arm/boot/dts/allwinner/sun8i-t113s-netcube-dobermann.dts
--
2.47.3
^ permalink raw reply
* [PATCH] KVM: arm64: Fix block mapping validity check in stage-1 walker
From: Wei-Lin Chang @ 2026-06-05 18:52 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm, linux-kernel
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Wei-Lin Chang
For the 64K granule size, FEAT_LPA determines whether a level 1 mapping
is allowed. Using the result of has_52bit_pa() is too restrictive, as it
also checks the selected output addressi size in TCR.(I)PS. Fix it by
only checking FEAT_LPA.
Fixes: 5da3a3b27a01 ("KVM: arm64: Expand valid block mappings to FEAT_LPA/LPA2 support")
Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
arch/arm64/kvm/at.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 2ddb5b5a055e..30e6fa8ac07c 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -564,15 +564,18 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
/* Block mapping, check the validity of the level */
if (!(desc & BIT(1))) {
bool valid_block = false;
+ bool lpa = kvm_has_feat_enum(vcpu->kvm, ID_AA64MMFR0_EL1, PARANGE, 52);
switch (BIT(wi->pgshift)) {
case SZ_4K:
valid_block = level == 1 || level == 2 || (wi->pa52bit && level == 0);
break;
case SZ_16K:
- case SZ_64K:
valid_block = level == 2 || (wi->pa52bit && level == 1);
break;
+ case SZ_64K:
+ valid_block = level == 2 || (lpa && level == 1);
+ break;
}
if (!valid_block)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v5 5/5] watchdog: aaeon: Add watchdog driver for SRG-IMX8P MCU
From: Thomas Perrot @ 2026-06-05 18:42 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Bartosz Golaszewski, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam,
Jérémie Dautheribes, Wim Van Sebroeck, Lee Jones
Cc: thomas.perrot@bootlin.com, devicetree, linux-kernel, linux-gpio,
imx, linux-arm-kernel, linux-watchdog, Thomas Petazzoni,
Miquel Raynal
In-Reply-To: <bcc88b28-fa45-4a75-8a09-98d25a9377c9@roeck-us.net>
[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]
Hello Guenter,
On Fri, 2026-04-10 at 08:49 -0700, Guenter Roeck wrote:
> On 4/8/26 10:21, Thomas Perrot (Schneider Electric) wrote:
> > Add watchdog driver for the Aaeon SRG-IMX8P embedded controller.
> > This driver provides system monitoring and recovery capabilities
> > through the MCU's watchdog timer.
> >
> > The watchdog supports start, stop, and ping operations with a
> > maximum
> > hardware heartbeat of 25 seconds and a default timeout of 240
> > seconds.
> >
> > snip
> >
>
> Odd, unusual, unnecessary, I would argue that most people would
> consider a fixed
> timeout of 240s as anything but reasonable, and as the comment says
> arbitrary.
> Since I am sure that I pointed this out before, you still insist, and
> I am
> tired of arguing: Your funeral, so
>
I apologize for not addressing this in previous iterations.
This will be addressed in v6 to make the software timeout configurable.
The 240s value remains as the default fallback.
Kind regards,
Thomas
> Acked-by: Guenter Roeck <linux@roeck-us.net>
>
> Guenter
>
> >
> >
--
Thomas Perrot, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/3] fs/resctrl: Factor MBA parse-time conversion to be per-arch
From: Reinette Chatre @ 2026-06-05 18:43 UTC (permalink / raw)
To: Ben Horgan
Cc: james.morse, fenghuay, linux-kernel, linux-arm-kernel, tglx,
mingo, bp, dave.hansen, hpa, corbet, x86, linux-doc, dave.martin
In-Reply-To: <20260515140612.1205251-4-ben.horgan@arm.com>
Hi Ben,
On 5/15/26 7:06 AM, Ben Horgan wrote:
> From: Dave Martin <Dave.Martin@arm.com>
>
> The control value parser for the MB resource currently coerces the
> memory bandwidth percentage value from userspace to be an exact
> multiple of the rdt_resource::resctrl_membw::bw_gran parameter.
>
> On MPAM systems, this results in somewhat worse-than-worst-case
> rounding, since the bandwidth granularity advertised to resctrl by the
> MPAM driver is in general only an approximation to the actual hardware
> granularity on these systems, and the hardware bandwidth allocation
> control value is not natively a percentage -- necessitating a further
> conversion in the resctrl_arch_update_domains() path, regardless of the
> conversion done at parse time.
>
> For MPAM and x86 use their custom pre-prepared parse-time conversion,
> resctrl_arch_preconvert_bw(). This will avoid accumulated error
> from rounding the value twice on MPAM systems. For x86 systems there
> is no functional change.
>
> Clarify the documentation, but avoid overly exact promises.
>
> Clamping to bw_min and bw_max still feels generic: leave it in the core
> code, for now.
Same comment as v2: please use max line length available. Some more context here:
When resctrl patches are formatted as above the x86 maintainers end up reformatting
them if they can afford to spend the time doing so. Having changelog formatted
correctly from beginning avoids this extra churn.
You can find related comment from Boris at
https://lore.kernel.org/lkml/20250916105447.GCaMlB976WLxHHeNMD@fat_crate.local/
...
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index 9a7dfc48cb2e..934e12f5d145 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -37,8 +37,8 @@ typedef int (ctrlval_parser_t)(struct rdt_parse_data *data,
> /*
> * Check whether MBA bandwidth percentage value is correct. The value is
> * checked against the minimum and max bandwidth values specified by the
> - * hardware. The allocated bandwidth percentage is rounded to the next
> - * control step available on the hardware.
> + * hardware. The allocated bandwidth percentage is converted as
> + * appropriate for consumption by the specific hardware driver.
Same comment as v2: Adjusting right margin mid-paragraph looks awkward.
> */
> static bool bw_validate(char *buf, u32 *data, struct rdt_resource *r)
> {
> @@ -71,7 +71,7 @@ static bool bw_validate(char *buf, u32 *data, struct rdt_resource *r)
> return false;
> }
>
> - *data = roundup(bw, (unsigned long)r->membw.bw_gran);
> + *data = resctrl_arch_preconvert_bw(bw, r);
> return true;
> }
>
With line lengths adjusted (and rebased on patch #1 proposed changes):
| Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Reinette
^ permalink raw reply
* Re: [PATCH v3 2/3] arm_mpam: resctrl: Add pass-through resctrl_arch_preconvert_bw()
From: Reinette Chatre @ 2026-06-05 18:43 UTC (permalink / raw)
To: Ben Horgan
Cc: james.morse, fenghuay, linux-kernel, linux-arm-kernel, tglx,
mingo, bp, dave.hansen, hpa, corbet, x86, linux-doc, dave.martin
In-Reply-To: <20260515140612.1205251-3-ben.horgan@arm.com>
Hi Ben,
On 5/15/26 7:06 AM, Ben Horgan wrote:
> resctrl rounds up the percentage value of the MBA based on the bw_gran. As
> MPAM uses a binary fixed point fraction format for MBA rather than a
> decimal percentage, this introduces rounding errors.
>
> Without this additional rounding, if the user reads the value in an MB
> schema and then writes it back to the schema, the value in hardware won't
> change. However, with this additional rounding, this guarantee is broken
> for systems with mbw_wd < 7.
>
> resctrl is introducing resctrl_arch_preconvert_bw() to allow the arch code
> to specify the conversion resctrl does to the user-provided bandwidth
> value. Add the MPAM version of resctrl_arch_preconvert_bw(). This does no
> conversion.
>
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
If rebased on patch #1 proposed changes:
| Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Reinette
^ permalink raw reply
* Re: [PATCH v3 1/3] x86/resctrl: Add resctrl_arch_preconvert_bw()
From: Reinette Chatre @ 2026-06-05 18:42 UTC (permalink / raw)
To: Ben Horgan
Cc: james.morse, fenghuay, linux-kernel, linux-arm-kernel, tglx,
mingo, bp, dave.hansen, hpa, corbet, x86, linux-doc, dave.martin
In-Reply-To: <20260515140612.1205251-2-ben.horgan@arm.com>
Hi Ben,
Since this patch also impacts resctrl fs API in include/linux/resctrl.h the
subject prefix would more accurate as "x86,fs/resctrl: Add ..."
On 5/15/26 7:06 AM, Ben Horgan wrote:
...
> @@ -500,6 +500,25 @@ bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r);
> */
> int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable);
>
> +/**
> + * resctrl_arch_preconvert_bw() - Prepare bandwidth control value for arch use.
> + * @val: Bandwidth control value written to the schemata file by userspace.
> + * @r: Resource whose schema was written.
> + *
> + * Convert the user provided bandwidth control value to an appropriate form for
> + * consumption by the hardware driver for resource @r. Converted value is stored
> + * in rdt_ctrl_domain::staged_config[] for later consumption by
> + * resctrl_arch_update_domains(). Is not called when MBA software controller is
> + * enabled.
> + *
> + * Architectures for which this pre-conversion hook is not useful should supply
> + * an implementation of this function that just returns val unmodified.
nit: "val" -> "@val"
> + *
> + * Return:
> + * The converted value.
> + */
> +u32 resctrl_arch_preconvert_bw(u32 val, const struct rdt_resource *r);
Could you please switch the resource to be the first parameter? When comparing
this to other similar arch helpers in include/linux/resctrl.h it is custom for the resource
to be the first parameter.
> +
> /*
> * Update the ctrl_val and apply this config right now.
> * Must be called on one of the domain's CPUs.
| Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Reinette
^ permalink raw reply
* Re: [PATCH v3 0/3] x86,fs/resctrl,arm_mpam: Factor MBA parse-time conversion to be per-arch
From: Reinette Chatre @ 2026-06-05 18:41 UTC (permalink / raw)
To: Ben Horgan
Cc: james.morse, fenghuay, linux-kernel, linux-arm-kernel, tglx,
mingo, bp, dave.hansen, hpa, corbet, x86, linux-doc, dave.martin
In-Reply-To: <20260515140612.1205251-1-ben.horgan@arm.com>
Hi Ben,
On 5/15/26 7:06 AM, Ben Horgan wrote:
> This is a new version of Dave Martin's patch [1] to delegate rounding of
> bandwidth control user values to the arch code. As there is now more than one
> architecture using resctrl, I split the original patch into two, a core resctrl
> patch and an x86 patch, and added an MPAM patch. Please let me know if the patch
> break down and ordering is sensible and whether the pattern should be followed
> for any future similar changes.
This ordering is sensible to me. I find the patch breakdown a bit fragmented since
the logical resctrl fs change is split yet I also find that you did what was best
to ensure bisectability. I find small changes that are local to subsystems easier to
consider and believe it would be ideal to only have patches touching multiple
subsystems when it cannot be avoided, for example when doing otherwise would break
bisect. Even so, we may learn of better ways to do this when this series is considered
for x86 inclusion.
Reinette
^ permalink raw reply
* Re: [PATCH 2/3] iio: adc: add Axiado SARADC driver
From: Andy Shevchenko @ 2026-06-05 18:26 UTC (permalink / raw)
To: Petar Stepanovic
Cc: Akhila Kavi, Prasad Bolisetty, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Harshit Shah, linux-iio, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260528-axiado-ax3000-ax3005-saradc-v1-2-345dd5f6608a@axiado.com>
On Thu, May 28, 2026 at 01:10:24AM -0700, Petar Stepanovic wrote:
> Add support for the SARADC controller found on Axiado AX3000 and
> AX3005 SoCs.
>
> The driver supports single-shot voltage reads through the IIO
> subsystem. The number of available input channels is selected from
> the SoC match data, allowing AX3000 and AX3005 variants to use the
> same driver.
(I'll try to not duplicate what Joshua noticed already.)
...
> +config AXIADO_SARADC
> + tristate "Axiado SARADC driver"
> + depends on ARCH_AXIADO || COMPILE_TEST
> + depends on OF
No, in IIO we want a good justification on non-agnostic requirements.
Why can't this device driver be agnostic?
...
> +#include <linux/bitfield.h>
+ bits.h
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/iio/iio.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
No driver should have this header to be included.
Rare and well justified exceptions are possible
(and no, not in this case).
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
...
> +struct axiado_saradc {
> + void __iomem *regs;
> + struct clk *clk;
> + unsigned long clk_rate;
> + int vref_uv;
_uV (yes, capital letter as per SI).
> + struct mutex lock; /* Serializes ADC conversions. */
> +};
...
> +static int axiado_saradc_conversion(struct axiado_saradc *info,
> + struct iio_chan_spec const *chan, int *val)
> +{
> + unsigned long usecs;
Missing blank line here.
> + /* Select the channel to be used and trigger conversion */
> + iowrite32(AX_SARADC_MANUAL_CTRL_EN(chan->channel),
> + info->regs + AX_SARADC_MANUAL_CTRL);
Why not writel()?
> +
> + /* Hardware requires 13 conversion cycles at clk_rate */
> + usecs = DIV_ROUND_UP(AX_SARADC_CONV_CYCLES * 1000000, info->clk_rate);
USe USEC_PER_SEC from time.h.
> + usleep_range(usecs, usecs + 10);
> +
> + *val = ioread32(info->regs + AX_SARADC_DOUT) &
> + GENMASK(AX_RESOLUTION_BITS - 1, 0);
> +
> + /* Stop manual conversion */
> + iowrite32(0, info->regs + AX_SARADC_MANUAL_CTRL);
> + return 0;
> +}
...
> +static int axiado_saradc_probe(struct platform_device *pdev)
> +{
> + struct axiado_saradc *info;
> + const struct axiado_saradc_soc_data *soc_data;
> + struct iio_dev *indio_dev;
> + int ret;
> + u32 reg;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + info = iio_priv(indio_dev);
> +
> + info->regs = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(info->regs))
> + return PTR_ERR(info->regs);
> +
> + info->clk = devm_clk_get_enabled(&pdev->dev, NULL);
Why no name? It will make harder for the next generations of HW in case they
want more than one clock to be used.
> + if (IS_ERR(info->clk))
> + return PTR_ERR(info->clk);
> +
> + info->clk_rate = clk_get_rate(info->clk);
> + if (!info->clk_rate)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "invalid clock rate\n");
> + info->vref_uv = devm_regulator_get_enable_read_voltage(&pdev->dev,
> + "vref");
Having
struct device *dev = &pdev->dev;
will make the code shorter and easier to read.
> + if (info->vref_uv < 0)
> + return dev_err_probe(&pdev->dev, info->vref_uv,
> + "failed to get vref voltage\n");
> +
> + soc_data = device_get_match_data(&pdev->dev);
> + if (!soc_data)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "failed to get match data\n");
> +
> + mutex_init(&info->lock);
> + reg = FIELD_PREP(AX_SARADC_CH_EN_MASK,
> + GENMASK(soc_data->num_channels - 1, 0)) |
> + AX_SARADC_SAMPLE_16 | AX_SARADC_MODE | AX_SARADC_ENABLE;
FIELD_PREP_CONST() ?
> + iowrite32(AX_SARADC_PD, info->regs + AX_SARADC_GLOBAL_CTRL);
> + iowrite32(reg, info->regs + AX_SARADC_GLOBAL_CTRL);
> +
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->info = &axiado_saradc_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = axiado_saradc_iio_channels;
> + indio_dev->num_channels = soc_data->num_channels;
> +
> + ret = devm_iio_device_register(&pdev->dev, indio_dev);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to register IIO device\n");
> +
> + return 0;
> +}
...
> +static const struct of_device_id axiado_saradc_match[] = {
> + {
> + .compatible = "axiado,ax3000-saradc",
> + .data = &ax3000_saradc_data,
> + },
> + {
> + .compatible = "axiado,ax3005-saradc",
> + .data = &ax3005_saradc_data,
> + },
> + {},
No comma for the terminator entry.
> +};
...
> +static struct platform_driver axiado_saradc_driver = {
> + .driver = {
> + .name = KBUILD_MODNAME,
We want to have these kind of strings to be fixed.
> + .of_match_table = axiado_saradc_match,
> + },
> + .probe = axiado_saradc_probe,
> +};
> +
Unnecessary blank line.
> +module_platform_driver(axiado_saradc_driver);
--
With Best Regards,
Andy Shevchenko
^ 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