Is: alternative_asm as dependency for STAC/CLAC/new features? Was:Re: [PATCH v1 1/6] x86: Add support for STAC/CLAC instructions

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Apr 23, 2014 at 01:43:35PM +0000, Wu, Feng wrote:
> 
> 
> > -----Original Message-----
> > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> > Sent: Tuesday, April 22, 2014 9:10 PM
> > To: Wu, Feng
> > Cc: Jan Beulich; Dong, Eddie; Ian.Campbell@citrix.com; Nakajima, Jun;
> > xen-devel@lists.xen.org
> > Subject: Re: [Xen-devel] [PATCH v1 1/6] x86: Add support for STAC/CLAC
> > instructions
> > 
> > On Tue, Apr 22, 2014 at 12:19:48PM +0000, Wu, Feng wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jan Beulich [mailto:JBeulich@suse.com]
> > > > Sent: Tuesday, April 22, 2014 5:17 PM
> > > > To: Wu, Feng
> > > > Cc: Ian.Campbell@citrix.com; Dong, Eddie; Nakajima, Jun;
> > > > xen-devel@lists.xen.org
> > > > Subject: RE: [PATCH v1 1/6] x86: Add support for STAC/CLAC instructions
> > > >
> > > > >>> On 22.04.14 at 10:46, <feng.wu@intel.com> wrote:
> > > > >> From: Jan Beulich [mailto:JBeulich@suse.com]
> > > > >> >>> On 22.04.14 at 09:41, <feng.wu@intel.com> wrote:
> > > > >> > 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?
> > > >
> > > > Generally this would seem acceptable (as long as you give at least a
> > > > rough estimate on when to expect that second step), but then we
> > > > have this sad experience with promises by Intel engineers to work
> > > > on certain things...
> > > >
> > >
> > > Thanks a lot!
> > > I think I can work on the alternative mechanism after this SMAP patch is
> > finished.
> > 
> > Any time estimates when the alternative patching mechanism would be done?
> > Asking
> > because it seems to me that we would want that in Xen 4.5 - so need to figure
> > out
> > your timeline to fold it in the release time-frame.
> 
> I am sorry I feel it is a little hard for me to say when the patch will be done, since I am not
> quite clear about how big the effort would be right now. But I can start porting it ASAP
> after SMAP is done. BTW, do you know when will Xen 4.5 be released? About 4~5 months later?

The 4.5 roadmap is not yet clear. I hope that at the Xen Hackathon
it will be discussed by the release manager.

I think that without this runtime patching the hypervisor will
suffer a performance penalty on hosts that don't support this.

I say *think* becuase I don't have any hard numbers. And as such
this argument might be completly wrong if testing shows otherwise
(say, running this on AMD hardware with and without these
patches).

Bearing that in mind if I this patching is not done by the time Xen 4.5
hits feature freeze window it might be neccessary to:
 - #ifdef out the code so that it only is compiled in for folks
   who really want this and are OK with the potential performance
   setback.
 - But this #ifdef is a maintaince nightmare - code often bitrots,
   another QA matrix row, etc.
   In which case reverting the code is a better option and then
   it can be targetted for Xen 4.6

Perhaps reorganizing your deliverables would be better. As in,
focus on getting alternative_asm first in, and _then_ on this feature.

Aka, alternative_asm is a dependency and this work requires
that in addition.

Or all of this pointless and testing on AMD/Intel hardware with or
without these patches (and with / without the CPU feature) shows
no performance degradation.