On 15.03.21 07:19, Jan Kiszka via Xenomai wrote:
> On 14.03.21 18:14, Philippe Gerum wrote:
>>
>> Jan Kiszka via Xenomai <xenomai@xenomai.org> writes:
>>
>>> From: Jan Kiszka <jan.kis...@siemens.com>
>>>
>>> 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

Reply via email to