* local_irq_disable_full() in x86's __bad_area_nosemaphore @ 2026-11-11 12:58 Jan Kiszka 2026-11-11 22:57 ` Florian Bezdeka 0 siblings, 1 reply; 6+ messages in thread From: Jan Kiszka @ 2026-11-11 12:58 UTC (permalink / raw) To: Philippe Gerum, Florian Bezdeka; +Cc: Xenomai Hi Philippe, hi Florian, I start to love pattern-matching machines: Since 6.19-dovetail, commit 6af591980d1750b29474f001902c7dc58c3f1ba2, we have diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index b83a06739b511..024a67bd2de6a 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -821,6 +821,8 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, force_sig_pkuerr((void __user *)address, pkey); else force_sig_fault(SIGSEGV, si_code, (void __user *)address); + + local_irq_disable_full(); } static noinline void The final code is ... oob_trap_notify(X86_TRAP_PF, regs); if (!running_inband()) goto out; if (likely(show_unhandled_signals)) show_signal_msg(regs, error_code, address, tsk); set_signal_archinfo(address, error_code); if (si_code == SEGV_PKUERR) force_sig_pkuerr((void __user *)address, pkey); else force_sig_fault(SIGSEGV, si_code, (void __user *)address); local_irq_disable_full(); out: oob_trap_unwind(X86_TRAP_PF, regs); } Do you remember why? As this was folded into the forward port, this delta never made it into anything older. If it is valid, we would need some reasoning. But this rather looks like a mismerge to me. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: local_irq_disable_full() in x86's __bad_area_nosemaphore 2026-11-11 12:58 local_irq_disable_full() in x86's __bad_area_nosemaphore Jan Kiszka @ 2026-11-11 22:57 ` Florian Bezdeka 2026-11-12 6:58 ` Jan Kiszka 0 siblings, 1 reply; 6+ messages in thread From: Florian Bezdeka @ 2026-11-11 22:57 UTC (permalink / raw) To: Jan Kiszka, Philippe Gerum; +Cc: Xenomai On Wed, 2026-11-11 at 13:58 +0100, Jan Kiszka wrote: > Hi Philippe, hi Florian, > > I start to love pattern-matching machines: > > Since 6.19-dovetail, commit 6af591980d1750b29474f001902c7dc58c3f1ba2, we have > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > index b83a06739b511..024a67bd2de6a 100644 > --- a/arch/x86/mm/fault.c > +++ b/arch/x86/mm/fault.c > @@ -821,6 +821,8 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, > force_sig_pkuerr((void __user *)address, pkey); > else > force_sig_fault(SIGSEGV, si_code, (void __user *)address); > + > + local_irq_disable_full(); > } > > static noinline void > > The final code is > > ... > oob_trap_notify(X86_TRAP_PF, regs); > if (!running_inband()) > goto out; > > if (likely(show_unhandled_signals)) > show_signal_msg(regs, error_code, address, tsk); > > set_signal_archinfo(address, error_code); > > if (si_code == SEGV_PKUERR) > force_sig_pkuerr((void __user *)address, pkey); > else > force_sig_fault(SIGSEGV, si_code, (void __user *)address); > > local_irq_disable_full(); > out: > oob_trap_unwind(X86_TRAP_PF, regs); > } > > Do you remember why? As this was folded into the forward port, this > delta never made it into anything older. If it is valid, we would need > some reasoning. But this rather looks like a mismerge to me. This one needs some more cycles here. It might already start with the call to cond_reenable_irqs_user() earlier in this function which is unconditionally enabling hard IRQs, even if the oob stage might be stalled. The possible in-band migration happens later. If we leave with hard IRQs enabled, we would corrupt the oob IRQ state. I need to check the calling context in more detail. Maybe this cannot happen, maybe it can... Touching the inband IRQ mask (one part of disable_full()) seems strange to me - at this stage - especially as Linux does not do any kind of "restore" here. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: local_irq_disable_full() in x86's __bad_area_nosemaphore 2026-11-11 22:57 ` Florian Bezdeka @ 2026-11-12 6:58 ` Jan Kiszka 2026-11-12 9:03 ` Philippe Gerum 0 siblings, 1 reply; 6+ messages in thread From: Jan Kiszka @ 2026-11-12 6:58 UTC (permalink / raw) To: Florian Bezdeka, Philippe Gerum; +Cc: Xenomai On 11.11.26 23:57, Florian Bezdeka wrote: > On Wed, 2026-11-11 at 13:58 +0100, Jan Kiszka wrote: >> Hi Philippe, hi Florian, >> >> I start to love pattern-matching machines: >> >> Since 6.19-dovetail, commit 6af591980d1750b29474f001902c7dc58c3f1ba2, we have >> >> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c >> index b83a06739b511..024a67bd2de6a 100644 >> --- a/arch/x86/mm/fault.c >> +++ b/arch/x86/mm/fault.c >> @@ -821,6 +821,8 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, >> force_sig_pkuerr((void __user *)address, pkey); >> else >> force_sig_fault(SIGSEGV, si_code, (void __user *)address); >> + >> + local_irq_disable_full(); >> } >> >> static noinline void >> >> The final code is >> >> ... >> oob_trap_notify(X86_TRAP_PF, regs); >> if (!running_inband()) >> goto out; >> >> if (likely(show_unhandled_signals)) >> show_signal_msg(regs, error_code, address, tsk); >> >> set_signal_archinfo(address, error_code); >> >> if (si_code == SEGV_PKUERR) >> force_sig_pkuerr((void __user *)address, pkey); >> else >> force_sig_fault(SIGSEGV, si_code, (void __user *)address); >> >> local_irq_disable_full(); >> out: >> oob_trap_unwind(X86_TRAP_PF, regs); >> } >> >> Do you remember why? As this was folded into the forward port, this >> delta never made it into anything older. If it is valid, we would need >> some reasoning. But this rather looks like a mismerge to me. > > This one needs some more cycles here. It might already start with the > call to cond_reenable_irqs_user() earlier in this function which is > unconditionally enabling hard IRQs, even if the oob stage might be > stalled. The possible in-band migration happens later. > > If we leave with hard IRQs enabled, we would corrupt the oob IRQ state. > I need to check the calling context in more detail. Maybe this cannot > happen, maybe it can... > The oracle says: "No. With a working companion core, the oob stage can't be stalled when this code runs, and the handler never returns with hard IRQs still enabled." The point is that the cond_reenable_irqs_user is only run for faults triggered over userspace, and there we should never have OOB stalled / hard IRQs off. Kernel faults are filtered early in the function ("if (!user_mode(regs))"). > Touching the inband IRQ mask (one part of disable_full()) seems strange > to me - at this stage - especially as Linux does not do any kind of > "restore" here. This is why I suspect a mismerge during forward porting. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: local_irq_disable_full() in x86's __bad_area_nosemaphore 2026-11-12 6:58 ` Jan Kiszka @ 2026-11-12 9:03 ` Philippe Gerum 2026-11-12 9:40 ` Jan Kiszka 0 siblings, 1 reply; 6+ messages in thread From: Philippe Gerum @ 2026-11-12 9:03 UTC (permalink / raw) To: Jan Kiszka; +Cc: Florian Bezdeka, Xenomai Jan Kiszka <jan.kiszka@siemens.com> writes: > On 11.11.26 23:57, Florian Bezdeka wrote: >> On Wed, 2026-11-11 at 13:58 +0100, Jan Kiszka wrote: >>> Hi Philippe, hi Florian, >>> >>> I start to love pattern-matching machines: >>> >>> Since 6.19-dovetail, commit 6af591980d1750b29474f001902c7dc58c3f1ba2, we have >>> >>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c >>> index b83a06739b511..024a67bd2de6a 100644 >>> --- a/arch/x86/mm/fault.c >>> +++ b/arch/x86/mm/fault.c >>> @@ -821,6 +821,8 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, >>> force_sig_pkuerr((void __user *)address, pkey); >>> else >>> force_sig_fault(SIGSEGV, si_code, (void __user *)address); >>> + >>> + local_irq_disable_full(); >>> } >>> >>> static noinline void >>> >>> The final code is >>> >>> ... >>> oob_trap_notify(X86_TRAP_PF, regs); >>> if (!running_inband()) >>> goto out; >>> >>> if (likely(show_unhandled_signals)) >>> show_signal_msg(regs, error_code, address, tsk); >>> >>> set_signal_archinfo(address, error_code); >>> >>> if (si_code == SEGV_PKUERR) >>> force_sig_pkuerr((void __user *)address, pkey); >>> else >>> force_sig_fault(SIGSEGV, si_code, (void __user *)address); >>> >>> local_irq_disable_full(); >>> out: >>> oob_trap_unwind(X86_TRAP_PF, regs); >>> } >>> >>> Do you remember why? As this was folded into the forward port, this >>> delta never made it into anything older. If it is valid, we would need >>> some reasoning. But this rather looks like a mismerge to me. >> >> This one needs some more cycles here. It might already start with the >> call to cond_reenable_irqs_user() earlier in this function which is >> unconditionally enabling hard IRQs, even if the oob stage might be >> stalled. The possible in-band migration happens later. >> >> If we leave with hard IRQs enabled, we would corrupt the oob IRQ state. >> I need to check the calling context in more detail. Maybe this cannot >> happen, maybe it can... >> > > The oracle says: > > "No. With a working companion core, the oob stage can't be stalled when > this code runs, and the handler never returns with hard IRQs still enabled." > > The point is that the cond_reenable_irqs_user is only run for faults > triggered over userspace, and there we should never have OOB stalled / > hard IRQs off. Kernel faults are filtered early in the function > ("if (!user_mode(regs))"). > >> Touching the inband IRQ mask (one part of disable_full()) seems strange >> to me - at this stage - especially as Linux does not do any kind of >> "restore" here. > > This is why I suspect a mismerge during forward porting. > Although this might be the case due to the significant code churn in this area which is even worse on arm, on x86 and arm64 for that matter, the whole logic is driven by the generic irqentry/irqexit scheme (irq-entry-common.h). In short, the exit path expects the in-band stage to be stalled if running in-band, hard irqs off. There are lockdep assertions to enforce this, and specific provisions to unstall the in-band stage as appropriate right before unwinding to the asm entry point, e.g. exit_to_user_mode(). -- Philippe. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: local_irq_disable_full() in x86's __bad_area_nosemaphore 2026-11-12 9:03 ` Philippe Gerum @ 2026-11-12 9:40 ` Jan Kiszka 2026-11-12 13:28 ` Philippe Gerum 0 siblings, 1 reply; 6+ messages in thread From: Jan Kiszka @ 2026-11-12 9:40 UTC (permalink / raw) To: Philippe Gerum; +Cc: Florian Bezdeka, Xenomai On 12.11.26 10:03, Philippe Gerum wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> On 11.11.26 23:57, Florian Bezdeka wrote: >>> On Wed, 2026-11-11 at 13:58 +0100, Jan Kiszka wrote: >>>> Hi Philippe, hi Florian, >>>> >>>> I start to love pattern-matching machines: >>>> >>>> Since 6.19-dovetail, commit 6af591980d1750b29474f001902c7dc58c3f1ba2, we have >>>> >>>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c >>>> index b83a06739b511..024a67bd2de6a 100644 >>>> --- a/arch/x86/mm/fault.c >>>> +++ b/arch/x86/mm/fault.c >>>> @@ -821,6 +821,8 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, >>>> force_sig_pkuerr((void __user *)address, pkey); >>>> else >>>> force_sig_fault(SIGSEGV, si_code, (void __user *)address); >>>> + >>>> + local_irq_disable_full(); >>>> } >>>> >>>> static noinline void >>>> >>>> The final code is >>>> >>>> ... >>>> oob_trap_notify(X86_TRAP_PF, regs); >>>> if (!running_inband()) >>>> goto out; >>>> >>>> if (likely(show_unhandled_signals)) >>>> show_signal_msg(regs, error_code, address, tsk); >>>> >>>> set_signal_archinfo(address, error_code); >>>> >>>> if (si_code == SEGV_PKUERR) >>>> force_sig_pkuerr((void __user *)address, pkey); >>>> else >>>> force_sig_fault(SIGSEGV, si_code, (void __user *)address); >>>> >>>> local_irq_disable_full(); >>>> out: >>>> oob_trap_unwind(X86_TRAP_PF, regs); >>>> } >>>> >>>> Do you remember why? As this was folded into the forward port, this >>>> delta never made it into anything older. If it is valid, we would need >>>> some reasoning. But this rather looks like a mismerge to me. >>> >>> This one needs some more cycles here. It might already start with the >>> call to cond_reenable_irqs_user() earlier in this function which is >>> unconditionally enabling hard IRQs, even if the oob stage might be >>> stalled. The possible in-band migration happens later. >>> >>> If we leave with hard IRQs enabled, we would corrupt the oob IRQ state. >>> I need to check the calling context in more detail. Maybe this cannot >>> happen, maybe it can... >>> >> >> The oracle says: >> >> "No. With a working companion core, the oob stage can't be stalled when >> this code runs, and the handler never returns with hard IRQs still enabled." >> >> The point is that the cond_reenable_irqs_user is only run for faults >> triggered over userspace, and there we should never have OOB stalled / >> hard IRQs off. Kernel faults are filtered early in the function >> ("if (!user_mode(regs))"). >> >>> Touching the inband IRQ mask (one part of disable_full()) seems strange >>> to me - at this stage - especially as Linux does not do any kind of >>> "restore" here. >> >> This is why I suspect a mismerge during forward porting. >> > > Although this might be the case due to the significant code churn in > this area which is even worse on arm, on x86 and arm64 for that matter, > the whole logic is driven by the generic irqentry/irqexit scheme > (irq-entry-common.h). In short, the exit path expects the in-band stage > to be stalled if running in-band, hard irqs off. There are lockdep > assertions to enforce this, and specific provisions to unstall the > in-band stage as appropriate right before unwinding to the asm entry > point, e.g. exit_to_user_mode(). > This does not explain what makes things different here, both with and also without IRQ_PIPELINE. The more I look at it, the more that local_irq_disable_full() looks stray, at best. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: local_irq_disable_full() in x86's __bad_area_nosemaphore 2026-11-12 9:40 ` Jan Kiszka @ 2026-11-12 13:28 ` Philippe Gerum 0 siblings, 0 replies; 6+ messages in thread From: Philippe Gerum @ 2026-11-12 13:28 UTC (permalink / raw) To: Jan Kiszka; +Cc: Florian Bezdeka, Xenomai Jan Kiszka <jan.kiszka@siemens.com> writes: > On 12.11.26 10:03, Philippe Gerum wrote: >> Jan Kiszka <jan.kiszka@siemens.com> writes: >> >>> On 11.11.26 23:57, Florian Bezdeka wrote: >>>> On Wed, 2026-11-11 at 13:58 +0100, Jan Kiszka wrote: >>>>> Hi Philippe, hi Florian, >>>>> >>>>> I start to love pattern-matching machines: >>>>> >>>>> Since 6.19-dovetail, commit 6af591980d1750b29474f001902c7dc58c3f1ba2, we have >>>>> >>>>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c >>>>> index b83a06739b511..024a67bd2de6a 100644 >>>>> --- a/arch/x86/mm/fault.c >>>>> +++ b/arch/x86/mm/fault.c >>>>> @@ -821,6 +821,8 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, >>>>> force_sig_pkuerr((void __user *)address, pkey); >>>>> else >>>>> force_sig_fault(SIGSEGV, si_code, (void __user *)address); >>>>> + >>>>> + local_irq_disable_full(); >>>>> } >>>>> >>>>> static noinline void >>>>> >>>>> The final code is >>>>> >>>>> ... >>>>> oob_trap_notify(X86_TRAP_PF, regs); >>>>> if (!running_inband()) >>>>> goto out; >>>>> >>>>> if (likely(show_unhandled_signals)) >>>>> show_signal_msg(regs, error_code, address, tsk); >>>>> >>>>> set_signal_archinfo(address, error_code); >>>>> >>>>> if (si_code == SEGV_PKUERR) >>>>> force_sig_pkuerr((void __user *)address, pkey); >>>>> else >>>>> force_sig_fault(SIGSEGV, si_code, (void __user *)address); >>>>> >>>>> local_irq_disable_full(); >>>>> out: >>>>> oob_trap_unwind(X86_TRAP_PF, regs); >>>>> } >>>>> >>>>> Do you remember why? As this was folded into the forward port, this >>>>> delta never made it into anything older. If it is valid, we would need >>>>> some reasoning. But this rather looks like a mismerge to me. >>>> >>>> This one needs some more cycles here. It might already start with the >>>> call to cond_reenable_irqs_user() earlier in this function which is >>>> unconditionally enabling hard IRQs, even if the oob stage might be >>>> stalled. The possible in-band migration happens later. >>>> >>>> If we leave with hard IRQs enabled, we would corrupt the oob IRQ state. >>>> I need to check the calling context in more detail. Maybe this cannot >>>> happen, maybe it can... >>>> >>> >>> The oracle says: >>> >>> "No. With a working companion core, the oob stage can't be stalled when >>> this code runs, and the handler never returns with hard IRQs still enabled." >>> >>> The point is that the cond_reenable_irqs_user is only run for faults >>> triggered over userspace, and there we should never have OOB stalled / >>> hard IRQs off. Kernel faults are filtered early in the function >>> ("if (!user_mode(regs))"). >>> >>>> Touching the inband IRQ mask (one part of disable_full()) seems strange >>>> to me - at this stage - especially as Linux does not do any kind of >>>> "restore" here. >>> >>> This is why I suspect a mismerge during forward porting. >>> >> >> Although this might be the case due to the significant code churn in >> this area which is even worse on arm, on x86 and arm64 for that matter, >> the whole logic is driven by the generic irqentry/irqexit scheme >> (irq-entry-common.h). In short, the exit path expects the in-band stage >> to be stalled if running in-band, hard irqs off. There are lockdep >> assertions to enforce this, and specific provisions to unstall the >> in-band stage as appropriate right before unwinding to the asm entry >> point, e.g. exit_to_user_mode(). >> > > This does not explain what makes things different here, both with and > also without IRQ_PIPELINE. > Affecting !IRQ_PIPELINE is obviously a bug. > The more I look at it, the more that local_irq_disable_full() looks > stray, at best. > So, I asked git-blame which told me that 6af591980d1750 introduced it. As far as I can tell, this is neither a merge fallout nor stray stuff, but rather a misinterpretation of the local_irq_enable() statement which existed prior the introduction of cond_reenable_irqs_user() at that time. This re-enabling upstream wants was deemed to be persistent, and I made it temporary for the duration of the call, which is wrong. Although this change is going to be properly undone in the irqexit path I mentioned earlier, it is useless. It should not break the logic either, since anything we must do with the in-band stage unstalled is already done passed that point. So I'm ok for dropping that line. Meanwhile, the assertion I mentioned in my comment of the fixup becomes: WARN_ON_ONCE(irqs_pipelined() && irqs_disabled()) -- Philippe. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-12 13:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-11 12:58 local_irq_disable_full() in x86's __bad_area_nosemaphore Jan Kiszka 2026-11-11 22:57 ` Florian Bezdeka 2026-11-12 6:58 ` Jan Kiszka 2026-11-12 9:03 ` Philippe Gerum 2026-11-12 9:40 ` Jan Kiszka 2026-11-12 13:28 ` Philippe Gerum
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.