Le 08/08/2022 à 17:43, gjoyce@xxxxxxxxxxxxxxxxxx a écrit :
> From: Greg Joyce <gjoyce@xxxxxxxxxxxxxxxxxx>
>
> Self Encrypting Drives(SED) make use of POWER LPAR Platform KeyStore
> for storing its variables. Thus the block subsystem needs to access
> PowerPC specific functions to read/write objects in PLPKS.
>
> Override the default implementations in lib/arch_vars.c file with
> PowerPC specific versions.
>
> Signed-off-by: Greg Joyce <gjoyce@xxxxxxxxxxxxxxxxxx>
> ---
> arch/powerpc/platforms/pseries/Makefile | 1 +
> .../platforms/pseries/plpks_arch_ops.c | 167 ++++++++++++++++++
> 2 files changed, 168 insertions(+)
> create mode 100644 arch/powerpc/platforms/pseries/plpks_arch_ops.c
>
> diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
> index 14e143b946a3..3a545422eae5 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_PPC_SPLPAR) += vphn.o
> obj-$(CONFIG_PPC_SVM) += svm.o
> obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
> obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
> +obj-$(CONFIG_PSERIES_PLPKS) += plpks_arch_ops.o
>
> obj-$(CONFIG_SUSPEND) += suspend.o
> obj-$(CONFIG_PPC_VAS) += vas.o vas-sysfs.o
> diff --git a/arch/powerpc/platforms/pseries/plpks_arch_ops.c b/arch/powerpc/platforms/pseries/plpks_arch_ops.c
> new file mode 100644
> index 000000000000..fdea3322f696
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/plpks_arch_ops.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * POWER Platform arch specific code for SED
> + * Copyright (C) 2022 IBM Corporation
> + *
> + * Define operations for generic kernel subsystems to read/write keys
> + * from POWER LPAR Platform KeyStore(PLPKS).
> + *
> + * List of subsystems/usecase using PLPKS:
> + * - Self Encrypting Drives(SED)
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/ioctl.h>
> +#include <uapi/linux/sed-opal.h>
> +#include <linux/sed-opal.h>
> +#include <linux/arch_vars.h>
> +#include "plpks.h"
> +
> +/*
> + * variable structure that contains all SED data
> + */
> +struct plpks_sed_object_data {
> + u_char version;
> + u_char pad1[7];
> + u_long authority;
> + u_long range;
> + u_int key_len;
> + u_char key[32];
> +};
> +
> +/*
> + * ext_type values
> + * 00 no extension exists
> + * 01-1F common
> + * 20-3F AIX
> + * 40-5F Linux
> + * 60-7F IBMi
> + */
> +
> +/*
> + * This extension is optional for version 1 sed_object_data
> + */
> +struct sed_object_extension {
> + u8 ext_type;
> + u8 rsvd[3];
> + u8 ext_data[64];
> +};
> +
> +#define PKS_SED_OBJECT_DATA_V1 1
> +#define PKS_SED_MANGLED_LABEL "/default/pri"
> +#define PLPKS_SED_COMPONENT "sed-opal"
> +
> +#define PLPKS_ARCHVAR_POLICY WORLDREADABLE
> +#define PLPKS_ARCHVAR_OS_COMMON 4
> +
> +/*
> + * Read the variable data from PKS given the label
> + */
> +int arch_read_variable(enum arch_variable_type type, char *varname,
> + void *varbuf, u_int *varlen)
'enum arch_variable_type' is pointlessly long. And it only has two
possible values, so should be a 'bool'
> +{
> + struct plpks_var var;
> + struct plpks_sed_object_data *data;
> + u_int offset = 0;
Would be better to set it to 0 in the code that handles ARCH_VAR_OTHER
and leave it unitialised here.
> + char *buf = (char *)varbuf;
> + int ret;
> +
> + var.name = varname;
> + var.namelen = strlen(varname);
> + var.policy = PLPKS_ARCHVAR_POLICY;
> + var.os = PLPKS_ARCHVAR_OS_COMMON;
> + var.data = NULL;
> + var.datalen = 0;
> +
> + switch (type) {
A switch for a boolean value is pointless. Just do if/else, it will be a
lot more readable.
> + case ARCH_VAR_OPAL_KEY:
> + var.component = PLPKS_SED_COMPONENT;
> +#ifdef OPAL_AUTH_KEY
#ifdefs in C files should be avoided, refer
https://docs.kernel.org/process/coding-style.html#conditional-compilation
> + if (strcmp(OPAL_AUTH_KEY, varname) == 0) {
> + var.name = PKS_SED_MANGLED_LABEL;
> + var.namelen = strlen(varname);
> + }
> +#endif
> + offset = offsetof(struct plpks_sed_object_data, key);
> + break;
> + case ARCH_VAR_OTHER:
> + var.component = "";
> + break;
> + }
> +
> + ret = plpks_read_os_var(&var);
> + if (ret != 0)
> + return ret;
> +
> + if (offset > var.datalen)
> + offset = 0;
> +
> + switch (type) {
> + case ARCH_VAR_OPAL_KEY:
> + data = (struct plpks_sed_object_data *)var.data;
> + *varlen = data->key_len;
> + break;
> + case ARCH_VAR_OTHER:
> + *varlen = var.datalen;
> + break;
> + }
> +
> + if (var.data) {
> + memcpy(varbuf, var.data + offset, var.datalen - offset);
> + buf[*varlen] = '\0';
> + kfree(var.data);
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Write the variable data to PKS given the label
> + */
> +int arch_write_variable(enum arch_variable_type type, char *varname,
> + void *varbuf, u_int varlen)
> +{
> + struct plpks_var var;
> + struct plpks_sed_object_data data;
> + struct plpks_var_name vname;
> +
> + var.name = varname;
> + var.namelen = strlen(varname);
> + var.policy = PLPKS_ARCHVAR_POLICY;
> + var.os = PLPKS_ARCHVAR_OS_COMMON;
> + var.datalen = varlen;
> + var.data = varbuf;
> +
> + switch (type) {
> + case ARCH_VAR_OPAL_KEY:
> + var.component = PLPKS_SED_COMPONENT;
> +#ifdef OPAL_AUTH_KEY
> + if (strcmp(OPAL_AUTH_KEY, varname) == 0) {
> + var.name = PKS_SED_MANGLED_LABEL;
> + var.namelen = strlen(varname);
> + }
> +#endif
> + var.datalen = sizeof(struct plpks_sed_object_data);
> + var.data = (u8 *)&data;
> +
> + /* initialize SED object */
> + data.version = PKS_SED_OBJECT_DATA_V1;
> + data.authority = 0;
> + data.range = 0;
> + data.key_len = varlen;
> + memcpy(data.key, varbuf, varlen);
> + break;
> + case ARCH_VAR_OTHER:
> + var.component = "";
> + break;
> + }
That part of code seem to have a lot in common with
arch_read_variable(), can you refactor ?
> +
> + /* variable update requires delete first */
> + vname.namelen = var.namelen;
> + vname.name = var.name;
> + (void)plpks_remove_var(var.component, var.os, vname);
Do you really need that cast to (void) ?
> +
> + return plpks_write_var(var);
> +}
