[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/3] amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
On 10.03.2022 17:41, Roger Pau Monné wrote: > On Mon, Feb 14, 2022 at 05:02:52PM +0100, Jan Beulich wrote: >> On 01.02.2022 17:46, Roger Pau Monne wrote: >>> --- a/xen/arch/x86/hvm/svm/entry.S >>> +++ b/xen/arch/x86/hvm/svm/entry.S >>> @@ -71,7 +71,9 @@ __UNLIKELY_END(nsvm_hap) >>> mov %al, CPUINFO_last_spec_ctrl(%rsp) >>> 1: /* No Spectre v1 concerns. Execution will hit VMRUN >>> imminently. */ >>> .endm >>> - ALTERNATIVE "", svm_vmentry_spec_ctrl, X86_FEATURE_SC_MSR_HVM >>> + ALTERNATIVE_2 "", STR(call vmentry_virt_spec_ctrl), \ >> >> I'm afraid this violates the "ret" part of the warning a few lines up, >> while ... >> >>> + X86_FEATURE_VIRT_SC_MSR_HVM, \ >>> + svm_vmentry_spec_ctrl, X86_FEATURE_SC_MSR_HVM >>> >>> pop %r15 >>> pop %r14 >>> @@ -111,7 +113,9 @@ __UNLIKELY_END(nsvm_hap) >>> wrmsr >>> mov %al, CPUINFO_last_spec_ctrl(%rsp) >>> .endm >>> - ALTERNATIVE "", svm_vmexit_spec_ctrl, X86_FEATURE_SC_MSR_HVM >>> + ALTERNATIVE_2 "", STR(call vmexit_virt_spec_ctrl), \ >> >> ... this violates ... >> >>> + X86_FEATURE_VIRT_SC_MSR_HVM, \ >>> + svm_vmexit_spec_ctrl, X86_FEATURE_SC_MSR_HVM >>> /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */ >> >> ... the "ret" part of this warning. > > Hm, so while I could load VIRT_SPEC_CTRL easily from assembly, loading > of the legacy non-architectural setting of SSBD for Fam18h and earlier > it's likely not doable from assembly. > > Since those helpers would only set SSBD, isn't it fine to execute a > `ret` after either having set or clear SSBD? > > AFAICT the requirement would be to either have loaded SPEC_CTRL first > (if present) in the VM exit path, or to set SSBD before setting > SPEC_CTRL in the VM entry path. Yes, setting SSBD with SPEC_CTRL already / still set ought to be fine. >> Furthermore, opposite to what the change to amd_init_ssbd() suggests, >> the ordering of the alternatives here means you prefer SPEC_CTRL over >> VIRT_SPEC_CTRL; see the comment near the top of _apply_alternatives(). >> Unless I've missed logic guaranteeing that both of the keyed to >> features can't be active at the same time. > > Xen itself will only use a single one (either SPEC_CTRL.SSBD or > VIRT_SPEC_CTRL.SSBD) in order to implement support on behalf of the > guest. amd_init_ssbd already prefer to use SPEC_CTRL.SSBD over > VIRT_SPEC_CTRL.SSBD when both are available, so we aim to do the same > here. Hmm, I can't see the change to init_speculation_mitigations() guaranteeing that at most one of the two would be enabled. > I think part of the confusion steams from using info->{last_spec_ctrl, > xen_spec_ctrl} even when SPEC_CTRL MSR is not used by Xen, I need to > clarify this somehow, maybe by not using those fields in the first > place. I don't think this matters for this particular aspect of my reply. It was possibly causing some confusion to me, but elsewhere. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |