* 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.