Re: [PATCH 3/7] x86/viridian: don't put Xen version information in CPUID leaf 2

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 20 March 2017 12:03
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; xen-
> devel@lists.xenproject.org
> Subject: RE: [PATCH 3/7] x86/viridian: don't put Xen version information in
> CPUID leaf 2
> 
> >>> On 20.03.17 at 12:57, <Paul.Durrant@citrix.com> wrote:
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: 20 March 2017 11:42
> >> >>> On 17.03.17 at 10:57, <paul.durrant@citrix.com> wrote:
> >> > So, clearly putting Xen hypervisor version information in that leaf is
> >> > spurious, but we probably get away with it because Xen's major version
> >> > is lower than the major version of Windows in which Hyper-V first
> >> > appeared (Server 2008).
> >> >
> >> > This patch changes the leaf to hard-code the kernel major and minor
> >> > versions from Windows Server 2008.
> >>
> >> It's not really clear to me what the version numbers are supposed
> >> to be used for: Is there any functionality availability of which can
> >> be determined solely by looking at version?
> >
> > There's not *supposed* to be, but I'm going entirely off the algorithm
> > quoted above, which implies choosing the oldest possible
> > hypervisor-main-version gives that best chance of compatibility.
> > Interestingly KVM chooses a version of 6.1 and build number of 0x1bbc.
> 
> Might that be because Windows itself considers the older version
> buggy in some respect?

No clue. Maybe it's safer to match KVM.

> 
> >> If so, wouldn't
> >> hard coding a specific Windows Server version limit ourselves? If
> >> not, does it really matter what we put there?
> >>
> >
> > Well I want to avoid a scenario where Xen's version gets bumped and
> suddenly
> > some viridian enlightenment stops working. As you say, everything is
> supposed
> > to be based on feature flags anyway, so hardcoding a version seems safest.
> 
> Sure, using Xen's version here was plain wrong. What I'm rather
> wondering is whether the returned number needs to be a little
> more dynamic (e.g. depending on other, newer features we
> support, partly - iirc - optionally).
> 

The algorithm would suggest that bumping up the hypervisor version would only be done with make it *incompatible* with older versions of Windows. E.g. if the ABI were being deliberately broken. I guess there is a chance we'd need to do that but I'd hope not.

> >> > --- a/xen/arch/x86/hvm/viridian.c
> >> > +++ b/xen/arch/x86/hvm/viridian.c
> >> > @@ -134,8 +134,8 @@ void cpuid_viridian_leaves(const struct vcpu *v,
> >> uint32_t leaf,
> >> >             own version number. */
> >> >          if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
> >> >              break;
> >> > -        res->a = 1; /* Build number */
> >> > -        res->b = (xen_major_version() << 16) | xen_minor_version();
> >> > +        res->a = 0; /* Build number */
> >>
> >> Is a build number of 0 valid at all?
> >
> > The algorithm appears to ignore it and I've not observed any problems with
> > Server 2008, Server 200R2, Windows 10 Anniversary, Server 2016 or
> Redstone2,
> > so I think it's safe. I don't mind picking the real one from a fully updated
> > Server 2008 if you'd prefer.
> 
> Or the one KVM is using, provided they have somewhere spelled
> out why it was this one they've picked?
> 

I'll see if I mine out the commit that the chose the number and look for some justification. If not then maybe going with default values from Server 2008 with command line overrides 'just in case' would be best?

  Paul

> Jan


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