Re: [PATCH v4 1/3] powerpc/pseries: Correct secvar format representation for static key management

[R Nageswara Sastry <rnsastry@linux.ibm.com>]

On 11/06/25 2:49 AM, Srish Srinivasan wrote:
> On a PLPKS enabled PowerVM LPAR, the secvar format property for static
> key management is misrepresented as "ibm,plpks-sb-unknown", creating
> reason for confusion.
>
> Static key management mode uses fixed, built-in keys. Dynamic key
> management mode allows keys to be updated in production to handle
> security updates without firmware rebuilds.
>
> Define a function named plpks_get_sb_keymgmt_mode() to retrieve the
> key management mode based on the existence of the SB_VERSION property
> in the firmware.
>
> Set the secvar format property to either "ibm,plpks-sb-v<version>" or
> "ibm,plpks-sb-v0" based on the key management mode, and return the
> length of the secvar format property.
>
> Co-developed-by: Souradeep <soura@imap.linux.ibm.com>
> Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>

Tested-by: R Nageswara Sastry <rnsastry@linux.ibm.com>
With the following scenarios:
1. With and with out secure boot by enabling keystore_signed_updates and 
keystore_kbytes
2. With Dynamic Key Guest Secure Boot
3. With Static Key Guest Secure Boot

> ---
>   Documentation/ABI/testing/sysfs-secvar        | 10 ++-
>   arch/powerpc/platforms/pseries/plpks-secvar.c | 76 +++++++++++--------
>   2 files changed, 53 insertions(+), 33 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
> index 857cf12b0904..f001a4f4bd2e 100644
> --- a/Documentation/ABI/testing/sysfs-secvar
> +++ b/Documentation/ABI/testing/sysfs-secvar
> @@ -22,9 +22,13 @@ Description:	A string indicating which backend is in use by the firmware.
>   		and is expected to be "ibm,edk2-compat-v1".
>   
>   		On pseries/PLPKS, this is generated by the kernel based on the
> -		version number in the SB_VERSION variable in the keystore, and
> -		has the form "ibm,plpks-sb-v<version>", or
> -		"ibm,plpks-sb-unknown" if there is no SB_VERSION variable.
> +		version number in the SB_VERSION variable in the keystore. The
> +		version numbering in the SB_VERSION variable starts from 1. The
> +		format string takes the form "ibm,plpks-sb-v<version>" in the
> +		case of dynamic key management mode. If the SB_VERSION variable
> +		does not exist (or there is an error while reading it), it takes
> +		the form "ibm,plpks-sb-v0", indicating that the key management
> +		mode is static.
>   
>   What:		/sys/firmware/secvar/vars/<variable name>
>   Date:		August 2019
> diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
> index 257fd1f8bc19..767e5e8c6990 100644
> --- a/arch/powerpc/platforms/pseries/plpks-secvar.c
> +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
> @@ -152,39 +152,55 @@ static int plpks_set_variable(const char *key, u64 key_len, u8 *data,
>   	return rc;
>   }
>   
> -// PLPKS dynamic secure boot doesn't give us a format string in the same way OPAL does.
> -// Instead, report the format using the SB_VERSION variable in the keystore.
> -// The string is made up by us, and takes the form "ibm,plpks-sb-v<n>" (or "ibm,plpks-sb-unknown"
> -// if the SB_VERSION variable doesn't exist). Hypervisor defines the SB_VERSION variable as a
> -// "1 byte unsigned integer value".
> -static ssize_t plpks_secvar_format(char *buf, size_t bufsize)
> +/*
> + * Return the key management mode.
> + *
> + * SB_VERSION is defined as a "1 byte unsigned integer value", taking values
> + * starting from 1. It is owned by the Partition Firmware and its presence
> + * indicates that the key management mode is dynamic. Any failure in
> + * reading SB_VERSION defaults the key management mode to static. The error
> + * codes -ENOENT or -EPERM are expected in static key management mode. An
> + * unexpected error code will have to be investigated. Only signed variables
> + * have null bytes in their names, SB_VERSION does not.
> + *
> + * Return 0 to indicate that the key management mode is static. Otherwise
> + * return the SB_VERSION value to indicate that the key management mode is
> + * dynamic.
> + */
> +static u8 plpks_get_sb_keymgmt_mode(void)
>   {
> -	struct plpks_var var = {0};
> -	ssize_t ret;
> -	u8 version;
> -
> -	var.component = NULL;
> -	// Only the signed variables have null bytes in their names, this one doesn't
> -	var.name = "SB_VERSION";
> -	var.namelen = strlen(var.name);
> -	var.datalen = 1;
> -	var.data = &version;
> -
> -	// Unlike the other vars, SB_VERSION is owned by firmware instead of the OS
> -	ret = plpks_read_fw_var(&var);
> -	if (ret) {
> -		if (ret == -ENOENT) {
> -			ret = snprintf(buf, bufsize, "ibm,plpks-sb-unknown");
> -		} else {
> -			pr_err("Error %ld reading SB_VERSION from firmware\n", ret);
> -			ret = -EIO;
> -		}
> -		goto err;
> +	u8 mode;
> +	ssize_t rc;
> +	struct plpks_var var = {
> +		.component = NULL,
> +		.name = "SB_VERSION",
> +		.namelen = 10,
> +		.datalen = 1,
> +		.data = &mode,
> +	};
> +
> +	rc = plpks_read_fw_var(&var);
> +	if (rc) {
> +		if (rc != -ENOENT && rc != -EPERM)
> +			pr_info("Error %ld reading SB_VERSION from firmware\n", rc);
> +		mode = 0;
>   	}
> +	return mode;
> +}
>   
> -	ret = snprintf(buf, bufsize, "ibm,plpks-sb-v%hhu", version);
> -err:
> -	return ret;
> +/*
> + * PLPKS dynamic secure boot doesn't give us a format string in the same way
> + * OPAL does. Instead, report the format using the SB_VERSION variable in the
> + * keystore. The string, made up by us, takes the form of either
> + * "ibm,plpks-sb-v<n>" or "ibm,plpks-sb-v0", based on the key management mode,
> + * and return the length of the secvar format property.
> + */
> +static ssize_t plpks_secvar_format(char *buf, size_t bufsize)
> +{
> +	u8 mode;
> +
> +	mode = plpks_get_sb_keymgmt_mode();
> +	return snprintf(buf, bufsize, "ibm,plpks-sb-v%hhu", mode);
>   }
>   
>   static int plpks_max_size(u64 *max_size)

-- 
Thanks and Regards
R.Nageswara Sastry