[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 1/6] x86: Add support for STAC/CLAC instructions
>>> On 22.04.14 at 11:43, <andrew.cooper3@xxxxxxxxxx> wrote: > On 22/04/14 09:07, Jan Beulich wrote: >>>>> On 22.04.14 at 09:41, <feng.wu@xxxxxxxxx> wrote: >>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx] >>>> That said, the macro contents itself is horrible too: A control register >>>> access and two conditional branches in code intended to be used in >>>> fast paths? Definitely not an option. Even the simplest possible >>>> solution - adding a global flag to be checked here - would already be >>>> questionable. Hence I think you should at least consider porting over >>>> proper instruction patching abstraction from Linux. >>>> >>> Jan, I did some investigation about how to handle this two instructions >>> in Linux, basically, it uses the alternatives mechanism to handle these >>> kind of cases. Let's take the following definition of ASM_STAC in Linux for >>> example: >>> >>> #define ASM_CLAC \ >>> 661: ASM_NOP3 ; \ >>> .pushsection .altinstr_replacement, "ax" ; \ >>> 662: __ASM_CLAC ; \ >>> .popsection ; \ >>> .pushsection .altinstructions, "a" ; \ >>> altinstruction_entry 661b, 662b, X86_FEATURE_SMAP, 3, 3 ; \ >>> .popsection >>> >>> ASM_CLAC is defined as NOP by default, it puts the real CLAC instruction in >>> section "altinstr_replacement" and >>> the needed information to " altinstructions " section, which is useful to >>> replace the default >>> definition by the alternative one. Here is the routine call path: >>> start_kernel () --> check_bugs() --> alternative_instructions(). >>> >>> In function alternative_instructions(), it will check the related features >>> in CPU, if it exists, the alternative definition will >>> overwrite the default one. So there is no conditional branches after this >>> replacement when the Macro is being used. >>> >>> Do you think we need to port this whole mechanism to Xen to support >>> CLAC/STAC? I am not sure if it is a little overkilled. >> Obviously we could use this machinery for other things. But whether it's >> needed here depends on the alternatives. >> >>> BTW, from the Linux implementation, I think we don't need to check the >>> 'cr4' > >>> for the macros, we just need >>> to check whether the feature exists in the CPU. So is it acceptable to use >>> the original code by eliminating the cr4 check? >> That _might_ be acceptable if you bring it down to just the three >> really necessary instructions: BT, JNC, CLAC/STAC. But the "might" >> has to stand - this, after all, remains an addition of a conditional >> branch (and for the performance of STAC/CLAC I haven't seen any >> documentation so far either) to several fast paths, and hence the >> patching alternative can't be discarded as the potentially better one. > > copy_{to,from}_guest() are already long paths (particularly for HVM) so > a single extra conditional is not going to be too bad (and as after boot > it will remain constant, the branch predictor will have a reliable time > with it). It would certainly be fine for a v1 to get SMAP support working. But that's not the paths I'm concerned about. There's going to be a CLAC at exception, interrupt, and hypercall entry points. Those are the ones I'm concerned about. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |