Wolfgang Mauerer wrote: > Jan Kiszka wrote: >> Wolfgang Mauerer wrote: >>> Jan Kiszka wrote: >>>> Wolfgang Mauerer wrote: >>>>> Jan Kiszka wrote: >>>>>> If we enter __ipipe_handle_exception over a non-root domain and leave it >>>>>> due to migration in the event handler over root, we must not restore the >>>>>> root domain state so far saved on entry. This caused subtle pipeline >>>>>> state corruptions. Instead, only save and restore them if we were >>>>>> entering over root. >>>>>> >>>>>> However, the x86-32 regs.flags fixup is required nevertheless to take >>>>>> care of mismatches between the root domain state and the hardware flags >>>>>> on entry. That may happen if we fault in the iret path. But also in this >>>>>> case we must not restore an invalid root domain state. So if we entered >>>>>> over non-root, pick up the input for __fixup_if from the root domain >>>>>> after running the ipipe handler. >>>>>> >>>>>> Signed-off-by: Jan Kiszka <[email protected]> >>>>>> --- >>>>>> >>>>>> Next try. But this time I think I finally understood what scenario >>>>>> __fixup_if is actually fixing. Please correct me if I'm still missing >>>>>> one. >>>>> looks good - it works for my test cases and solves the problems with >>>>> the hw/pipeline state mismatch during early bootup. But do you happen >>>>> to have any scenario at hand with ipipe_domain_root_p && !root_entry? >>>>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags >>>>> fixup is excercised, though I guess it can't do any harm to really >>>>> ensure that the explanation fits reality this time... >>>> You mean non-root entry -> migration -> __fixup_if? In that case we pick >>>> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or >>>> what do you mean? >>> (...) >>>>>> - if (unlikely(!ipipe_root_domain_p)) { >>>>>> + if (likely(ipipe_root_domain_p)) { >>>>>> + /* >>>>>> + * 32-bit: In case we faulted in the iret path, >>>>>> regs.flags do >>>>>> + * not match the root domain state as the low-level >>>>>> return >>>>>> + * code will evaluate it. Fix this up, either by the >>>>>> root >>>>>> + * state sampled on entry or, if we migrated to root, >>>>>> with the >>>>>> + * current state. >>>>>> + */ >>>>>> + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : >>>>>> + raw_irqs_disabled(), regs); >>> I'm referring to the case that evaluates to >>> __fixup_if(raw_irqs_disabled(), regs); That is, something that >>> triggers >>> >>> if (!root_entry) >>> do_something(); >>> >>> Could be that we're talking about to the same case, although I'm not >>> sure ;-) >> Right, that's the case I described above. What problem do you precisely >> see or what concerns to you have about the suggested behavior? > > None. I'd just like to be able to trigger it to avoid that > there are any unforseen problems we're still missing. > Since this corner of Ipipe seems to have proven tricky before > AFAIK, I thought it might perhaps be worth while to really excercise > every possible code path.
To trigger it, you need to recreate the scenario that our colleagues managed to generate on the MARS: Have Linux IRQs disabled (or enabled, to have both variations), schedule in a Xenomai task, let it raise a fault so that Xenomai migrates it back. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux _______________________________________________ Adeos-main mailing list [email protected] https://mail.gna.org/listinfo/adeos-main
