Re: [PATCH v3 07/18] x86/intel_rdt: Add Haswell feature discovery

[Fenghua Yu]

On Sun, Oct 09, 2016 at 01:41:16PM +0200, Borislav Petkov wrote:
> On Fri, Oct 07, 2016 at 07:45:52PM -0700, Fenghua Yu wrote:
> > From: Fenghua Yu <fenghua.yu@xxxxxxxxx>
> > 
> > Some Haswell generation CPUs support RDT, but they don't enumerate this
> > using CPUID.  Use rdmsr_safe() and wrmsr_safe() to probe the MSRs on
> > cpu model 63 (INTEL_FAM6_HASWELL_X)
> > 
> > Signed-off-by: Fenghua Yu <fenghua.yu@xxxxxxxxx>
> > Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> > ---
> >  arch/x86/events/intel/cqm.c             |  2 +-
> >  arch/x86/include/asm/intel_rdt_common.h |  6 ++++++
> >  arch/x86/kernel/cpu/intel_rdt.c         | 38 +++++++++++++++++++++++++++++++++
> >  3 files changed, 45 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/include/asm/intel_rdt_common.h
> 
> ...
> 
> > +static inline bool cache_alloc_hsw_probe(void)
> > +{
> > +	u32 l, h_old, h_new, h_tmp;
> > +
> > +	if (rdmsr_safe(MSR_IA32_PQR_ASSOC, &l, &h_old))
> > +		return false;
> > +
> > +	/*
> > +	 * Default value is always 0 if feature is present.
> > +	 */
> > +	h_tmp = h_old ^ 0x1U;
> > +	if (wrmsr_safe(MSR_IA32_PQR_ASSOC, l, h_tmp))
> 
> I don't understand - you do the family/model check below and yet still
> use the _safe() variants. Isn't the presence of that MSR guaranteed on
> those machines?

The MSR is not guaranteed on every stepping of the family and model machine
because some parts may have the MSR fused off. And some bits in the MSR
may not be implemented on some parts. And in KVM or guest, the MSR may not
implemented. Those are reasons why we use wrmsr_safe/rdmsr_safe in Haswell
probe.

Thanks.

-Fenghua