linux-kernel - Re: [PATCH 06/10] HWMON: Convert coretemp to x86 cpuid autoprobing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111208143556.GC24062@one.firstfloor.org>
Date:	Thu, 8 Dec 2011 15:35:56 +0100
From:	Andi Kleen <andi@...stfloor.org>
To:	Guenter Roeck <guenter.roeck@...csson.com>
Cc:	Andi Kleen <andi@...stfloor.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kay.sievers@...y.org" <kay.sievers@...y.org>,
	"trenn@...e.de" <trenn@...e.de>, Andi Kleen <ak@...ux.intel.com>,
	"fenghua.yu@...el.com" <fenghua.yu@...el.com>,
	"khali@...ux-fr.org" <khali@...ux-fr.org>
Subject: Re: [PATCH 06/10] HWMON: Convert coretemp to x86 cpuid autoprobing

On Wed, Dec 07, 2011 at 06:40:01PM -0800, Guenter Roeck wrote:
> Andi,
> 
> On Wed, Dec 07, 2011 at 07:41:19PM -0500, Andi Kleen wrote:
> > From: Andi Kleen <ak@...ux.intel.com>
> > 
> > Use the new x86 cpuid autoprobe interface for the Intel coretemp
> > driver.
> > 
> > Cc: fenghua.yu@...el.com
> > Cc: khali@...ux-fr.org
> > Cc: guenter.roeck@...csson.com
> > Signed-off-by: Andi Kleen <ak@...ux.intel.com>
> > ---
> >  drivers/hwmon/coretemp.c |   17 ++++++++++++++---
> >  1 files changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> > index 104b376..5de1579 100644
> > --- a/drivers/hwmon/coretemp.c
> > +++ b/drivers/hwmon/coretemp.c
> > @@ -39,6 +39,7 @@
> >  #include <linux/moduleparam.h>
> >  #include <asm/msr.h>
> >  #include <asm/processor.h>
> > +#include <asm/cpu_device_id.h>
> >  
> >  #define DRVNAME	"coretemp"
> >  
> > @@ -756,13 +757,23 @@ static struct notifier_block coretemp_cpu_notifier __refdata = {
> >  	.notifier_call = coretemp_cpu_callback,
> >  };
> >  
> > +static struct x86_cpu_id coretemp_ids[] = {
> > +	{ X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_DTS },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(x86cpu, coretemp_ids);
> > +
> >  static int __init coretemp_init(void)
> >  {
> >  	int i, err = -ENODEV;
> >  
> > -	/* quick check if we run Intel */
> > -	if (cpu_data(0).x86_vendor != X86_VENDOR_INTEL)
> > -		goto exit;
> > +	/*
> > +	 * CPUID.06H.EAX[0] indicates whether the CPU has thermal
> > +	 * sensors. We check this bit only, all the early CPUs
> > +	 * without thermal sensors will be filtered out.
> > +	 */
> > +	if (!x86_match_cpu(coretemp_ids))
> > +		return -ENODEV;
> >  
> 
> X86_FEATURE_DTS is checked elsewhere in the driver, in function get_core_online().
> Can that check be removed ?

It's a bit fuzzy -- that function checks the target CPU,
while x86_match_cpu only checks the boot cpu. Now in practice 
we don't really support assymetric configurations for other reasons
anyways, but in theory it's still more correct and future proof to check 
each CPU individually. That's why I kept that other check.
So it's not really needed right now, but I would still keep it.

-Andi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/