On Wed, Sep 13, 2023 at 9:56 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > > Hi Greg, > > On Fri, Sep 08, 2023 at 10:30:54AM -0500, gjoyce@xxxxxxxxxxxxxxxxxx wrote: > > From: Greg Joyce <gjoyce@xxxxxxxxxxxxxxxxxx> > > > > Add read and write functions that allow SED Opal keys to stored > > in a permanent keystore. > > > > Signed-off-by: Greg Joyce <gjoyce@xxxxxxxxxxxxxxxxxx> > > Reviewed-by: Jonathan Derrick <jonathan.derrick@xxxxxxxxx> > > --- > > 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 > > 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