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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.03.17 at 12:57, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 20 March 2017 11:42
>> >>> On 17.03.17 at 10:57, <paul.durrant@xxxxxxxxxx> 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?

>> 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).

>> > --- 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?

Jan


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