| Message ID | 20220224194853.17774-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/vmx: Don't spuriously crash the domain when INIT is received | expand |
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? And it also cannot be that the INIT was received by the vCPU while running on another CPU: With APs not coming back up, it cannot have been scheduled to run there. And it would have been de-scheduled before suspending (i.e. before any INITs are sent). Jan
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. > 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. Xen still ignores the INIT, but now doesn't crash an entirely innocent domain as a side effect of the platform sending an INIT IPI. ~Andrew P.S. This is also fun without interrupt remapping. XSA-3 didn't imagine the full scope of problems which could occur.
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. 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. 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. 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). Jan
On Fri, Feb 25, 2022 at 02:19:39PM +0100, 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. 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. 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. A shutdown could be an alternative here, with remark that it would make debugging such issues significantly harder. Note the INIT is delivered to BSP, but the actual reason (in this case) is on some AP. Shutdown (crash) in this case would prevent (still functioning) BSP to show you the message (unless you have serial console, which is rather rare in laptops - which are significant target for Qubes OS). > 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). No, TXT wasn't (intentionally) active. I think Andrew mentioned it as explanation why VMX behaves this way: to let the OS do something _if_ TXT is active (the check for TXT is the OS responsibility here). But that's my guess only...
On 25.02.2022 14:51, Marek Marczykowski-Górecki wrote: > On Fri, Feb 25, 2022 at 02:19:39PM +0100, 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. 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. 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. > > A shutdown could be an alternative here, with remark that it would make > debugging such issues significantly harder. Note the INIT is delivered > to BSP, but the actual reason (in this case) is on some AP. Shutdown > (crash) in this case would prevent (still functioning) BSP to show you > the message (unless you have serial console, which is rather rare in > laptops - which are significant target for Qubes OS). Well, I didn't necessarily mean shutting down silently. I fully appreciate the usefulness of getting state dumped out for debugging of an issue. >> 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). > > No, TXT wasn't (intentionally) active. I think Andrew mentioned it as > explanation why VMX behaves this way: to let the OS do something _if_ TXT > is active (the check for TXT is the OS responsibility here). But that's > my guess only... One part here that I don't understand: How would the OS become aware of the INIT if it didn't try to enter a VMX guest (i.e. non- root mode)? Jan
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. > 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. ~Andrew
On Fri, Feb 25, 2022 at 12:12 PM Andrew Cooper <Andrew.Cooper3@citrix.com> wrote: > > On 25/02/2022 13:19, Jan Beulich wrote: > > 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. It's curious since the CPU, i5-1135G7, is listed as *not* supporting TXT. However, it does support Boot Guard, and both Boot Guard and TXT use Authenticated Code Modules (ACMs) to implement their functionality. There is the below quote from the Measured Launched Environment Developer’s Guide: http://kib.kiev.ua/x86docs/Intel/TXT/315168-014.pdf "Both Server TXT and Boot Guard (BtG) technologies require Startup ACM to be executed at platform reset. Intel ® CPUs can support only single such ACM and therefore combining of BtG ACM with a Startup ACM is inevitable for platforms supporting both technologies. This combining requirement triggered the whole set of upgrades targeted to better alignment of both technologies, and their mutual benefits." So I'm just speculating, but it seems there is TXT-ish stuff going on when it resumes. Regards, Jason
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. > >> 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? Jan
> From: Jan Beulich <jbeulich@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). Thanks Kevin
On 14.03.2022 07:35, Tian, Kevin wrote: >> From: Jan Beulich <jbeulich@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
> From: Jan Beulich <jbeulich@suse.com> > Sent: Monday, March 14, 2022 3:43 PM > > On 14.03.2022 07:35, Tian, Kevin wrote: > >> From: Jan Beulich <jbeulich@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. > I feel that 2) will be done in a way that the admin can choose the policy. It could be killing all VMs or in a mode where further diagnose is allowed. Given that part is unclear at this point, I'm inclined to ack 1) first: Reviewed-by: Kevin Tian <kevin.tian@intel.com> Thanks Kevin
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index c075370f642a..883213ce8f6a 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -3965,6 +3965,10 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) case EXIT_REASON_MCE_DURING_VMENTRY: do_machine_check(regs); break; + + case EXIT_REASON_INIT: + printk(XENLOG_ERR "Error: INIT received - ignoring\n"); + return; /* Renter the guest without further processing */ } /* Now enable interrupts so it's safe to take locks. */
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. Link: https://github.com/QubesOS/qubes-issues/issues/7283 Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Jun Nakajima <jun.nakajima@intel.com> CC: Kevin Tian <kevin.tian@intel.com> CC: Thiner Logoer <logoerthiner1@163.com> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>