RE: x86/vmx: Don't spuriously crash the domain when INIT is received

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Jan Beulich <jbeulich@xxxxxxxx>
> 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). 

Thanks
Kevin