Message-ID: <aac15eb0-ca25-5012-6346-86a89ec461ac@linux.ibm.com>
Date:   Tue, 31 Jan 2023 10:49:35 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Subject: Re: [PATCH v5 10/25] powerpc/secvar: Extend sysfs to include config
 vars
Content-Language: en-US
To:     Andrew Donnellan <ajd@linux.ibm.com>,
        linuxppc-dev@lists.ozlabs.org, linux-integrity@vger.kernel.org
Cc:     ruscur@russell.cc, bgray@linux.ibm.com, nayna@linux.ibm.com,
        gcwilson@linux.ibm.com, gjoyce@linux.ibm.com, brking@linux.ibm.com,
        sudhakar@linux.ibm.com, erichte@linux.ibm.com,
        gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
        zohar@linux.ibm.com, joel@jms.id.au, npiggin@gmail.com
References: <20230131063928.388035-1-ajd@linux.ibm.com>
 <20230131063928.388035-11-ajd@linux.ibm.com>
From:   Stefan Berger <stefanb@linux.ibm.com>
In-Reply-To: <20230131063928.388035-11-ajd@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 1/31/23 01:39, Andrew Donnellan wrote:
> From: Russell Currey <ruscur@russell.cc>
> 
> The forthcoming pseries consumer of the secvar API wants to expose a
> number of config variables.  Allowing secvar implementations to provide
> their own sysfs attributes makes it easy for consumers to expose what
> they need to.
> 
> This is not being used by the OPAL secvar implementation at present, and
> the config directory will not be created if no attributes are set.
> 
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

> 
> ---
> 
> v3: Remove unnecessary "secvar:" prefix from error messages (ajd)
> 
>      Merge config attributes into secvar_operations (mpe)
> ---
>   arch/powerpc/include/asm/secvar.h  |  2 ++
>   arch/powerpc/kernel/secvar-sysfs.c | 33 +++++++++++++++++++++++++-----
>   2 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
> index bf396215903d..011a53a8076c 100644
> --- a/arch/powerpc/include/asm/secvar.h
> +++ b/arch/powerpc/include/asm/secvar.h
> @@ -10,6 +10,7 @@
>   
>   #include <linux/types.h>
>   #include <linux/errno.h>
> +#include <linux/sysfs.h>
>   
>   extern const struct secvar_operations *secvar_ops;
>   
> @@ -19,6 +20,7 @@ struct secvar_operations {
>   	int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
>   	ssize_t (*format)(char *buf, size_t bufsize);
>   	int (*max_size)(u64 *max_size);
> +	const struct attribute **config_attrs;
>   };
>   
>   #ifdef CONFIG_PPC_SECURE_BOOT
> diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
> index 8f3deff94009..7df32be86507 100644
> --- a/arch/powerpc/kernel/secvar-sysfs.c
> +++ b/arch/powerpc/kernel/secvar-sysfs.c
> @@ -144,6 +144,19 @@ static int update_kobj_size(void)
>   	return 0;
>   }
>   
> +static int secvar_sysfs_config(struct kobject *kobj)
> +{
> +	struct attribute_group config_group = {
> +		.name = "config",
> +		.attrs = (struct attribute **)secvar_ops->config_attrs,
> +	};
> +
> +	if (secvar_ops->config_attrs)
> +		return sysfs_create_group(kobj, &config_group);
> +
> +	return 0;
> +}
> +
>   static int secvar_sysfs_load(void)
>   {
>   	struct kobject *kobj;
> @@ -208,26 +221,36 @@ static int secvar_sysfs_init(void)
>   
>   	rc = sysfs_create_file(secvar_kobj, &format_attr.attr);
>   	if (rc) {
> -		kobject_put(secvar_kobj);
> -		return -ENOMEM;
> +		pr_err("Failed to create format object\n");
> +		rc = -ENOMEM;
> +		goto err;
>   	}
>   
>   	secvar_kset = kset_create_and_add("vars", NULL, secvar_kobj);
>   	if (!secvar_kset) {
>   		pr_err("sysfs kobject registration failed\n");
> -		kobject_put(secvar_kobj);
> -		return -ENOMEM;
> +		rc = -ENOMEM;
> +		goto err;
>   	}
>   
>   	rc = update_kobj_size();
>   	if (rc) {
>   		pr_err("Cannot read the size of the attribute\n");
> -		return rc;
> +		goto err;
> +	}
> +
> +	rc = secvar_sysfs_config(secvar_kobj);
> +	if (rc) {
> +		pr_err("Failed to create config directory\n");
> +		goto err;
>   	}
>   
>   	secvar_sysfs_load();
>   
>   	return 0;
> +err:
> +	kobject_put(secvar_kobj);
> +	return rc;
>   }
>   
>   late_initcall(secvar_sysfs_init);