On 16/05/18 07:38, Jan Beulich wrote: >>>> On 15.05.18 at 21:52, <andrew.coop...@citrix.com> wrote: >> On 14/05/18 16:27, Jan Beulich wrote: >>>>>> On 11.05.18 at 12:38, <andrew.coop...@citrix.com> wrote: >>>> --- a/xen/arch/x86/spec_ctrl.c >>>> +++ b/xen/arch/x86/spec_ctrl.c >>>> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk, >>>> uint64_t caps) >>>> thunk == THUNK_RETPOLINE ? "RETPOLINE" : >>>> thunk == THUNK_LFENCE ? "LFENCE" : >>>> thunk == THUNK_JMP ? "JMP" : "?", >>>> - boot_cpu_has(X86_FEATURE_SC_MSR) ? >>>> + (boot_cpu_has(X86_FEATURE_SC_MSR_PV) || >>>> + boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ? >>>> default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" : >>>> " IBRS-" : "", >>>> opt_ibpb ? " IBPB" : "", >>>> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void) >>>> * need the IBRS entry/exit logic to virtualise IBRS support for >>>> * guests. >>>> */ >>>> - setup_force_cpu_cap(X86_FEATURE_SC_MSR); >>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV); >>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM); >>> Besides these sort of open coding alternative_io_2() (you'd really want an >>> output-less variant here, I agree) these are slightly bending the rules of >>> when/how to use multiple alternatives: The above ends up correct only >>> because of both replacements being identical. >> Actually, by reordering patch 10 ahead of this patch, we never get to >> needing the ALTERNATIVE_2()'s in the first place, and lose any concerns >> with bending the rules along the series. > Ah yes, indeed. And you would better use alternative_input() there then, > instead of open coding it.
The reason this doesn't use alternative_input() at the moment is because of the memory clobber. (And the lack of a memory clobber is called out as a peculiarity in comment). The current code looks dangerously inconsistent WRT barriers. As for bending the rules, I now disagree with your assessment. The alternative_*() wrappers do nothing but make it harder to express the parameters, as perfectly demonstrated by the ASM_OUTPUT2() bodge. I don't see their value, and they have a cost of making an asm volatile statement not look and work quite as an asm volatile statement does in all other callsites. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
