Re: [PATCH 5.4] x86: ipipe: Harden path between use_temporary_mm and unuse_temporary_mm

[Philippe Gerum <rpm@xenomai.org>]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 15.03.21 10:00, Philippe Gerum wrote:
>> 
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>> 
>>> On 14.03.21 18:14, Philippe Gerum wrote:
>>>>
>>>> Jan Kiszka via Xenomai <xenomai@xenomai.org> writes:
>>>>
>>>>> From: Jan Kiszka <jan.kiszka@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.
>>>
>> 
>> I guess you mean local_irq_save_full(). After a second look, the hard
>> irqs off section should cover the entire code poking new text into a
>> live kernel. This is what switching to the _full() forms enforced
>> already. So I see no point in changing the IRQ state in the use/unuse
>> helpers eventually.
>
> Right, either way is needed. If the tlb flush should be fully protected,
> the existing pattern is needed. But maybe more:
>
> Under I-pipe, playing with full disabling didn't work out well. Some
> spinlock function called via get_locked_pte reenabled hard IRQs. Not
> sure if dovetail is affected by that as well.
>

This was likely caused by ipipe_unstall_root(void) force enabling hard
irqs on. This is gone in Dovetail, and unstalling the inband stage with
hard irqs off is even considered a bug now (inband_irq_enable()
complains about this) so that we catch the callers who would still be
making the original assumption.

IOW, allocating page table directories when locking PTEs should not
cause hard IRQs to be force enabled with Dovetail. I'm going to check
this for peace of mind.

-- 
Philippe.