Philippe Gerum <[email protected]> writes:
> Jan Kiszka <[email protected]> writes: > >> On 15.03.21 10:00, Philippe Gerum wrote: >>> >>> Jan Kiszka <[email protected]> writes: >>> >>>> 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. >>>> >>> >>> 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. Confirmed (with enabling dynamic tracepoints), get_locked_pte() won't change the hard irq state. -- Philippe.
