linux-kernel - Re: [PATCH v3 2/8] platform/x86/intel/ifs: IFS cleanup

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: <e4cbc603-ca16-41db-d08d-b5d5250d62b4@redhat.com>
Date:   Mon, 13 Mar 2023 16:02:29 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Jithu Joseph <jithu.joseph@...el.com>, markgross@...nel.org
Cc:     tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
        gregkh@...uxfoundation.org, rostedt@...dmis.org,
        ashok.raj@...el.com, tony.luck@...el.com,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        patches@...ts.linux.dev, ravi.v.shankar@...el.com,
        thiago.macieira@...el.com, athenas.jimenez.gonzalez@...el.com,
        sohil.mehta@...el.com
Subject: Re: [PATCH v3 2/8] platform/x86/intel/ifs: IFS cleanup

Hi,

On 3/1/23 02:59, Jithu Joseph wrote:
> Cleanup incorporating misc review comments
> 
>  - Remove the subdirectory intel_ifs/0 for devicenode [1]
>  - Make plat_ifs_groups non static and use it directly without using a
>     function [2]
> 
> Link: https://lore.kernel.org/lkml/Y+4kQOtrHt5pdsSO@kroah.com/ [1]
> Link: https://lore.kernel.org/lkml/Y9nyxNesVHCUXAcH@kroah.com/  [2]
> 
> Signed-off-by: Jithu Joseph <jithu.joseph@...el.com>
> Reviewed-by: Tony Luck <tony.luck@...el.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@...hat.com>

Please add my Reviewed-by to this patch for the next version of
the series, so that I know which patches I have already reviewed.

Regards,

Hans

> ---
>  drivers/platform/x86/intel/ifs/ifs.h   | 2 +-
>  drivers/platform/x86/intel/ifs/core.c  | 6 +++---
>  drivers/platform/x86/intel/ifs/sysfs.c | 9 +--------
>  3 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index e07463c794d4..ab168ddf28f1 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -246,6 +246,6 @@ static inline struct ifs_data *ifs_get_data(struct device *dev)
>  extern bool *ifs_pkg_auth;
>  int ifs_load_firmware(struct device *dev);
>  int do_core_test(int cpu, struct device *dev);
> -const struct attribute_group **ifs_get_groups(void);
> +extern struct attribute *plat_ifs_attrs[];
>  
>  #endif
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index b518b661daf0..62c44dbae757 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -20,6 +20,8 @@ static const struct x86_cpu_id ifs_cpu_ids[] __initconst = {
>  };
>  MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);
>  
> +ATTRIBUTE_GROUPS(plat_ifs);
> +
>  bool *ifs_pkg_auth;
>  
>  static struct ifs_device ifs_device = {
> @@ -29,8 +31,8 @@ static struct ifs_device ifs_device = {
>  	},
>  	.misc = {
>  		.name = "intel_ifs_0",
> -		.nodename = "intel_ifs/0",
>  		.minor = MISC_DYNAMIC_MINOR,
> +		.groups = plat_ifs_groups,
>  	},
>  };
>  
> @@ -53,8 +55,6 @@ static int __init ifs_init(void)
>  	if (rdmsrl_safe(MSR_INTEGRITY_CAPS, &msrval))
>  		return -ENODEV;
>  
> -	ifs_device.misc.groups = ifs_get_groups();
> -
>  	if (!(msrval & BIT(ifs_device.ro_data.integrity_cap_bit)))
>  		return -ENODEV;
>  
> diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
> index ee636a76b083..2007d8054f04 100644
> --- a/drivers/platform/x86/intel/ifs/sysfs.c
> +++ b/drivers/platform/x86/intel/ifs/sysfs.c
> @@ -141,7 +141,7 @@ static ssize_t image_version_show(struct device *dev,
>  static DEVICE_ATTR_RO(image_version);
>  
>  /* global scan sysfs attributes */
> -static struct attribute *plat_ifs_attrs[] = {
> +struct attribute *plat_ifs_attrs[] = {
>  	&dev_attr_details.attr,
>  	&dev_attr_status.attr,
>  	&dev_attr_run_test.attr,
> @@ -149,10 +149,3 @@ static struct attribute *plat_ifs_attrs[] = {
>  	&dev_attr_image_version.attr,
>  	NULL
>  };
> -
> -ATTRIBUTE_GROUPS(plat_ifs);
> -
> -const struct attribute_group **ifs_get_groups(void)
> -{
> -	return plat_ifs_groups;
> -}