Re: [PATCH v7 3/3 RESEND] powerpc/pseries: PLPKS SED Opal keystore support

[Michael Ellerman <mpe@ellerman.id.au>]

Michal Suchánek <msuchanek@suse.de> writes:
> Hello,
>
> On Thu, Sep 14, 2023 at 02:13:32PM +1000, Michael Ellerman wrote:
>> Nathan Chancellor <nathan@kernel.org> writes:
>> > Hi Greg,
>> >
>> > On Fri, Sep 08, 2023 at 10:30:56AM -0500, gjoyce@linux.vnet.ibm.com wrote:
>> >> From: Greg Joyce <gjoyce@linux.vnet.ibm.com>
>> >>
>> >> Define operations for SED Opal to read/write keys
>> >> from POWER LPAR Platform KeyStore(PLPKS). This allows
>> >> non-volatile storage of SED Opal keys.
>> >>
>> >> Signed-off-by: Greg Joyce <gjoyce@linux.vnet.ibm.com>
>> >> Reviewed-by: Jonathan Derrick <jonathan.derrick@linux.dev>
>> >> Reviewed-by: Hannes Reinecke <hare@suse.de>
>> >
>> > After this change in -next as commit 9f2c7411ada9 ("powerpc/pseries:
>> > PLPKS SED Opal keystore support"), I see the following crash when
>> > booting some distribution configurations, such as OpenSUSE's [1] (the
>> > rootfs is available at [2] if necessary):
>> 
>> Thanks for testing Nathan.
>> 
>> The code needs to check plpks_is_available() somewhere, before calling
>> the plpks routines.
>
> would this fixup do it?
>
> I don't really see any other place to plug the check with the current
> code structure.
 
I think the plpks_sed code should call plpks_is_available() once at init
time and cache the result.

Otherwise it's will be doing an extra hcall (in _plpks_get_config()) for
every call, which would be wasteful. 

cheers

> diff --git a/arch/powerpc/platforms/pseries/plpks_sed_ops.c b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> index c1d08075e850..f8038d998eae 100644
> --- a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> +++ b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> @@ -64,6 +64,9 @@ int sed_read_key(char *keyname, char *key, u_int *keylen)
>  	int ret;
>  	u_int len;
>  
> +	if (!plpks_is_available())
> +		return -ENODEV;
> +
>  	plpks_init_var(&var, keyname);
>  	var.data = (u8 *)&data;
>  	var.datalen = sizeof(data);
> @@ -89,6 +92,9 @@ int sed_write_key(char *keyname, char *key, u_int keylen)
>  	struct plpks_sed_object_data data;
>  	struct plpks_var_name vname;
>  
> +	if (!plpks_is_available())
> +		return -ENODEV;
> +
>  	plpks_init_var(&var, keyname);
>  
>  	var.datalen = sizeof(struct plpks_sed_object_data);
> -- 
> 2.41.0