| 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
| ||
|
Date: Tue, 19 Apr 2022 19:20:54 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Tony Luck <tony.luck@...el.com>
Cc: hdegoede@...hat.com, markgross@...nel.org, tglx@...utronix.de,
mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
x86@...nel.org, hpa@...or.com, corbet@....net,
andriy.shevchenko@...ux.intel.com, jithu.joseph@...el.com,
ashok.raj@...el.com, rostedt@...dmis.org, dan.j.williams@...el.com,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
platform-driver-x86@...r.kernel.org, patches@...ts.linux.dev,
ravi.v.shankar@...el.com
Subject: Re: [PATCH v3 09/11] platform/x86/intel/ifs: Add IFS sysfs interface
On Tue, Apr 19, 2022 at 09:38:57AM -0700, Tony Luck wrote:
> From: Jithu Joseph <jithu.joseph@...el.com>
>
> Implement sysfs interface to trigger ifs test for a specific cpu.
> Additional interfaces related to checking the status of the
> scan test and seeing the version of the loaded IFS binary
> are also added.
>
> The basic usage is as below.
> - To start test, for example on cpu5:
> echo 5 > /sys/devices/platform/intel_ifs/run_test
> - To see the status of the last test
> cat /sys/devices/platform/intel_ifs/status
> - To see the version of the loaded scan binary
> cat /sys/devices/platform/intel_ifs/image_version
>
> Reviewed-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@...el.com>
> Co-developed-by: Tony Luck <tony.luck@...el.com>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
> drivers/platform/x86/intel/ifs/Makefile | 2 +-
> drivers/platform/x86/intel/ifs/core.c | 9 ++
> drivers/platform/x86/intel/ifs/ifs.h | 3 +
> drivers/platform/x86/intel/ifs/runtest.c | 7 ++
> drivers/platform/x86/intel/ifs/sysfs.c | 151 +++++++++++++++++++++++
> 5 files changed, 171 insertions(+), 1 deletion(-)
> create mode 100644 drivers/platform/x86/intel/ifs/sysfs.c
>
> diff --git a/drivers/platform/x86/intel/ifs/Makefile b/drivers/platform/x86/intel/ifs/Makefile
> index 7de27361b479..dbbe0bf66987 100644
> --- a/drivers/platform/x86/intel/ifs/Makefile
> +++ b/drivers/platform/x86/intel/ifs/Makefile
> @@ -2,4 +2,4 @@ obj-$(CONFIG_INTEL_IFS_DEVICE) += intel_ifs_device.o
>
> obj-$(CONFIG_INTEL_IFS) += intel_ifs.o
>
> -intel_ifs-objs := core.o load.o runtest.o
> +intel_ifs-objs := core.o load.o runtest.o sysfs.o
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 0dc4cdda35ff..f56cde0cdfd6 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -3,6 +3,7 @@
>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/semaphore.h>
>
> #include "ifs.h"
>
> @@ -26,10 +27,18 @@ static int ifs_probe(struct platform_device *pdev)
> return 0;
> }
>
> +/*
> + * Note there is no need for a ->remove() call back. There isn't an
> + * "unload" operation to remove the scan binary from the BIOS reserved
> + * area. Also ".dev_groups" removal order will guarantee that any in
> + * flight tests have completed.
> + */
So you are ok with the warning the kernel gives you when you unload the
driver? That feels wrong :(
> +
> static struct platform_driver intel_ifs_driver = {
> .probe = ifs_probe,
> .driver = {
> .name = "intel_ifs",
> + .dev_groups = plat_ifs_groups,
> },
> };
>
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index f5e3636d709f..4e6662f2d2f8 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -97,4 +97,7 @@ int ifs_setup_wq(void);
> void ifs_destroy_wq(void);
> int do_core_test(int cpu, struct device *dev);
>
> +extern const struct attribute_group *plat_ifs_groups[];
> +extern struct semaphore ifs_sem;
> +
> #endif
> diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
> index 7793a01f7b94..246eff250563 100644
> --- a/drivers/platform/x86/intel/ifs/runtest.c
> +++ b/drivers/platform/x86/intel/ifs/runtest.c
> @@ -10,6 +10,13 @@
>
> #include "ifs.h"
>
> +/*
> + * Note all code and data in this file is protected by
> + * ifs_sem. On HT systems all threads on a core will
> + * execute together, but only the first thread on the
> + * core will update results of the test and indicate
> + * completion.
> + */
> static struct workqueue_struct *ifs_wq;
> static struct completion test_thread_done;
> static atomic_t siblings_in;
> diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
> new file mode 100644
> index 000000000000..41db2a12fbc8
> --- /dev/null
> +++ b/drivers/platform/x86/intel/ifs/sysfs.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2022 Intel Corporation. */
> +
> +#include <linux/cpu.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/platform_device.h>
> +#include <linux/semaphore.h>
> +#include <linux/slab.h>
> +
> +#include "ifs.h"
> +
> +/*
> + * Protects against simultaneous tests on multiple cores, or
> + * reloading can file while a test is in progress
> + */
> +DEFINE_SEMAPHORE(ifs_sem);
> +
> +/*
> + * The sysfs interface to check additional details of last test
> + * cat /sys/devices/system/platform/ifs/details
> + */
> +static ssize_t details_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ifs_data *ifsd = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%#llx\n", ifsd->scan_details);
> +}
> +
> +static DEVICE_ATTR_RO(details);
> +
> +static const char * const status_msg[] = {
> + [SCAN_NOT_TESTED] = "untested",
> + [SCAN_TEST_PASS] = "pass",
> + [SCAN_TEST_FAIL] = "fail"
> +};
> +
> +/*
> + * The sysfs interface to check the test status:
> + * To check the status of last test
> + * cat /sys/devices/platform/ifs/status
> + */
> +static ssize_t status_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ifs_data *ifsd = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%s\n", status_msg[ifsd->status]);
> +}
> +
> +static DEVICE_ATTR_RO(status);
> +
> +/*
> + * The sysfs interface for single core testing
> + * To start test, for example, cpu5
> + * echo 5 > /sys/devices/platform/ifs/run_test
> + * To check the result:
> + * cat /sys/devices/platform/ifs/result
> + * The sibling core gets tested at the same time.
> + */
> +static ssize_t run_test_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ifs_data *ifsd = dev_get_drvdata(dev);
> + unsigned int cpu;
> + int rc;
> +
> + rc = kstrtouint(buf, 0, &cpu);
> + if (rc < 0 || cpu >= nr_cpu_ids)
> + return -EINVAL;
> +
> + if (down_interruptible(&ifs_sem))
> + return -EINTR;
> +
> + if (!ifsd->loaded)
> + rc = -EPERM;
> + else
> + rc = do_core_test(cpu, dev);
> +
> + up(&ifs_sem);
> +
> + return rc ? rc : count;
> +}
> +
> +static DEVICE_ATTR_WO(run_test);
> +
> +/*
> + * Reload the IFS image. When user wants to install new IFS image
> + */
> +static ssize_t reload_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ifs_data *ifsd = dev_get_drvdata(dev);
> + int rc;
> +
> + if (!sysfs_streq(buf, "1"))
kstrtobool()?
> + return -EINVAL;
> +
> + if (down_interruptible(&ifs_sem))
> + return -EINTR;
> +
> + rc = load_ifs_binary(dev);
> +
> + ifsd->loaded = (rc == 0);
> +
> + up(&ifs_sem);
> +
> + return rc ? rc : count;
> +}
> +
> +static DEVICE_ATTR_WO(reload);
> +
> +/*
> + * Display currently loaded IFS image version.
> + */
> +static ssize_t image_version_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ifs_data *ifsd = dev_get_drvdata(dev);
> +
> + if (!ifsd->loaded)
> + return sysfs_emit(buf, "%s\n", "none");
> + else
> + return sysfs_emit(buf, "%#x\n", ifsd->loaded_version);
> +}
> +
> +static DEVICE_ATTR_RO(image_version);
> +
> +/* global scan sysfs attributes */
> +static struct attribute *plat_ifs_attrs[] = {
> + &dev_attr_details.attr,
> + &dev_attr_status.attr,
> + &dev_attr_run_test.attr,
> + &dev_attr_reload.attr,
> + &dev_attr_image_version.attr,
> + NULL
> +};
> +
> +static const struct attribute_group plat_ifs_attr_group = {
> + .attrs = plat_ifs_attrs,
> +};
> +
> +const struct attribute_group *plat_ifs_groups[] = {
> + &plat_ifs_attr_group,
> + NULL
> +};
ATTRIBUTE_GROUPS()?
thanks,
greg k-h
Powered by blists - more mailing lists