diff mbox series

[1/3] x86/Intel: skip PLATFORM_INFO reads on family 0xf

Message ID 6f56a75c-cd68-0dad-b1ef-a3421271ee47@suse.com (mailing list archive)
State New, archived
Headers show
Series x86/Intel: family 0xF tidying | expand

Commit Message

Jan Beulich Feb. 10, 2022, 2:55 p.m. UTC
This avoids unnecessary (and always somewhat scary) log messages for the
recovered from #GP(0).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Perhaps even use "!= 6" in at least the CPUID-faulting family check?

Comments

Roger Pau Monné Feb. 11, 2022, 9:40 a.m. UTC | #1
On Thu, Feb 10, 2022 at 03:55:52PM +0100, Jan Beulich wrote:
> This avoids unnecessary (and always somewhat scary) log messages for the
> recovered from #GP(0).

Could we maybe get rid of the #GP messages for cases like this where we
are explicitly probing for MSR presence? (ie: it's expected that we
can get a #GP)

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Perhaps even use "!= 6" in at least the CPUID-faulting family check?

Likely, or else you would also need to check for family 11 (Knights
Corner?) which doesn't seem to support PLATFORM_INFO either.

Thanks, Roger.
Jan Beulich Feb. 11, 2022, 9:59 a.m. UTC | #2
On 11.02.2022 10:40, Roger Pau Monné wrote:
> On Thu, Feb 10, 2022 at 03:55:52PM +0100, Jan Beulich wrote:
>> This avoids unnecessary (and always somewhat scary) log messages for the
>> recovered from #GP(0).
> 
> Could we maybe get rid of the #GP messages for cases like this where we
> are explicitly probing for MSR presence? (ie: it's expected that we
> can get a #GP)

This would mean some form of annotation of such RDMSR attempts (for
the recovery code to recognize in order to skip the printk()). Not
all rdmsr_safe() uses are, strictly speaking, probes, so I wouldn't
want to put such in rdmsr_safe() itself.

In any event - quite a bit more work. Plus I'm not convinced it's a
good idea to suppress any such log messages.

>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Perhaps even use "!= 6" in at least the CPUID-faulting family check?
> 
> Likely, or else you would also need to check for family 11 (Knights
> Corner?) which doesn't seem to support PLATFORM_INFO either.

I don't think Xen is able to run on these (likewise for IA64, which
iirc were surfacing as x86 model 7)? These are the co-processor ones,
aren't they? My question was more towards whether we would (wrongly)
exclude future processors when using != 6, if Intel decided to ever
make new CPUs with a family other than 6.

Jan
Roger Pau Monné Feb. 11, 2022, 10:41 a.m. UTC | #3
On Fri, Feb 11, 2022 at 10:59:10AM +0100, Jan Beulich wrote:
> On 11.02.2022 10:40, Roger Pau Monné wrote:
> > On Thu, Feb 10, 2022 at 03:55:52PM +0100, Jan Beulich wrote:
> >> This avoids unnecessary (and always somewhat scary) log messages for the
> >> recovered from #GP(0).
> > 
> > Could we maybe get rid of the #GP messages for cases like this where we
> > are explicitly probing for MSR presence? (ie: it's expected that we
> > can get a #GP)
> 
> This would mean some form of annotation of such RDMSR attempts (for
> the recovery code to recognize in order to skip the printk()). Not
> all rdmsr_safe() uses are, strictly speaking, probes, so I wouldn't
> want to put such in rdmsr_safe() itself.
> 
> In any event - quite a bit more work. Plus I'm not convinced it's a
> good idea to suppress any such log messages.
> 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

> >> ---
> >> Perhaps even use "!= 6" in at least the CPUID-faulting family check?
> > 
> > Likely, or else you would also need to check for family 11 (Knights
> > Corner?) which doesn't seem to support PLATFORM_INFO either.
> 
> I don't think Xen is able to run on these (likewise for IA64, which
> iirc were surfacing as x86 model 7)? These are the co-processor ones,
> aren't they?

Right, Knights Corner uses a socket mount but it's still a
co-processor. It was Knights Landing the first one that could be used
as a host processor.

> My question was more towards whether we would (wrongly)
> exclude future processors when using != 6, if Intel decided to ever
> make new CPUs with a family other than 6.

In the case here I think we should only avoid the probe for family
0xf. Newer families (or even models on family 6 not supporting
PLATFORM_INFO) will just get a #GP message which is OK I think, we
could fix that in due time.

It's better to get a #GP message for probing than to just skip
detection of CPUID faulting on unknown newer families IMO.

Thanks, Roger.
diff mbox series

Patch

--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -127,9 +127,12 @@  bool __init probe_cpuid_faulting(void)
 
 	/*
 	 * Don't bother looking for CPUID faulting if we aren't virtualised on
-	 * AMD or Hygon hardware - it won't be present.
+	 * AMD or Hygon hardware - it won't be present.  Likewise for Fam0F
+	 * Intel hardware.
 	 */
-	if ((boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
+	if (((boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
+	     ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
+	      boot_cpu_data.x86 == 0xf)) &&
 	    !cpu_has_hypervisor)
 		return false;
 
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -455,7 +455,7 @@  static void intel_log_freq(const struct
         }
     }
 
-    if ( rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msrval) )
+    if ( c->x86 == 0xf || rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msrval) )
         return;
     max_ratio = msrval >> 8;