On 15.03.21 07:19, Jan Kiszka via Xenomai wrote:
> On 14.03.21 18:14, Philippe Gerum wrote:
>>
>> Jan Kiszka via Xenomai <[email protected]> writes:
>>
>>> From: Jan Kiszka <[email protected]>
>>>
>>> This is only called during early init, e.g. for switching alternatives.
>>> Still, switch_mm_irqs_off would complain without this, and we are better
>>> safe than sorry.
>>>
>>
>> The way this is done in Dovetail is fragile too, since the protection we
>> have there still expects the pipeline entry code not to mess up on
>> handling an interrupt, which defeats the purpose of such
>> precaution. Besides, the temp_state should be snapshot under protection
>> too. IOW, IRQs should be hard disabled fully while using the temporary
>> mm.
>>
>> Upstreaming a similar patch for Dovetail.
>
> Just saw it: It's wrong as it left the hard_irq_save_full at the caller
> site. Would raise a warning when debugging is enabled. Please fix.
>
IOW, this should be merged into the original patch changing it:
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index cbaa584a9f23..d6c9fb7bd790 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -908,7 +908,7 @@ static void *__text_poke(void *addr, const void *opcode,
size_t len)
*/
VM_BUG_ON(!ptep);
- local_irq_save_full(flags);
+ local_irq_save(flags);
pte = mk_pte(pages[0], pgprot);
set_pte_at(poking_mm, poking_addr, ptep, pte);
@@ -959,7 +959,7 @@ static void *__text_poke(void *addr, const void *opcode,
size_t len)
*/
BUG_ON(memcmp(addr, opcode, len));
- local_irq_restore_full(flags);
+ local_irq_restore(flags);
pte_unmap_unlock(ptep, ptl);
return addr;
}
Without it, you get
[ 0.352686] WARNING: CPU: 0 PID: 1 at ../arch/x86/kernel/alternative.c:824
__text_poke+0x265/0x4a0
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
