On 29/08/18 07:26, Jan Beulich wrote: >>>> On 29.08.18 at 01:06, <andrew.coop...@citrix.com> wrote: >> On 15/08/18 07:09, Jan Beulich wrote: >>> @@ -96,13 +101,12 @@ __UNLIKELY_END(nsvm_hap) >>> SPEC_CTRL_ENTRY_FROM_HVM /* Req: b=curr %rsp=regs/cpuinfo, >>> Clob: acd */ >>> /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */ >>> >>> - STGI >>> -GLOBAL(svm_stgi_label) >>> + sti >> Nack. As indicated in v1, moving this breaks SPEC_CTRL_ENTRY_FROM_HVM >> (Even if there is an unexpected bug on the VT-x side of things which >> needs fixing differently). > Well, this increases the pressure to fix the issue. I very much > expect that fix to also take care of the code here. It was > therefore intentional that I did only the technically necessary > adjustments in v2. > > I would, btw, likely have tried to find time to look into fixing > that issue, but so far I was under the impression that you are > planning to, and since you've written the code I thought you'd > likely also be in a better position to do so.
Its somewhere in the massive pile of still-security work that needs doing. It is not the most urgent of the still-security work to do, partly because this protection is currently in place. Top of the list, and most important, is still the toolstack CPUID/MSR interactions so we can feasibly tell a guest what it needs to know WRT L1TF. > >> Furthermore, to fix LBR handling, the first thing I'd have to do is >> revert this, so please leave it as it is. > Mind being a little more specific as to the whys here? When vmcb->virt_ext.fields.lbr_enable is clear and DEBUGCTL.LBR is set, the hypervisor must atomically switch the 5 LBR MSRs inside the critical region. Usage of DEBUGCTL.LBR is broken on pre-lbrv hardware (gen 1/2 svm?), and when suitably (mis)configured by nested virt. Other bugs include Xen's LBR setting leaking into guests. > From a purely formal pov, Boris'es R-b allows me to put the change in > as is. I'm not overly happy to do the requested change, but I > certainly will, provided - once again - I understand the reason. > > Furthermore, please don't forget that the more we delay > especially #MC, the more likely it becomes that a system will > crash in a far less obvious way than by logging an #MC. #MC's only get raised after the bank registers have been updated, so even if a shutdown occurs, the data will be captured by the firmware after reset. As for non-deferrable #MC's, a couple of extra stack/msr/vcpu references in the critical region doesn't alter the chances. We were never going to survive, or won't trigger a new one. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
