Re: [PATCH 4/4] powerpc/pseries: update SED for PLPKS api changes

2023-06-01 Thread Greg Joyce
On Mon, 2023-05-15 at 15:52 +1000, Andrew Donnellan wrote:
> On Fri, 2023-05-05 at 14:44 -0500, gjo...@linux.vnet.ibm.com wrote:
> > From: Greg Joyce 
> > 
> > Changes to the PLPKS API require minor updates to the SED Opal
> > PLPKS keystore code.
> > 
> > Signed-off-by: Greg Joyce 
> 
> [+ Nayna]
> 
> This patch will need to be squashed with patch 2.

Thanks. I've squashed the patches and will resend shortly.

> 
> > ---
> >  arch/powerpc/platforms/pseries/Kconfig|  6 +
> >  arch/powerpc/platforms/pseries/Makefile   |  2 +-
> >  .../powerpc/platforms/pseries/plpks_sed_ops.c | 22 +
> > 
> > --
> >  block/Kconfig |  1 +
> >  4 files changed, 13 insertions(+), 18 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/Kconfig
> > b/arch/powerpc/platforms/pseries/Kconfig
> > index 21b22bf16ce6..c2f8a29e7b9b 100644
> > --- a/arch/powerpc/platforms/pseries/Kconfig
> > +++ b/arch/powerpc/platforms/pseries/Kconfig
> > @@ -163,6 +163,12 @@ config PSERIES_PLPKS
> > # This option is selected by in-kernel consumers that
> > require
> > # access to the PKS.
> >  
> > +config PSERIES_PLPKS_SED
> > +   depends on PPC_PSERIES
> > +   bool
> > +   # This option is selected by in-kernel consumers that
> > require
> > +   # access to the SED PKS keystore.
> > +
> >  config PAPR_SCM
> > depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
> > tristate "Support for the PAPR Storage Class Memory
> > interface"
> > diff --git a/arch/powerpc/platforms/pseries/Makefile
> > b/arch/powerpc/platforms/pseries/Makefile
> > index 4242aed0d5d3..1476c5e4433c 100644
> > --- a/arch/powerpc/platforms/pseries/Makefile
> > +++ b/arch/powerpc/platforms/pseries/Makefile
> > @@ -29,7 +29,7 @@ obj-$(CONFIG_PPC_SVM) += svm.o
> >  obj-$(CONFIG_FA_DUMP)  += rtas-fadump.o
> >  obj-$(CONFIG_PSERIES_PLPKS)+= plpks.o
> >  obj-$(CONFIG_PPC_SECURE_BOOT)  += plpks-secvar.o
> > -obj-$(CONFIG_PSERIES_PLPKS_SED)+= plpks-sed.o
> > +obj-$(CONFIG_PSERIES_PLPKS_SED)+= plpks_sed_ops.o
> 
> I think you could just use obj-$(CONFIG_BLK_SED_OPAL) and then there
> wouldn't be a need to introduce a new option? Unless there's going to
> be a second consumer.

I was following the model of CONFIG_PPC_SECURE_BOOT. That gives a
littler finer control and flexibilty for using SED and PLPKS. This also
confines use of CONFIG_BLK_SED_OPAL to the base SED OPAL code.

