On Wed, Sep 27, 2023 at 1:26 PM Greg Joyce [off-list ref] wrote:

On Wed, 2023-09-13 at 13:49 -0700, Nick Desaulniers wrote:

quoted

On Wed, Sep 13, 2023 at 9:56 AM Nathan Chancellor [off-list ref]
wrote:

quoted

Hi Greg,

On Fri, Sep 08, 2023 at 10:30:54AM -0500, gjoyce@linux.vnet.ibm.com
 wrote:

quoted

From: Greg Joyce <redacted>

Add read and write functions that allow SED Opal keys to stored
in a permanent keystore.

Signed-off-by: Greg Joyce <redacted>
Reviewed-by: Jonathan Derrick <jonathan.derrick@linux.dev>
---
 block/Makefile               |  2 +-
 block/sed-opal-key.c         | 24 ++++++++++++++++++++++++
 include/linux/sed-opal-key.h | 15 +++++++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 block/sed-opal-key.c
 create mode 100644 include/linux/sed-opal-key.h

diff --git a/block/Makefile b/block/Makefile
index 46ada9dc8bbf..ea07d80402a6 100644
--- a/block/Makefile
+++ b/block/Makefile

@@ -34,7 +34,7 @@ obj-$(CONFIG_BLK_DEV_ZONED) += blk-zoned.o
 obj-$(CONFIG_BLK_WBT)                += blk-wbt.o
 obj-$(CONFIG_BLK_DEBUG_FS)   += blk-mq-debugfs.o
 obj-$(CONFIG_BLK_DEBUG_FS_ZONED)+= blk-mq-debugfs-zoned.o
-obj-$(CONFIG_BLK_SED_OPAL)   += sed-opal.o
+obj-$(CONFIG_BLK_SED_OPAL)   += sed-opal.o sed-opal-key.o
 obj-$(CONFIG_BLK_PM)         += blk-pm.o
 obj-$(CONFIG_BLK_INLINE_ENCRYPTION)  += blk-crypto.o blk-crypto-

profile.o \
                                         blk-crypto-sysfs.o

diff --git a/block/sed-opal-key.c b/block/sed-opal-key.c
new file mode 100644
index 000000000000..16f380164c44
--- /dev/null
+++ b/block/sed-opal-key.c

@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SED key operations.
+ *
+ * Copyright (C) 2022 IBM Corporation
+ *
+ * These are the accessor functions (read/write) for SED Opal
+ * keys. Specific keystores can provide overrides.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/sed-opal-key.h>
+
+int __weak sed_read_key(char *keyname, char *key, u_int *keylen)
+{
+     return -EOPNOTSUPP;
+}
+
+int __weak sed_write_key(char *keyname, char *key, u_int keylen)
+{
+     return -EOPNOTSUPP;
+}

This change causes a build failure for certain clang configurations
due
to an unfortunate issue [1] with recordmcount, clang's integrated
assembler, and object files that contain a section with only weak
functions/symbols (in this case, the .text section in sed-opal-
key.c),
resulting in

  Cannot find symbol for section 2: .text.
  block/sed-opal-key.o: failed

when building this file.

The definitions in
block/sed-opal-key.c
should be deleted. Instead, in
include/linux/sed-opal-key.h
CONFIG_PSERIES_PLPKS_SED should be used to define static inline
versions when CONFIG_PSERIES_PLPKS_SED is not defined.

#ifdef CONFIG_PSERIES_PLPKS_SED
int sed_read_key(char *keyname, char *key, u_int *keylen);
int sed_write_key(char *keyname, char *key, u_int keylen);
#else
static inline
int sed_read_key(char *keyname, char *key, u_int *keylen) {
  return -EOPNOTSUPP;
}
static inline
int sed_write_key(char *keyname, char *key, u_int keylen);
  return -EOPNOTSUPP;
}
#endif

This change will certainly work for pseries. The intent of the weak
functions was to allow a different unknown permanent keystore to be the
source for seeding SED Opal keys. It also kept platform specific code
out of the block directory.

I'm happy to switch to the approach above, if losing those two goals
isn't a concern.

Assuming those would have mutually exclusive KConfigs, then the
pattern I describe would be preferred.

quoted

quoted

Is there any real reason to have a separate translation unit for
these
two functions versus just having them living in sed-opal.c? Those
two
object files share the same Kconfig dependency. I am happy to send
a
patch if that is an acceptable approach.

[1]: https://github.com/ClangBuiltLinux/linux/issues/981

Cheers,
Nathan

-- 
Thanks,
~Nick Desaulniers