Re: [Xen-devel] [PATCH v5 1/4] xen/libxc: Allow changes to hypervisor CPUID leaf from config file

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-03-19 at 11:44 +0000, Jan Beulich wrote:
> >>> On 19.03.14 at 11:39, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Wed, 2014-03-19 at 10:06 +0000, Jan Beulich wrote:
> >> >>> On 19.03.14 at 10:52, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> >> > On Wed, 2014-03-19 at 09:32 +0000, Jan Beulich wrote:
> >> >> >>> On 19.03.14 at 10:27, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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 <boris.ostrovsky@xxxxxxxxxx>
> >> >> >> ---
> >> >> >>  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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel