On 14.03.2022 07:35, Tian, Kevin wrote: >> From: Jan Beulich <jbeul...@suse.com> >> Sent: Monday, February 28, 2022 3:36 PM >> >> On 25.02.2022 18:11, Andrew Cooper wrote: >>> On 25/02/2022 13:19, Jan Beulich wrote: >>>> On 25.02.2022 13:28, Andrew Cooper wrote: >>>>> On 25/02/2022 08:44, Jan Beulich wrote: >>>>>> On 24.02.2022 20:48, Andrew Cooper wrote: >>>>>>> In VMX operation, the handling of INIT IPIs is changed. >> EXIT_REASON_INIT has >>>>>>> nothing to do with the guest in question, simply signals that an INIT >> was >>>>>>> received. >>>>>>> >>>>>>> Ignoring the INIT is probably the wrong thing to do, but is helpful for >>>>>>> debugging. Crashing the domain which happens to be in context is >> definitely >>>>>>> wrong. Print an error message and continue. >>>>>>> >>>>>>> Discovered as collateral damage from when an AP triple faults on S3 >> resume on >>>>>>> Intel TigerLake platforms. >>>>>> I'm afraid I don't follow the scenario, which was (only) outlined in >>>>>> patch 1: Why would the BSP receive INIT in this case? >>>>> SHUTDOWN is a signal emitted by a core when it can't continue. Triple >>>>> fault is one cause, but other sources include a double #MC, etc. >>>>> >>>>> Some external component, in the PCH I expect, needs to turn this into a >>>>> platform reset, because one malfunctioning core can't. It is why a >>>>> triple fault on any logical processor brings the whole system down. >>>> I'm afraid this doesn't answer my question. Clearly the system didn't >>>> shut down. >>> >>> Indeed, *because* Xen caught and ignored the INIT which was otherwise >>> supposed to do it. >>> >>>> Hence I still don't see why the BSP would see INIT in the >>>> first place. >>>> >>>>>> And it also cannot be that the INIT was received by the vCPU while >> running on >>>>>> another CPU: >>>>> It's nothing (really) to do with the vCPU. INIT is a external signal to >>>>> the (real) APIC, just like NMI/etc. >>>>> >>>>> It is the next VMEntry on a CPU which received INIT that suffers a >>>>> VMEntry failure, and the VMEntry failure has nothing to do with the >>>>> contents of the VMCS. >>>>> >>>>> Importantly for Xen however, this isn't applicable for scheduling PV >>>>> vCPUs, which is why dom0 wasn't the one that crashed. This actually >>>>> meant that dom0 was alive an usable, albeit it sharing all vCPUs on a >>>>> single CPU. >>>>> >>>>> >>>>> The change in INIT behaviour exists for TXT, where is it critical that >>>>> software can clear secrets from RAM before resetting. I'm not wanting >>>>> to get into any of that because it's far more complicated than I have >>>>> time to fix. >>>> I guess there's something hidden behind what you say here, like INIT >>>> only being latched, but this latched state then causing the VM entry >>>> failure. Which would mean that really the INIT was a signal for the >>>> system to shut down / shutting down. >>> >>> Yes. > > why is INIT latched in root mode (take effect until vmentry) instead of > directly causing the CPU to reset? > >>> >>>> In which case arranging to >>>> continue by ignoring the event in VMX looks wrong. Simply crashing >>>> the guest would then be wrong as well, of course. We should shut >>>> down instead. >>> >>> It is software's discretion what to do when an INIT is caught, even if >>> the expectation is to honour it fairly promptly. >>> >>>> But I don't think I see the full picture here yet, unless your >>>> mentioning of TXT was actually implying that TXT was active at the >>>> point of the crash (which I don't think was said anywhere). >>> >>> This did cause confusion during debugging. As far as we can tell, TXT >>> is not active, but the observed behaviour certainly looks like TXT is >>> active. >>> >>> Then again, reset is a platform behaviour, not architectural. Also, >>> it's my understanding that Intel does not support S3 on TigerLake >>> (opting to only support S0ix instead), so I'm guessing that "Linux S3" >>> as it's called in the menu is something retrofitted by the OEM. >>> >>> But overall, the point isn't really about what triggered the INIT. We >>> also shouldn't nuke an innocent VM if an INIT IPI slips through >>> interrupt remapping. >> >> But we also shouldn't continue in such a case as if nothing had happened >> at all, should we? >> > > Now there are two problems: > > 1) An innocent VM is killed; > 2) The system continues as if nothing had happened; > > Andrew's patch fixes 1) which imo is welcomed anyway. > > 2) certainly needs more work but could come after 1).
That's one way to look at things, sure, and if you agree it makes sense to address 1), I won't go as far as trying to block such a change. But it feels wrong to me - 2) working properly really includes 1) plus the killing of all other innocent VMs that were running at the time. Jan
