[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
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > Sent: Tuesday, April 22, 2014 4:07 PM > To: Wu, Feng > Cc: Ian.Campbell@xxxxxxxxxx; Dong, Eddie; Nakajima, Jun; > xen-devel@xxxxxxxxxxxxx > Subject: RE: [PATCH v1 1/6] x86: Add support for STAC/CLAC instructions > > >>> 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. > Since the alternatives mechanism in Linux is something common and independent and needs a bit more efforts to be ported to Xen, can we use the method I mentioned above at the current stage. After that I will have a fully think about how to port the alternatives mechanism Xen. What do you think about this? > Jan Thanks, Feng _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |