On Wed, Sep 13, 2023 at 01:49:39PM -0700, Nick Desaulniers wrote:

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

Ah yes, this is the other solution. I figured the way that it was
written, sed_read_key() and sed_write_key() may be overridden by a
different architecture or translation unit in the future but I think
until it is needed, your solution would be perfectly fine. Thanks for
taking a look!

Cheers,
Nathan

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