Re: [RESEND PATCH 1/3] x86: Adding structs to reflect cpuid fields

From: Paolo Bonzini <pbonzini@redhat.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Nadav Amit <namit@cs.technion.ac.il>,
	Ingo Molnar <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	the arch/x86 maintainers <x86@kernel.org>,
	kvm <kvm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [RESEND PATCH 1/3] x86: Adding structs to reflect cpuid fields
Date: Thu, 18 Sep 2014 15:36:43 +0200	[thread overview]
Message-ID: <541ADFEB.6030801@redhat.com> (raw)
In-Reply-To: <20140918132635.GB27008@nazgul.tnic>

Il 18/09/2014 15:26, Borislav Petkov ha scritto:
> On Thu, Sep 18, 2014 at 03:06:59PM +0200, Paolo Bonzini wrote:
>> The extra bit used to be reserved and thus will be zero on older
>> families.  So, nothing?
> 
> "thus will be zero" is unfortunately simply not true.
> 
> From the SDM:
> 
> "1.3.2 Reserved Bits and Software Compatibility
> 
> In many register and memory layout descriptions, certain bits are marked
> as reserved. When bits are marked as reserved, it is essential for
> compatibility with future processors that software treat these bits as
> having a future, though unknown, effect. The behavior of reserved bits
> should be regarded as not only undefined, but unpredict- able. Software
> should follow these guidelines in dealing with reserved bits:

We're talking about the case where the field is not reserved anymore and
we _know_ that the vendor has just decided to grow the bitfield that
precedes it.

As soon as we know that the field is not reserved anymore, we obviously
rely on reserved bits being zero in older processors, and in future
processors from other vendors.  The trivial example is feature bits like
XSAVE.  We query them all the time without checking the family when they
were first introduced, don't we?

> * Do not depend on the states of any reserved bits when testing the values of registers that contain such bits.
> Mask out the reserved bits before testing.

Done by the bitfields.

> *  Do not depend on the states of any reserved bits when storing to memory or to a register.

Doesn't apply to CPUID.

> * Do not depend on the ability to retain information written into any reserved bits.

Doesn't apply to CPUID.

> * When loading a register, always load the reserved bits with the values indicated in the documentation, if any,
> or reload them with values previously read from the same register."

Only applies to KVM, where it can be done with the bitfields too.

> And so on and so on.
> 
> Aaanyway, I see kvm folks are strongly determined to do this so I'll
> propose you do it for kvm only and not touch the rest of arch/x86/.
> 
> I really really disagree with casting CPUID words in stone and with
> touching code which is perfectly fine.

It's not really the KVM folks.  I actually kinda agree with you about
"touching code that works", which is why I hadn't yet commented on the
series.  But Ingo and Peter like it, so I can accept the patch as well.

> But this is just me and the final word lies with x86 people so it'll
> happen whatever they say.

Yep, same with me---KVM will just follow suit.

Paolo