From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH RFC 02/31] tools/libxc: Use public/featureset.h for cpuid policy generation Date: Tue, 5 Jan 2016 15:42:39 +0000 Message-ID: <568BE46F.9070906@citrix.com> References: <1450301073-28191-1-git-send-email-andrew.cooper3@citrix.com> <1450301073-28191-3-git-send-email-andrew.cooper3@citrix.com> <5679889402000078000C25CB@prv-mh.provo.novell.com> <1452003213.13361.278.camel@citrix.com> <568BD06F.2000608@citrix.com> <1452003495.13361.279.camel@citrix.com> <568BD1E4.6020409@citrix.com> <1452006171.13361.307.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1452006171.13361.307.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Jan Beulich Cc: Ian Jackson , Tim Deegan , Xen-devel List-Id: xen-devel@lists.xenproject.org On 05/01/16 15:02, Ian Campbell wrote: > On Tue, 2016-01-05 at 14:23 +0000, Andrew Cooper wrote: >> On 05/01/16 14:18, Ian Campbell wrote: >>> On Tue, 2016-01-05 at 14:17 +0000, Andrew Cooper wrote: >>>> On 05/01/16 14:13, Ian Campbell wrote: >>>>> On Tue, 2015-12-22 at 09:29 -0700, Jan Beulich wrote: >>>>>>>>> On 16.12.15 at 22:24, wrote: >>>>>>> --- a/xen/include/public/arch-x86/featureset.h >>>>>>> +++ b/xen/include/public/arch-x86/featureset.h >>>>>>> @@ -163,6 +163,7 @@ >>>>>>> >>>>>>> /* Intel-defined CPU features, CPUID level 0x00000007:0.ebx, >>>>>>> word >>>>>>> 5 */ >>>>>>> #define X86_FEATURE_FSGSBASE ( 5*32+ 0) /* >>>>>>> {RD,WR}{FS,GS}BASE >>>>>>> instructions */ >>>>>>> +#define X86_FEATURE_TSC_ADJUST ( 5*32+ 1) /* TSC_ADJUST MSR >>>>>>> available */ >>>>>> This would probably better go into patch 1. >>>>> Tools would then see this defined twice with only patch 1 applied, >>>>> and >>>>> since the value is actually different I think the compiler will >>>>> complain. >>>>> >>>>> -#define X86_FEATURE_TSC_ADJUST 1 /* Tsc thread offset */ >>>>> +#define X86_FEATURE_TSC_ADJUST ( 5*32+ 1) /* TSC_ADJUST MSR >>>>> available */ >>>>> >>>>> The comment change seems to be a semantic one? Or was it wrong >>>>> beofre? >>>> Changing patch 1 won't affect the compilation of libxc. Observe in >>>> the >>>> penultimate hunk that I also change the #include >>> Ah yes. >>> >>> What about my comment on the comment changing? >> Ah yes - the changes are just semantic. Also observe that the same hunk >> also modifies the libxc macros to modulo 32. > I'm not talking about the value at all, since I realised that the masking > makes the values the same. I'm talking about the /* comment */ next to it > which has also changed. > >> In the end, both the hypervisor and libxc are dealing with >> hardware-specified bits in registers. This patch is no resulting change >> to behaviour. > But do "Tsc thread offset" and "TSC_ADJUST MSR available" mean the same > thing? Or was "Tsc thread offset" previously actively wrong or misleading > in some way? The latter is the accurate meaning of the bit in cpuid. The former is misleading IMO, and covers one use for it in a Linux world. ~Andrew