The X.509 certificates trusted by the platform and required to secure boot
the OS kernel are wrapped in secure variables, which are controlled by
OPAL.
This patch adds firmware/kernel interface to read and write OPAL secure
variables based on the unique key.
This support can be enabled using CONFIG_OPAL_SECVAR.
Signed-off-by: Claudio Carvalho <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
---
arch/powerpc/include/asm/opal-api.h | 5 +-
arch/powerpc/include/asm/opal.h | 6 ++
arch/powerpc/include/asm/secvar.h | 55 ++++++++++
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/secvar-ops.c | 25 +++++
arch/powerpc/platforms/powernv/Kconfig | 6 ++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-call.c | 3 +
arch/powerpc/platforms/powernv/opal-secvar.c | 102 +++++++++++++++++++
arch/powerpc/platforms/powernv/opal.c | 5 +
10 files changed, 208 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/secvar.h
create mode 100644 arch/powerpc/kernel/secvar-ops.c
create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 383242eb0dea..b238b4f26c5b 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -208,7 +208,10 @@
#define OPAL_HANDLE_HMI2 166
#define OPAL_NX_COPROC_INIT 167
#define OPAL_XIVE_GET_VP_STATE 170
-#define OPAL_LAST 170
+#define OPAL_SECVAR_GET 173
+#define OPAL_SECVAR_GET_NEXT 174
+#define OPAL_SECVAR_ENQUEUE_UPDATE 175
+#define OPAL_LAST 175
#define QUIESCE_HOLD 1 /* Spin all calls at entry */
#define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 57bd029c715e..247adec2375f 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -388,6 +388,12 @@ void opal_powercap_init(void);
void opal_psr_init(void);
void opal_sensor_groups_init(void);
+extern int opal_secvar_get(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_data, uint64_t k_data_size);
+extern int opal_secvar_get_next(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_key_size);
+extern int opal_secvar_enqueue_update(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_data, uint64_t k_data_size);
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_OPAL_H */
diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
new file mode 100644
index 000000000000..645654456265
--- /dev/null
+++ b/arch/powerpc/include/asm/secvar.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PowerPC secure variable operations.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <[email protected]>
+ *
+ */
+#ifndef SECVAR_OPS_H
+#define SECVAR_OPS_H
+
+#include<linux/types.h>
+#include<linux/errno.h>
+
+struct secvar_operations {
+ int (*get_variable)(const char *key, unsigned long key_len, u8 *data,
+ unsigned long *data_size);
+ int (*get_next_variable)(const char *key, unsigned long *key_len,
+ unsigned long keysize);
+ int (*set_variable)(const char *key, unsigned long key_len, u8 *data,
+ unsigned long data_size);
+};
+
+#ifdef CONFIG_PPC_SECURE_BOOT
+
+extern void set_secvar_ops(struct secvar_operations *ops);
+extern struct secvar_operations *get_secvar_ops(void);
+
+#else
+
+static inline void set_secvar_ops(struct secvar_operations *ops)
+{
+}
+
+static inline struct secvar_operations *get_secvar_ops(void)
+{
+ return NULL;
+}
+
+#endif
+
+#ifdef CONFIG_OPAL_SECVAR
+
+extern int secvar_init(void);
+
+#else
+
+static inline int secvar_init(void)
+{
+ return -EINVAL;
+}
+
+#endif
+
+#endif
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 520b1c814197..9041563f1c74 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -157,7 +157,7 @@ endif
obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o
obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
-obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o
+obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o secvar-ops.o
# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c
new file mode 100644
index 000000000000..198222499848
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-ops.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <[email protected]>
+ *
+ * secvar-ops.c
+ * - initialize secvar operations for PowerPC Secureboot
+ */
+
+#include<stddef.h>
+#include<asm/secvar.h>
+
+static struct secvar_operations *secvars_ops;
+
+void set_secvar_ops(struct secvar_operations *ops)
+{
+ if (!ops)
+ secvars_ops = NULL;
+ secvars_ops = ops;
+}
+
+struct secvar_operations *get_secvar_ops(void)
+{
+ return secvars_ops;
+}
diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
index 850eee860cf2..65b060539b5c 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -47,3 +47,9 @@ config PPC_VAS
VAS adapters are found in POWER9 based systems.
If unsure, say N.
+
+config OPAL_SECVAR
+ bool "OPAL Secure Variables"
+ depends on PPC_POWERNV
+ help
+ This enables the kernel to access OPAL secure variables.
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index da2e99efbd04..6651c742e530 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
obj-$(CONFIG_PPC_MEMTRACE) += memtrace.o
obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o
obj-$(CONFIG_OCXL_BASE) += ocxl.o
+obj-$(CONFIG_OPAL_SECVAR) += opal-secvar.o
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 29ca523c1c79..93106e867924 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -287,3 +287,6 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar, OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
OPAL_CALL(opal_sensor_read_u64, OPAL_SENSOR_READ_U64);
OPAL_CALL(opal_sensor_group_enable, OPAL_SENSOR_GROUP_ENABLE);
OPAL_CALL(opal_nx_coproc_init, OPAL_NX_COPROC_INIT);
+OPAL_CALL(opal_secvar_get, OPAL_SECVAR_GET);
+OPAL_CALL(opal_secvar_get_next, OPAL_SECVAR_GET_NEXT);
+OPAL_CALL(opal_secvar_enqueue_update, OPAL_SECVAR_ENQUEUE_UPDATE);
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
new file mode 100644
index 000000000000..b0f97cea7675
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PowerNV code for secure variables
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Claudio Carvalho <[email protected]>
+ *
+ * APIs to access secure variables managed by OPAL.
+ *
+ */
+
+#define pr_fmt(fmt) "secvar: "fmt
+
+#include <linux/types.h>
+#include <asm/opal.h>
+#include <asm/secvar.h>
+
+static bool is_opal_secvar_supported(void)
+{
+ static bool opal_secvar_supported;
+ static bool initialized;
+
+ if (initialized)
+ return opal_secvar_supported;
+
+ if (!opal_check_token(OPAL_SECVAR_GET)
+ || !opal_check_token(OPAL_SECVAR_GET_NEXT)
+ || !opal_check_token(OPAL_SECVAR_ENQUEUE_UPDATE)) {
+ pr_err("OPAL doesn't support secure variables\n");
+ opal_secvar_supported = false;
+ } else {
+ opal_secvar_supported = true;
+ }
+
+ initialized = true;
+
+ return opal_secvar_supported;
+}
+
+static int opal_get_variable(const char *key, unsigned long ksize,
+ u8 *data, unsigned long *dsize)
+{
+ int rc;
+
+ if (!is_opal_secvar_supported())
+ return OPAL_UNSUPPORTED;
+
+ if (dsize)
+ *dsize = cpu_to_be64(*dsize);
+
+ rc = opal_secvar_get(__pa(key), ksize,
+ __pa(data), __pa(dsize));
+
+ if (dsize)
+ *dsize = be64_to_cpu(*dsize);
+
+ return rc;
+}
+
+static int opal_get_next_variable(const char *key, unsigned long *keylen,
+ unsigned long keysize)
+{
+ int rc;
+
+ if (!is_opal_secvar_supported())
+ return OPAL_UNSUPPORTED;
+
+ if (keylen)
+ *keylen = cpu_to_be64(*keylen);
+
+ rc = opal_secvar_get_next(__pa(key), __pa(keylen), keysize);
+
+ if (keylen)
+ *keylen = be64_to_cpu(*keylen);
+
+ return rc;
+}
+
+static int opal_set_variable(const char *key, unsigned long ksize, u8 *data,
+ unsigned long dsize)
+{
+ int rc;
+
+ if (!is_opal_secvar_supported())
+ return OPAL_UNSUPPORTED;
+
+ rc = opal_secvar_enqueue_update(__pa(key), ksize, __pa(data), dsize);
+
+ return rc;
+}
+
+static struct secvar_operations secvar_ops = {
+ .get_variable = opal_get_variable,
+ .get_next_variable = opal_get_next_variable,
+ .set_variable = opal_set_variable,
+};
+
+int secvar_init(void)
+{
+ set_secvar_ops(&secvar_ops);
+ return 0;
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index aba443be7daa..ffe6f1cf0830 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -32,6 +32,8 @@
#include <asm/mce.h>
#include <asm/imc-pmu.h>
#include <asm/bug.h>
+#include <asm/secvar.h>
+#include <asm/secboot.h>
#include "powernv.h"
@@ -988,6 +990,9 @@ static int __init opal_init(void)
/* Initialise OPAL Power control interface */
opal_power_control_init();
+ if (is_powerpc_secvar_supported())
+ secvar_init();
+
return 0;
}
machine_subsys_initcall(powernv, opal_init);
--
2.20.1
On Thu, Aug 22, 2019 at 3:02 PM Oliver O'Halloran <[email protected]> wrote:
>
> On Wed, 2019-08-21 at 11:08 -0400, Nayna Jain wrote:
> > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> > index aba443be7daa..ffe6f1cf0830 100644
> > --- a/arch/powerpc/platforms/powernv/opal.c
> > +++ b/arch/powerpc/platforms/powernv/opal.c
> > @@ -32,6 +32,8 @@
> > #include <asm/mce.h>
> > #include <asm/imc-pmu.h>
> > #include <asm/bug.h>
> > +#include <asm/secvar.h>
> > +#include <asm/secboot.h>
> >
> > #include "powernv.h"
> >
> > @@ -988,6 +990,9 @@ static int __init opal_init(void)
> > /* Initialise OPAL Power control interface */
> > opal_power_control_init();
> >
> > + if (is_powerpc_secvar_supported())
> > + secvar_init();
> > +
>
> The usual pattern here is to have the init function check for support
> internally.
>
> Also, is_powerpc_secvar_supported() doesn't appear to be defined
> anywhere. Is that supposed to be is_opal_secvar_supported()? Or is this
> series supposed to be applied on top of another series?
To answer my own question, yes it depends on the series at [1] which
adds IMA support. Turns out actually reading the cover letter helps,
who knew.
That said, I'm still not entirely sure about this. The implementation
of is_powerpc_secvar_supported() in [2] parses the DT and seems to
assume the DT bindings that OPAL produces. Are those common with the
DT bindings produced by OF when running on pseries?
[1] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=125961
[2] http://patchwork.ozlabs.org/patch/1149257/
>
> > return 0;
> > }
> > machine_subsys_initcall(powernv, opal_init);
>
On Wed, 2019-08-21 at 11:08 -0400, Nayna Jain wrote:
> The X.509 certificates trusted by the platform and required to secure boot
> the OS kernel are wrapped in secure variables, which are controlled by
> OPAL.
>
> This patch adds firmware/kernel interface to read and write OPAL secure
> variables based on the unique key.
>
> This support can be enabled using CONFIG_OPAL_SECVAR.
>
> Signed-off-by: Claudio Carvalho <[email protected]>
> Signed-off-by: Nayna Jain <[email protected]>
> ---
> arch/powerpc/include/asm/opal-api.h | 5 +-
> arch/powerpc/include/asm/opal.h | 6 ++
> arch/powerpc/include/asm/secvar.h | 55 ++++++++++
> arch/powerpc/kernel/Makefile | 2 +-
> arch/powerpc/kernel/secvar-ops.c | 25 +++++
> arch/powerpc/platforms/powernv/Kconfig | 6 ++
> arch/powerpc/platforms/powernv/Makefile | 1 +
> arch/powerpc/platforms/powernv/opal-call.c | 3 +
> arch/powerpc/platforms/powernv/opal-secvar.c | 102 +++++++++++++++++++
> arch/powerpc/platforms/powernv/opal.c | 5 +
> 10 files changed, 208 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/include/asm/secvar.h
> create mode 100644 arch/powerpc/kernel/secvar-ops.c
> create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
>
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 383242eb0dea..b238b4f26c5b 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -208,7 +208,10 @@
> #define OPAL_HANDLE_HMI2 166
> #define OPAL_NX_COPROC_INIT 167
> #define OPAL_XIVE_GET_VP_STATE 170
> -#define OPAL_LAST 170
> +#define OPAL_SECVAR_GET 173
> +#define OPAL_SECVAR_GET_NEXT 174
> +#define OPAL_SECVAR_ENQUEUE_UPDATE 175
> +#define OPAL_LAST 175
>
> #define QUIESCE_HOLD 1 /* Spin all calls at entry */
> #define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 57bd029c715e..247adec2375f 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -388,6 +388,12 @@ void opal_powercap_init(void);
> void opal_psr_init(void);
> void opal_sensor_groups_init(void);
Put these with the rest of the OPAL API wrapper prototypes
(i.e. just after opal_nx_coproc_init()) rather than with the
internal functions.
> > +extern int opal_secvar_get(uint64_t k_key, uint64_t k_key_len,
> + uint64_t k_data, uint64_t k_data_size);
> +extern int opal_secvar_get_next(uint64_t k_key, uint64_t k_key_len,
> + uint64_t k_key_size);
> +extern int opal_secvar_enqueue_update(uint64_t k_key, uint64_t k_key_len,
> + uint64_t k_data, uint64_t k_data_size);
Everything in asm/opal.h is intended for consumption by the kernel, so
use a useful kernel type (or annotation) rather than blank uint64_t for
the parameters that are actually pointers. You should also ditch the k_
prefix since it doesn't make much sense having it inside the kernel.
As a general comment, don't use extern on function prototypes. They're
extern by default and, more importantly, it's contrary to the normal
kernel style.
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_OPAL_H */
> diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
> new file mode 100644
> index 000000000000..645654456265
> --- /dev/null
> +++ b/arch/powerpc/include/asm/secvar.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * PowerPC secure variable operations.
> + *
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Nayna Jain <[email protected]>
> + *
> + */
> +#ifndef SECVAR_OPS_H
> +#define SECVAR_OPS_H
> +
> +#include<linux/types.h>
> +#include<linux/errno.h>
missing space
> +
> +struct secvar_operations {
> + int (*get_variable)(const char *key, unsigned long key_len, u8 *data,
> + unsigned long *data_size);
> + int (*get_next_variable)(const char *key, unsigned long *key_len,
> + unsigned long keysize);
> + int (*set_variable)(const char *key, unsigned long key_len, u8 *data,
> + unsigned long data_size);
> +};
Calling them requires writing code like:
secvar_ops->get_variable(blah);
Why not shorten it to:
secvar_ops->get(blah);
> +#ifdef CONFIG_PPC_SECURE_BOOT
> +
> +extern void set_secvar_ops(struct secvar_operations *ops);
> +extern struct secvar_operations *get_secvar_ops(void);
> +
> +#else
> +
> +static inline void set_secvar_ops(struct secvar_operations *ops)
> +{
> +}
> +
> +static inline struct secvar_operations *get_secvar_ops(void)
> +{
> + return NULL;
> +}
> +
> +#endif
> +#ifdef CONFIG_OPAL_SECVAR
> +
> +extern int secvar_init(void);
> +
> +#else
> +
> +static inline int secvar_init(void)
> +{
> + return -EINVAL;
> +}
> +
> +#endif
The init function is always going to be platform specific so having an
opal-specific secvar_init() in a generic header doesn't make much sense
to me.
> +
> +#endif
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 520b1c814197..9041563f1c74 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -157,7 +157,7 @@ endif
> obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o
> obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
>
> -obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o
> +obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o secvar-ops.o
>
> # Disable GCOV, KCOV & sanitizers in odd or sensitive code
> GCOV_PROFILE_prom_init.o := n
> diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c
> new file mode 100644
> index 000000000000..198222499848
> --- /dev/null
> +++ b/arch/powerpc/kernel/secvar-ops.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Nayna Jain <[email protected]>
> + *
> + * secvar-ops.c
> + * - initialize secvar operations for PowerPC Secureboot
> + */
> +
> +#include<stddef.h>
> +#include<asm/secvar.h>
missing space
> +
> +static struct secvar_operations *secvars_ops;
should be const
> +
> +void set_secvar_ops(struct secvar_operations *ops)
> +{
> + if (!ops)
> + secvars_ops = NULL;
> + secvars_ops = ops;
> +}
> +
> +struct secvar_operations *get_secvar_ops(void)
> +{
> + return secvars_ops;
> +}
Is this really any better than just making the ops pointer global?
> diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
> index 850eee860cf2..65b060539b5c 100644
> --- a/arch/powerpc/platforms/powernv/Kconfig
> +++ b/arch/powerpc/platforms/powernv/Kconfig
> @@ -47,3 +47,9 @@ config PPC_VAS
> VAS adapters are found in POWER9 based systems.
>
> If unsure, say N.
> +
> +config OPAL_SECVAR
> + bool "OPAL Secure Variables"
> + depends on PPC_POWERNV
> + help
> + This enables the kernel to access OPAL secure variables.
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index da2e99efbd04..6651c742e530 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
> obj-$(CONFIG_PPC_MEMTRACE) += memtrace.o
> obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o
> obj-$(CONFIG_OCXL_BASE) += ocxl.o
> +obj-$(CONFIG_OPAL_SECVAR) += opal-secvar.o
> diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
> index 29ca523c1c79..93106e867924 100644
> --- a/arch/powerpc/platforms/powernv/opal-call.c
> +++ b/arch/powerpc/platforms/powernv/opal-call.c
> @@ -287,3 +287,6 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar, OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
> OPAL_CALL(opal_sensor_read_u64, OPAL_SENSOR_READ_U64);
> OPAL_CALL(opal_sensor_group_enable, OPAL_SENSOR_GROUP_ENABLE);
> OPAL_CALL(opal_nx_coproc_init, OPAL_NX_COPROC_INIT);
> +OPAL_CALL(opal_secvar_get, OPAL_SECVAR_GET);
> +OPAL_CALL(opal_secvar_get_next, OPAL_SECVAR_GET_NEXT);
> +OPAL_CALL(opal_secvar_enqueue_update, OPAL_SECVAR_ENQUEUE_UPDATE);
> diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
> new file mode 100644
> index 000000000000..b0f97cea7675
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/opal-secvar.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PowerNV code for secure variables
> + *
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Claudio Carvalho <[email protected]>
> + *
> + * APIs to access secure variables managed by OPAL.
> + *
> + */
> +
> +#define pr_fmt(fmt) "secvar: "fmt
> +
> +#include <linux/types.h>
> +#include <asm/opal.h>
> +#include <asm/secvar.h>
> +
> +static bool is_opal_secvar_supported(void)
> +{
> + static bool opal_secvar_supported;
> + static bool initialized;
> +
> + if (initialized)
> + return opal_secvar_supported;
> +
> + if (!opal_check_token(OPAL_SECVAR_GET)
> + || !opal_check_token(OPAL_SECVAR_GET_NEXT)
> + || !opal_check_token(OPAL_SECVAR_ENQUEUE_UPDATE)) {
> + pr_err("OPAL doesn't support secure variables\n");
This should only print an error if OPAL has advertised support for
secure variables through the DT, but doesn't support the OPAL
calls. Otherwise we'll get a spurious error message on any system
running currently released firmware.
> + opal_secvar_supported = false;
> + } else {
> + opal_secvar_supported = true;
> + }
> +
> + initialized = true;
> +
> + return opal_secvar_supported;
> +}
> +
> +static int opal_get_variable(const char *key, unsigned long ksize,
> + u8 *data, unsigned long *dsize)
> +{
> + int rc;
> +
> + if (!is_opal_secvar_supported())
> + return OPAL_UNSUPPORTED;
This should be -ENXIO or -ENOSUPP. OPAL_UNSUPPORTED is an OPAL return
code, not a kernel one. That said, if the firmware doesn't support
secure variables we should never be calling this function anyway since
the ops pointer is never set.
> +
> + if (dsize)
> + *dsize = cpu_to_be64(*dsize);
> +
> + rc = opal_secvar_get(__pa(key), ksize,
> + __pa(data), __pa(dsize));
> +
> + if (dsize)
> + *dsize = be64_to_cpu(*dsize);
> +
> + return rc;
> +}
> +
> +static int opal_get_next_variable(const char *key, unsigned long *keylen,
> + unsigned long keysize)
> +{
> + int rc;
> +
> + if (!is_opal_secvar_supported())
> + return OPAL_UNSUPPORTED;
> +
> + if (keylen)
> + *keylen = cpu_to_be64(*keylen);
> +
> + rc = opal_secvar_get_next(__pa(key), __pa(keylen), keysize);
> +
> + if (keylen)
> + *keylen = be64_to_cpu(*keylen);
> +
> + return rc;
> +}
> +
> +static int opal_set_variable(const char *key, unsigned long ksize, u8 *data,
> + unsigned long dsize)
> +{
> + int rc;
> +
> + if (!is_opal_secvar_supported())
> + return OPAL_UNSUPPORTED;
> +
> + rc = opal_secvar_enqueue_update(__pa(key), ksize, __pa(data), dsize);
> +
> + return rc;
> +}
> +
> +static struct secvar_operations secvar_ops = {
> + .get_variable = opal_get_variable,
> + .get_next_variable = opal_get_next_variable,
> + .set_variable = opal_set_variable,
> +};
should be const
> +int secvar_init(void)
> +{
> + set_secvar_ops(&secvar_ops);
> + return 0;
> +}
Rename this to opal_secvar_init() and put the prototype in
arch/powerpc/platforms/powernv/powernv.h
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index aba443be7daa..ffe6f1cf0830 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -32,6 +32,8 @@
> #include <asm/mce.h>
> #include <asm/imc-pmu.h>
> #include <asm/bug.h>
> +#include <asm/secvar.h>
> +#include <asm/secboot.h>
>
> #include "powernv.h"
>
> @@ -988,6 +990,9 @@ static int __init opal_init(void)
> /* Initialise OPAL Power control interface */
> opal_power_control_init();
>
> + if (is_powerpc_secvar_supported())
> + secvar_init();
> +
The usual pattern here is to have the init function check for support
internally.
Also, is_powerpc_secvar_supported() doesn't appear to be defined
anywhere. Is that supposed to be is_opal_secvar_supported()? Or is this
series supposed to be applied on top of another series?
> return 0;
> }
> machine_subsys_initcall(powernv, opal_init);