On Wed, Sep 27, 2023 at 1:26 PM Greg Joyce <gjoyce@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, 2023-09-13 at 13:49 -0700, Nick Desaulniers wrote:
> > 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
>
> 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.
>
> >
> > > 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
