On 16/05/18 11:49, Jan Beulich wrote: >>>> On 16.05.18 at 12:28, <andrew.coop...@citrix.com> wrote: >> 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. > The "bending the rules" comment was unrelated to alternative_*() vs > ALTERNATIVE*() use, and instead was solely related to there being a > dependency here on both pieces of replacement code being identical. > >> 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. > I don't mind consistency being achieved to other way around (i.e. by > dropping those wrappers). But I'd prefer if we didn't mix things unless > there's a compelling reason to do so.
I'll see about doing some cleanup of the overall tree for 4.12. For now, its already mixed, and this doesn't make anything worse. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