> 
> >  obj-$(CONFIG_SUSPEND)  += suspend.o
> >  obj-$(CONFIG_PPC_VAS)  += vas.o vas-sysfs.o
> >  
> > diff --git a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> > b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> > index 086934b319a9..c1d08075e850 100644
> > --- a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> > +++ b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> > @@ -14,7 +14,7 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include "plpks.h"
> > +#include 
> >  
> >  /*
> >   * structure that contains all SED data
> > @@ -28,9 +28,6 @@ struct plpks_sed_object_data {
> > u_char key[32];
> >  };
> >  
> > -#define PLPKS_PLATVAR_POLICYWORLDREADABLE
> > -#define PLPKS_PLATVAR_OS_COMMON 4
> > -
> >  #define PLPKS_SED_OBJECT_DATA_V00
> >  #define PLPKS_SED_MANGLED_LABEL "/default/pri"
> >  #define PLPKS_SED_COMPONENT "sed-opal"
> > @@ -50,8 +47,8 @@ void plpks_init_var(struct plpks_var *var, char
> > *keyname)
> > var->name = PLPKS_SED_MANGLED_LABEL;
> > var->namelen = strlen(keyname);
> > }
> > -   var->policy = PLPKS_PLATVAR_POLICY;
> > -   var->os = PLPKS_PLATVAR_OS_COMMON;
> > +   var->policy = PLPKS_WORLDREADABLE;
> > +   var->os = PLPKS_VAR_COMMON;
> > var->data = NULL;
> > var->datalen = 0;
> > var->component = PLPKS_SED_COMPONENT;
> > @@ -64,28 +61,19 @@ int sed_read_key(char *keyname, char *key,
> > u_int
> > *keylen)
> >  {
> > struct plpks_var var;
> > struct plpks_sed_object_data data;
> > -   u_int offset;
> > int ret;
> > u_int len;
> >  
> > plpks_init_var(, keyname);
> > -   var.data = 
> > +   var.data = (u8 *)
> > var.datalen = sizeof(data);
> >  
> > ret = plpks_read_os_var();
> > if (ret != 0)
> > return ret;
> >  
> > -   offset = offsetof(struct plpks_sed_object_data, key);
> > -   if (offset > var.datalen) {
> > -   return -EINVAL;
> > -   }
> > -
> > -   len = min(be32_to_cpu(data.key_len), *keylen);
> > -
> > +   len = min_t(u16, be32_to_cpu(data.key_len), var.datalen);
> > memcpy(key, data.key, len);
> > -   kfree(var.data);
> > -
> > key[len] = '\0';
> > *keylen = len;
> >  
> > diff --git a/block/Kconfig b/block/Kconfig
> > index 76b23114fdeb..75d4db34df5a 100644
> > --- a/block/Kconfig
> > +++ b/block/Kconfig
> > @@ 

Re: [PATCH 4/4] powerpc/pseries: update SED for PLPKS api changes

2023-05-14 Thread Andrew Donnellan
On Fri, 2023-05-05 at 14:44 -0500, gjo...@linux.vnet.ibm.com wrote:
> From: Greg Joyce 
> 
> Changes to the PLPKS API require minor updates to the SED Opal
> PLPKS keystore code.
> 
> Signed-off-by: Greg Joyce 

[+ Nayna]

This patch will need to be squashed with patch 2.

> ---
>  arch/powerpc/platforms/pseries/Kconfig    |  6 +
>  arch/powerpc/platforms/pseries/Makefile   |  2 +-
>  .../powerpc/platforms/pseries/plpks_sed_ops.c | 22 +
> --
>  block/Kconfig |  1 +
>  4 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/Kconfig
> b/arch/powerpc/platforms/pseries/Kconfig
> index 21b22bf16ce6..c2f8a29e7b9b 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -163,6 +163,12 @@ config PSERIES_PLPKS
> # This option is selected by in-kernel consumers that require
> # access to the PKS.
>  
> +config PSERIES_PLPKS_SED
> +   depends on PPC_PSERIES
> +   bool
> +   # This option is selected by in-kernel consumers that require
> +   # access to the SED PKS keystore.
> +
>  config PAPR_SCM
> depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
> tristate "Support for the PAPR Storage Class Memory
> interface"
> diff --git a/arch/powerpc/platforms/pseries/Makefile
> b/arch/powerpc/platforms/pseries/Makefile
> index 4242aed0d5d3..1476c5e4433c 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -29,7 +29,7 @@ obj-$(CONFIG_PPC_SVM) += svm.o
>  obj-$(CONFIG_FA_DUMP)  += rtas-fadump.o
>  obj-$(CONFIG_PSERIES_PLPKS)+= plpks.o
>  obj-$(CONFIG_PPC_SECURE_BOOT)  += plpks-secvar.o
> -obj-$(CONFIG_PSERIES_PLPKS_SED)+= plpks-sed.o
> +obj-$(CONFIG_PSERIES_PLPKS_SED)+= plpks_sed_ops.o

I think you could just use obj-$(CONFIG_BLK_SED_OPAL) and then there
wouldn't be a need to introduce a new option? Unless there's going to
be a second consumer.

>  obj-$(CONFIG_SUSPEND)  += suspend.o
>  obj-$(CONFIG_PPC_VAS)  += vas.o vas-sysfs.o
>  
> diff --git a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> index 086934b319a9..c1d08075e850 100644
> --- a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> +++ b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> @@ -14,7 +14,7 @@
>  #include 
>  #include 
>  #include 
> -#include "plpks.h"
> +#include 
>  
>  /*
>   * structure that contains all SED data
> @@ -28,9 +28,6 @@ struct plpks_sed_object_data {
> u_char key[32];
>  };
>  
> -#define PLPKS_PLATVAR_POLICY    WORLDREADABLE
> -#define PLPKS_PLATVAR_OS_COMMON 4
> -
>  #define PLPKS_SED_OBJECT_DATA_V0    0
>  #define PLPKS_SED_MANGLED_LABEL "/default/pri"
>  #define PLPKS_SED_COMPONENT "sed-opal"
> @@ -50,8 +47,8 @@ void plpks_init_var(struct plpks_var *var, char
> *keyname)
> var->name = PLPKS_SED_MANGLED_LABEL;
> var->namelen = strlen(keyname);
> }
> -   var->policy = PLPKS_PLATVAR_POLICY;
> -   var->os = PLPKS_PLATVAR_OS_COMMON;
> +   var->policy = PLPKS_WORLDREADABLE;
> +   var->os = PLPKS_VAR_COMMON;
> var->data = NULL;
> var->datalen = 0;
> var->component = PLPKS_SED_COMPONENT;
> @@ -64,28 +61,19 @@ int sed_read_key(char *keyname, char *key, u_int
> *keylen)
>  {
> struct plpks_var var;
> struct plpks_sed_object_data data;
> -   u_int offset;
> int ret;
> u_int len;
>  
> plpks_init_var(, keyname);
> -   var.data = 
> +   var.data = (u8 *)
> var.datalen = sizeof(data);
>  
> ret = plpks_read_os_var();
> if (ret != 0)
> return ret;
>  
> -   offset = offsetof(struct plpks_sed_object_data, key);
> -   if (offset > var.datalen) {
> -   return -EINVAL;
> -   }
> -
> -   len = min(be32_to_cpu(data.key_len), *keylen);
> -
> +   len = min_t(u16, be32_to_cpu(data.key_len), var.datalen);
> memcpy(key, data.key, len);
> -   kfree(var.data);
> -
> key[len] = '\0';
> *keylen = len;
>  
> diff --git a/block/Kconfig b/block/Kconfig
> index 76b23114fdeb..75d4db34df5a 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -182,6 +182,7 @@ config BLK_SED_OPAL
> bool "Logic for interfacing with Opal enabled SEDs"
> depends on KEYS
> select PSERIES_PLPKS if PPC_PSERIES
> +   select PSERIES_PLPKS_SED if PPC_PSERIES
> help
> Builds Logic for interfacing with Opal enabled controllers.
> Enabling this option enables users to setup/unlock/lock

-- 
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com   IBM Australia Limited


[PATCH 4/4] powerpc/pseries: update SED for PLPKS api changes

2023-05-05 Thread gjoyce
From: Greg Joyce 

Changes to the PLPKS API require minor updates to the SED Opal
PLPKS keystore code.

Signed-off-by: Greg Joyce 
---
 arch/powerpc/platforms/pseries/Kconfig|  6 +
 arch/powerpc/platforms/pseries/Makefile   |  2 +-
 .../powerpc/platforms/pseries/plpks_sed_ops.c | 22 +--
 block/Kconfig |  1 +
 4 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/Kconfig 
b/arch/powerpc/platforms/pseries/Kconfig
index 21b22bf16ce6..c2f8a29e7b9b 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -163,6 +163,12 @@ config PSERIES_PLPKS
# This option is selected by in-kernel consumers that require
# access to the PKS.
 
+config PSERIES_PLPKS_SED
+   depends on PPC_PSERIES
+   bool
+   # This option is selected by in-kernel consumers that require
+   # access to the SED PKS keystore.
+
 config PAPR_SCM
depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
tristate "Support for the PAPR Storage Class Memory interface"
diff --git a/arch/powerpc/platforms/pseries/Makefile 
b/arch/powerpc/platforms/pseries/Makefile
index 4242aed0d5d3..1476c5e4433c 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -29,7 +29,7 @@ obj-$(CONFIG_PPC_SVM) += svm.o
 obj-$(CONFIG_FA_DUMP)  += rtas-fadump.o
 obj-$(CONFIG_PSERIES_PLPKS)+= plpks.o
 obj-$(CONFIG_PPC_SECURE_BOOT)  += plpks-secvar.o
-obj-$(CONFIG_PSERIES_PLPKS_SED)+= plpks-sed.o
+obj-$(CONFIG_PSERIES_PLPKS_SED)+= plpks_sed_ops.o
 obj-$(CONFIG_SUSPEND)  += suspend.o
 obj-$(CONFIG_PPC_VAS)  += vas.o vas-sysfs.o
 
diff --git a/arch/powerpc/platforms/pseries/plpks_sed_ops.c 
b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
index 086934b319a9..c1d08075e850 100644
--- a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
+++ b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
@@ -14,7 +14,7 @@
 #include 
 #include 
 #include 
-#include "plpks.h"
+#include 
 
 /*
  * structure that contains all SED data
@@ -28,9 +28,6 @@ struct plpks_sed_object_data {
u_char key[32];
 };
 
-#define PLPKS_PLATVAR_POLICYWORLDREADABLE
-#define PLPKS_PLATVAR_OS_COMMON 4
-
 #define PLPKS_SED_OBJECT_DATA_V00
 #define PLPKS_SED_MANGLED_LABEL "/default/pri"
 #define PLPKS_SED_COMPONENT "sed-opal"
@@ -50,8 +47,8 @@ void plpks_init_var(struct plpks_var *var, char *keyname)
var->name = PLPKS_SED_MANGLED_LABEL;
var->namelen = strlen(keyname);
}
-   var->policy = PLPKS_PLATVAR_POLICY;
-   var->os = PLPKS_PLATVAR_OS_COMMON;
+   var->policy = PLPKS_WORLDREADABLE;
+   var->os = PLPKS_VAR_COMMON;
var->data = NULL;
var->datalen = 0;
var->component = PLPKS_SED_COMPONENT;
@@ -64,28 +61,19 @@ int sed_read_key(char *keyname, char *key, u_int *keylen)
 {
struct plpks_var var;
struct plpks_sed_object_data data;
-   u_int offset;
int ret;
u_int len;
 
plpks_init_var(, keyname);
-   var.data = 
+   var.data = (u8 *)
var.datalen = sizeof(data);
 
ret = plpks_read_os_var();
if (ret != 0)
return ret;
 
-   offset = offsetof(struct plpks_sed_object_data, key);
-   if (offset > var.datalen) {
-   return -EINVAL;
-   }
-
-   len = min(be32_to_cpu(data.key_len), *keylen);
-
+   len = min_t(u16, be32_to_cpu(data.key_len), var.datalen);
memcpy(key, data.key, len);
-   kfree(var.data);
-
key[len] = '\0';
*keylen = len;
 
diff --git a/block/Kconfig b/block/Kconfig
index 76b23114fdeb..75d4db34df5a 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -182,6 +182,7 @@ config BLK_SED_OPAL
bool "Logic for interfacing with Opal enabled SEDs"
depends on KEYS
select PSERIES_PLPKS if PPC_PSERIES
+   select PSERIES_PLPKS_SED if PPC_PSERIES
help
Builds Logic for interfacing with Opal enabled controllers.
Enabling this option enables users to setup/unlock/lock
-- 
gjo...@linux.vnet.ibm.com