linux-kernel - Re: [PATCH v3 03/11] platform/x86/intel/ifs: Create device for Intel IFS (In Field Scan)

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]

Date:   Wed, 20 Apr 2022 19:46:11 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "Luck, Tony" <tony.luck@...el.com>
Cc:     Dan Williams <dan.j.williams@...el.com>,
        Hans de Goede <hdegoede@...hat.com>, markgross@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        X86 ML <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
        Jonathan Corbet <corbet@....net>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "Joseph, Jithu" <jithu.joseph@...el.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        platform-driver-x86@...r.kernel.org, patches@...ts.linux.dev,
        Ravi V Shankar <ravi.v.shankar@...el.com>
Subject: Re: [PATCH v3 03/11] platform/x86/intel/ifs: Create device for Intel
 IFS (In Field Scan)

On Wed, Apr 20, 2022 at 08:27:53AM -0700, Luck, Tony wrote:
> On Wed, Apr 20, 2022 at 09:48:58AM +0200, Greg KH wrote:
> > Don't write code today for stuff you do not have right now, you all know
> > that.  We can always revisit it in the future.
> 
> Direction check on the virtual device option. Is this what
> you are asking for in "core.c"?
> 
> The second test type is happening internally right away ... so I
> put in some example code of how it can be added. Upstream submission
> will just have the one test that exists today.
> 
> Static definition of:
> 
>  static struct ifs_data ifs_data[IFS_NUMTESTS];
> 
> keeps the code simpler (no need to have code to
> cleanup if dynamic allocation of this small structure
> fails). But if you feel strongly that all static allocation
> is bad, then I can kzallloc() per enumerated test type.
> 
> With this change it is no longer a platform driver. So maybe
> doesn't belong in drivers/platform/x86/intel/ifs/*
> 
> Any thoughts on where I should move it to?
> 
> -Tony
> 
> ---- core.c ---
> 
> // SPDX-License-Identifier: GPL-2.0-only
> /* Copyright(c) 2022 Intel Corporation. */
> 
> #include <linux/module.h>
> #include <linux/device.h>
> #include <linux/kdev_t.h>
> #include <linux/semaphore.h>
> 
> #include <asm/cpu_device_id.h>
> 
> #include "ifs.h"
> 
> enum test_types {
> 	IFS_SAF,
> 	IFS_ANOTHER,
> 	IFS_NUMTESTS
> };
> 
> static struct class *ifs_class;
> static struct ifs_data ifs_data[IFS_NUMTESTS];
> 
> #define MSR_IA32_CORE_CAPS_INTEGRITY_BIT	2
> #define MSR_IA32_CORE_CAPS_INTEGRITY		BIT(MSR_IA32_CORE_CAPS_INTEGRITY_BIT)
> 
> #define X86_MATCH(model)				\
> 	X86_MATCH_VENDOR_FAM_MODEL_FEATURE(INTEL, 6,	\
> 		INTEL_FAM6_##model, X86_FEATURE_CORE_CAPABILITIES, NULL)
> 
> static const struct x86_cpu_id ifs_cpu_ids[] __initconst = {
> 	X86_MATCH(SAPPHIRERAPIDS_X),
> 	{}
> };
> MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);
> 
> static int ifs_device_unregister(struct device *dev, void *data)
> {
> 	device_unregister(dev);
> 
> 	return 0;
> }
> 
> static int __init ifs_init(void)
> {
> 	const struct x86_cpu_id *m;
> 	u64 ia32_core_caps;
> 	struct device *dev;
> 	int ndevices = 0;
> 	int ret = 0;
> 
> 	m = x86_match_cpu(ifs_cpu_ids);
> 	if (!m)
> 		return -ENODEV;
> 
> 	if (rdmsrl_safe(MSR_IA32_CORE_CAPS, &ia32_core_caps))
> 		return -ENODEV;
> 
> 	ifs_class = class_create(THIS_MODULE, "intel_ifs");

Why do you need a class?  Why not just use a misc device?  Saves you
loads of boilerplate code that is sometimes tricky to get correct.

thanks,

greg k-h