From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v5 1/4] xen/libxc: Allow changes to hypervisor CPUID leaf from config file Date: Wed, 19 Mar 2014 12:02:07 +0000 Message-ID: <1395230527.10203.79.camel@kazak.uk.xensource.com> References: <1395190714-3802-1-git-send-email-boris.ostrovsky@oracle.com> <1395190714-3802-2-git-send-email-boris.ostrovsky@oracle.com> <1395221220.10203.8.camel@kazak.uk.xensource.com> <532972460200007800125962@nat28.tlf.novell.com> <1395222742.10203.29.camel@kazak.uk.xensource.com> <53297A5002000078001259DC@nat28.tlf.novell.com> <1395225598.10203.49.camel@kazak.uk.xensource.com> <532991380200007800125A97@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <532991380200007800125A97@nat28.tlf.novell.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: Jan Beulich Cc: keir@xen.org, eddie.dong@intel.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, jun.nakajima@intel.com, yang.z.zhang@intel.com, Boris Ostrovsky List-Id: xen-devel@lists.xenproject.org On Wed, 2014-03-19 at 11:44 +0000, Jan Beulich wrote: > >>> On 19.03.14 at 11:39, Ian Campbell wrote: > > On Wed, 2014-03-19 at 10:06 +0000, Jan Beulich wrote: > >> >>> On 19.03.14 at 10:52, Ian Campbell wrote: > >> > On Wed, 2014-03-19 at 09:32 +0000, Jan Beulich wrote: > >> >> >>> On 19.03.14 at 10:27, Ian Campbell wrote: > >> >> > On Tue, 2014-03-18 at 20:58 -0400, Boris Ostrovsky wrote: > >> >> >> Currently only "real" cpuid leaves can be overwritten by users via > >> >> >> 'cpuid' option in the configuration file. This patch provides ability to > >> >> >> do the same for hypervisor leaves (but for now only 0x40000000 is > > allowed). > >> >> >> > >> >> >> Signed-off-by: Boris Ostrovsky > >> >> >> --- > >> >> >> tools/libxc/xc_cpuid_x86.c | 71 > >> >> > ++++++++++++++++++++++++++++++++++++++++-- > >> >> >> xen/arch/x86/domain.c | 19 +++++++++-- > >> >> >> xen/arch/x86/traps.c | 3 ++ > >> >> >> xen/include/asm-x86/domain.h | 7 +++++ > >> >> >> 4 files changed, 95 insertions(+), 5 deletions(-) > >> >> >> > >> >> >> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c > >> >> >> index bbbf9b8..5501d5b 100644 > >> >> >> --- a/tools/libxc/xc_cpuid_x86.c > >> >> >> +++ b/tools/libxc/xc_cpuid_x86.c > >> >> >> @@ -33,6 +33,8 @@ > >> >> >> #define DEF_MAX_INTELEXT 0x80000008u > >> >> >> #define DEF_MAX_AMDEXT 0x8000001cu > >> >> >> > >> >> >> +#define IS_HYPERVISOR_LEAF(idx) (((idx) & 0xffff0000) == 0x40000000) > >> >> > > >> >> > Not idx == 0x40000000? > >> >> > > >> >> > Also as I think Jan said before if viridian support is enabled then the > >> >> > Xen leaves may be elsewhere (at 0x100 increments above that address > >> >> > IIRC). > >> >> > >> >> But that's exactly why the low 16 bits should be masked off in > >> >> above comparison. > >> > > >> > Is it 0x100 or 0x1000 increments? I thought it was 0x100, in which case > >> > shouldn't the mask be 0xfffff000? > >> > >> It's 0x100 increments, but that doesn't relate to the mask to be > >> applied here - major groups appear to be using 64k increments > >> (0000 - basic, 4000 - hypervisor, 8000 - extended, 8086 - > >> Transmeta, C000 - VIA/Cyrix, and I guess there are others I > >> don't know about). I don't think I've seen this publicly/formally > >> documented so far. > > > > OK, that makes sense from a major group perspective. > > > > But I think the "first minor group" of hypervisor nodes at 0x40000000 > > stops at 0x40010000, at least implicitly due to the existing code in e.g > > unmodified_drivers/linux-2.6/platform-pci/platform-pci.c and > > tools/misc/xen-detect.c. I don't think it is out of the question that we > > might want to put other stuff at e.g. 0x40020000 (or at least we should > > retain the option). I think I confused myself -- thinking I meant 0x40002000 here when the existing Xen leaves are to 0x40010000 not to 0x40001000 as I thought despite what I managed to write and then misinterpreted what that meant anyway. Sigh. > So that means you advocate for shrinking the number of significant > bits checked for. Question then is by how much - perhaps we should > then consider the whole range 40000000-7FFFFFFF as hypervisor > reserved? Regardless of my confusion, I don't think this is necessarily a bad idea, although it is somewhat subject to the whims of the h/w designs, meaning that in the end we should cross this bridge when we come to it rather than now. Sorry for the noise. Needed more coffee obviously. Ian.