Re: [PATCH v4] powerpc/pseries: make max polling consistent for longer H_CALLs
On Wed, 2024-04-17 at 23:12 -0400, Nayna Jain wrote:
> Currently, plpks_confirm_object_flushed() function polls for 5msec in
> total instead of 5sec.
>
> Keep max polling time consistent for all the H_CALLs, which take
> longer
> than expected, to be 5sec. Also, make use of fsleep() everywhere to
> insert delay.
>
> Reported-by: Nageswara R Sastry
> Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform
> KeyStore")
> Signed-off-by: Nayna Jain
> Tested-by: Nageswara R Sastry
Reviewed-by: Andrew Donnellan
> ---
> v4:
> * As per Andrew's feedback, squashed Patch 2 with Patch 1.
> Now it is single patch.
>
> v3:
> * Addition to Patch 1 timeout patch based on Andrew's feedback.
>
> v2:
> * Updated based on feedback from Michael Ellerman
> Replaced usleep_range with fsleep.
> Since there is no more need to specify range, sleep time is
> reverted back to 10 msec.
>
> arch/powerpc/include/asm/plpks.h | 5 ++---
> arch/powerpc/platforms/pseries/plpks.c | 10 +-
> 2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/plpks.h
> b/arch/powerpc/include/asm/plpks.h
> index 23b77027c916..7a84069759b0 100644
> --- a/arch/powerpc/include/asm/plpks.h
> +++ b/arch/powerpc/include/asm/plpks.h
> @@ -44,9 +44,8 @@
> #define PLPKS_MAX_DATA_SIZE 4000
>
> // Timeouts for PLPKS operations
> -#define PLPKS_MAX_TIMEOUT5000 // msec
> -#define PLPKS_FLUSH_SLEEP10 // msec
> -#define PLPKS_FLUSH_SLEEP_RANGE 400
> +#define PLPKS_MAX_TIMEOUT(5 * USEC_PER_SEC)
> +#define PLPKS_FLUSH_SLEEP1 // usec
>
> struct plpks_var {
> char *component;
> diff --git a/arch/powerpc/platforms/pseries/plpks.c
> b/arch/powerpc/platforms/pseries/plpks.c
> index febe18f251d0..4a595493d28a 100644
> --- a/arch/powerpc/platforms/pseries/plpks.c
> +++ b/arch/powerpc/platforms/pseries/plpks.c
> @@ -415,8 +415,7 @@ static int plpks_confirm_object_flushed(struct
> label *label,
> break;
> }
>
> - usleep_range(PLPKS_FLUSH_SLEEP,
> - PLPKS_FLUSH_SLEEP +
> PLPKS_FLUSH_SLEEP_RANGE);
> + fsleep(PLPKS_FLUSH_SLEEP);
> timeout = timeout + PLPKS_FLUSH_SLEEP;
> } while (timeout < PLPKS_MAX_TIMEOUT);
>
> @@ -464,9 +463,10 @@ int plpks_signed_update_var(struct plpks_var
> *var, u64 flags)
>
> continuetoken = retbuf[0];
> if (pseries_status_to_err(rc) == -EBUSY) {
> - int delay_ms = get_longbusy_msecs(rc);
> - mdelay(delay_ms);
> - timeout += delay_ms;
> + int delay_us = get_longbusy_msecs(rc) *
> 1000;
> +
> + fsleep(delay_us);
> + timeout += delay_us;
> }
> rc = pseries_status_to_err(rc);
> } while (rc == -EBUSY && timeout < PLPKS_MAX_TIMEOUT);
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
[PATCH 2/2] MAINTAINERS: Make cxl obsolete
The cxl driver is no longer actively maintained and we intend to remove it
in a future kernel release. Change its status to obsolete, and update the
sysfs ABI documentation accordingly.
Signed-off-by: Andrew Donnellan
---
Documentation/ABI/{testing => obsolete}/sysfs-class-cxl | 3 +++
MAINTAINERS | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
rename Documentation/ABI/{testing => obsolete}/sysfs-class-cxl (99%)
diff --git a/Documentation/ABI/testing/sysfs-class-cxl
b/Documentation/ABI/obsolete/sysfs-class-cxl
similarity index 99%
rename from Documentation/ABI/testing/sysfs-class-cxl
rename to Documentation/ABI/obsolete/sysfs-class-cxl
index cfc48a87706b..8cba1b626985 100644
--- a/Documentation/ABI/testing/sysfs-class-cxl
+++ b/Documentation/ABI/obsolete/sysfs-class-cxl
@@ -1,3 +1,6 @@
+The cxl driver is no longer maintained, and will be removed from the kernel in
+the near future.
+
Please note that attributes that are shared between devices are stored in
the directory pointed to by the symlink device/.
For example, the real path of the attribute /sys/class/cxl/afu0.0s/irqs_max is
diff --git a/MAINTAINERS b/MAINTAINERS
index 34f605498873..5eca2c23fb49 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5770,8 +5770,8 @@ CXL (IBM Coherent Accelerator Processor Interface CAPI)
DRIVER
M: Frederic Barrat
M: Andrew Donnellan
L: linuxppc-dev@lists.ozlabs.org
-S: Supported
-F: Documentation/ABI/testing/sysfs-class-cxl
+S: Obsolete
+F: Documentation/ABI/obsolete/sysfs-class-cxl
F: Documentation/arch/powerpc/cxl.rst
F: arch/powerpc/platforms/powernv/pci-cxl.c
F: drivers/misc/cxl/
--
2.44.0
[PATCH 1/2] MAINTAINERS: Make cxlflash obsolete
The cxlflash driver is no longer actively maintained and we intend to remove it in a future kernel release. Change its status to obsolete. While we're here, Matthew Ochs no longer works at IBM and is no longer in a position to access cxlflash hardware, so remove him from the maintainers list. Signed-off-by: Andrew Donnellan --- MAINTAINERS | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index aea47e04c3a5..34f605498873 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5780,10 +5780,9 @@ F: include/uapi/misc/cxl.h CXLFLASH (IBM Coherent Accelerator Processor Interface CAPI Flash) SCSI DRIVER M: Manoj N. Kumar -M: Matthew R. Ochs M: Uma Krishnan L: linux-s...@vger.kernel.org -S: Supported +S: Obsolete F: Documentation/arch/powerpc/cxlflash.rst F: drivers/scsi/cxlflash/ F: include/uapi/scsi/cxlflash_ioctl.h -- 2.44.0
Re: [PATCH v3 2/2] powerpc/pseries: increase timeout value for plpks_signed_update_var() H_CALL
On Thu, 2024-03-28 at 22:09 -0400, Nayna Jain wrote:
> Signed update H_CALL currently polls PHYP for 5msec. Update this to
> 5sec.
I think this description of the existing code's behaviour is incorrect:
- without your patch #1, the existing code polls for up to 5,000ms
- with your patch #1, the existing code polls for up to 5,000,000ms
(PLPKS_MAX_TIMEOUT is redefined in terms of microseconds, while we
still assume it's in milliseconds).
This patch should just be squashed into patch #1.
Andrew
>
> Signed-off-by: Nayna Jain
> Tested-by: Nageswara R Sastry
> ---
> v3:
> * Addition to Patch 1 timeout patch based on Andrew's feedback.
>
> arch/powerpc/platforms/pseries/plpks.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/plpks.c
> b/arch/powerpc/platforms/pseries/plpks.c
> index bcfcd5acc5c2..4a595493d28a 100644
> --- a/arch/powerpc/platforms/pseries/plpks.c
> +++ b/arch/powerpc/platforms/pseries/plpks.c
> @@ -463,9 +463,10 @@ int plpks_signed_update_var(struct plpks_var
> *var, u64 flags)
>
> continuetoken = retbuf[0];
> if (pseries_status_to_err(rc) == -EBUSY) {
> - int delay_ms = get_longbusy_msecs(rc);
> - mdelay(delay_ms);
> - timeout += delay_ms;
> + int delay_us = get_longbusy_msecs(rc) *
> 1000;
> +
> + fsleep(delay_us);
> + timeout += delay_us;
> }
> rc = pseries_status_to_err(rc);
> } while (rc == -EBUSY && timeout < PLPKS_MAX_TIMEOUT);
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v1] powerpc: Error on assembly warnings
On Tue, 2024-03-26 at 15:44 +1100, Benjamin Gray wrote: > We currently enable -Werror on the arch/powerpc subtree. However this > only catches C warnings. Assembly warnings are logged, but the make > invocation will still succeed. This can allow incorrect syntax such > as > > ori r3, r4, r5 > > to be compiled without catching that the assembler is treating r5 > as the immediate value 5. > > To prevent this in assembly files and inline assembly, add the > -fatal-warnings option to assembler invocations. > > Signed-off-by: Benjamin Gray Seems like a good idea to me! Reviewed-by: Andrew Donnellan Tested-by: Andrew Donnellan > --- > arch/powerpc/Kbuild | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/Kbuild b/arch/powerpc/Kbuild > index 22cd0d55a892..da862e9558bc 100644 > --- a/arch/powerpc/Kbuild > +++ b/arch/powerpc/Kbuild > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > -subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror > +subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror -Wa,-fatal-warnings > +subdir-asflags-$(CONFIG_PPC_WERROR) := -Wa,-fatal-warnings > > obj-y += kernel/ > obj-y += mm/ -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2] powerpc/pseries: fix max polling time in plpks_confirm_object_flushed() function
On Thu, 2024-03-14 at 00:17 -0400, Nayna Jain wrote:
> usleep_range() function takes input time and range in usec. However,
> currently it is assumed in msec in the function
> plpks_confirm_object_flushed().
>
> Fix the total polling time for the object flushing from 5msec to
> 5sec.
>
> Reported-by: Nageswara R Sastry
> Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform
> KeyStore")
> Suggested-by: Michael Ellerman
> Signed-off-by: Nayna Jain
> Tested-by: Nageswara R Sastry
plpks_signed_update_var() also relies on PLPKS_MAX_TIMEOUT, and assumes
it to be in milliseconds rather than microseconds.
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 02/11] cxl: Convert to platform remove callback returning void
On Wed, 2024-02-21 at 10:53 +0100, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which
> makes
> many driver authors wrongly assume it's possible to do error handling
> by
> returning an error code. However the value returned is ignored (apart
> from emitting a warning) and this typically results in resource
> leaks.
>
> To improve here there is a quest to make the remove callback return
> void. In the first step of this quest all drivers are converted to
> .remove_new(), which already returns void. Eventually after all
> drivers
> are converted, .remove_new() will be renamed to .remove().
>
> Trivially convert this driver from always returning zero in the
> remove
> callback to the void returning variant.
>
> Signed-off-by: Uwe Kleine-König
Acked-by: Andrew Donnellan
> ---
> drivers/misc/cxl/of.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
> index 25ce725035e7..bcc005dff1c0 100644
> --- a/drivers/misc/cxl/of.c
> +++ b/drivers/misc/cxl/of.c
> @@ -431,7 +431,7 @@ int cxl_of_read_adapter_properties(struct cxl
> *adapter, struct device_node *np)
> return 0;
> }
>
> -static int cxl_of_remove(struct platform_device *pdev)
> +static void cxl_of_remove(struct platform_device *pdev)
> {
> struct cxl *adapter;
> int afu;
> @@ -441,7 +441,6 @@ static int cxl_of_remove(struct platform_device
> *pdev)
> cxl_guest_remove_afu(adapter->afu[afu]);
>
> cxl_guest_remove_adapter(adapter);
> - return 0;
> }
>
> static void cxl_of_shutdown(struct platform_device *pdev)
> @@ -501,6 +500,6 @@ struct platform_driver cxl_of_driver = {
> .owner = THIS_MODULE
> },
> .probe = cxl_of_probe,
> - .remove = cxl_of_remove,
> + .remove_new = cxl_of_remove,
> .shutdown = cxl_of_shutdown,
> };
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2] cxl: Fix null pointer dereference in cxl_get_fd
On Fri, 2024-01-12 at 13:49 +0800, Kunwu Chan wrote: > +err: > + if (rc < 0) > + return ERR_PTR(rc); > return NULL; I don't think there's a way for this NULL to ever get returned? Apart from that it looks good: Reviewed-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] powerpc/powernv: Add a null pointer check to scom_debug_init_one
On Fri, 2023-12-08 at 16:59 +0800, Kunwu Chan wrote:
> kasprintf() returns a pointer to dynamically allocated memory
> which can be NULL upon failure.
> Add a null pointer check, and release 'ent' to avoid memory leaks.
>
> Fixes: bfd2f0d49aef ("powerpc/powernv: Get rid of old scom_controller
> abstraction")
[+ robh]
This commit just reshuffled around some existing code. The commit that
appears to have added this is actually b7c670d673d1 ("powerpc: Convert
to using %pOF instead of full_name").
> Cc: Kunwu Chan
> Signed-off-by: Kunwu Chan
> ---
> arch/powerpc/platforms/powernv/opal-xscom.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c
> b/arch/powerpc/platforms/powernv/opal-xscom.c
> index 262cd6fac907..748c2b97fa53 100644
> --- a/arch/powerpc/platforms/powernv/opal-xscom.c
> +++ b/arch/powerpc/platforms/powernv/opal-xscom.c
> @@ -165,6 +165,11 @@ static int scom_debug_init_one(struct dentry
> *root, struct device_node *dn,
> ent->chip = chip;
> snprintf(ent->name, 16, "%08x", chip);
> ent->path.data = (void *)kasprintf(GFP_KERNEL, "%pOF", dn);
> + if (!ent->path.data) {
> + kfree(ent);
> + return -ENOMEM;
The caller of this function (scom_debug_init()) uses a bitwise OR to
aggregate the return codes of multiple calls to scom_debug_init_one().
This doesn't really work for returning specific error codes, so I'd
just return -1 here (or change the way the return codes are handled on
the caller side).
> + }
> +
> ent->path.size = strlen((char *)ent->path.data);
>
> dir = debugfs_create_dir(ent->name, root);
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 2/4] eventfd: simplify eventfd_signal()
On Wed, 2023-11-22 at 13:48 +0100, Christian Brauner wrote:
> Ever since the evenfd type was introduced back in 2007 in commit
> e1ad7468c77d ("signal/timer/event: eventfd core") the
> eventfd_signal()
> function only ever passed 1 as a value for @n. There's no point in
> keeping that additional argument.
>
> Signed-off-by: Christian Brauner
Acked-by: Andrew Donnellan # ocxl
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] misc: ocxl: main: Remove unnecessary ‘0’ values from rc
On Mon, 2023-11-13 at 09:52 +0800, Li kunyu wrote:
> rc is assigned first, so it does not need to initialize the
> assignment.
>
> Signed-off-by: Li kunyu
I don't have strong feelings about whether to get rid of unnecessary
initialisations, but most of the code doesn't do it, so for
consistency:
Acked-by: Andrew Donnellan
> ---
> drivers/misc/ocxl/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/ocxl/main.c b/drivers/misc/ocxl/main.c
> index ef73cf35dda2b..658974143c3cc 100644
> --- a/drivers/misc/ocxl/main.c
> +++ b/drivers/misc/ocxl/main.c
> @@ -7,7 +7,7 @@
>
> static int __init init_ocxl(void)
> {
> - int rc = 0;
> + int rc;
>
> if (!tlbie_capable)
> return -EINVAL;
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] misc: ocxl: link: Remove unnecessary (void*) conversions
On Mon, 2023-11-13 at 09:45 +0800, Li zeming wrote:
> The link pointer does not need to cast the type.
>
> Signed-off-by: Li zeming
Acked-by: Andrew Donnellan
> ---
> drivers/misc/ocxl/link.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index c06c699c0e7b1..03402203cacdb 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -188,7 +188,7 @@ static void xsl_fault_handler_bh(struct
> work_struct *fault_work)
>
> static irqreturn_t xsl_fault_handler(int irq, void *data)
> {
> - struct ocxl_link *link = (struct ocxl_link *) data;
> + struct ocxl_link *link = data;
> struct spa *spa = link->spa;
> u64 dsisr, dar, pe_handle;
> struct pe_data *pe_data;
> @@ -483,7 +483,7 @@ static void release_xsl(struct kref *ref)
>
> void ocxl_link_release(struct pci_dev *dev, void *link_handle)
> {
> - struct ocxl_link *link = (struct ocxl_link *) link_handle;
> + struct ocxl_link *link = link_handle;
>
> mutex_lock(_list_lock);
> kref_put(>ref, release_xsl);
> @@ -540,7 +540,7 @@ int ocxl_link_add_pe(void *link_handle, int
> pasid, u32 pidr, u32 tidr,
> void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
> void *xsl_err_data)
> {
> - struct ocxl_link *link = (struct ocxl_link *) link_handle;
> + struct ocxl_link *link = link_handle;
> struct spa *spa = link->spa;
> struct ocxl_process_element *pe;
> int pe_handle, rc = 0;
> @@ -630,7 +630,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_add_pe);
>
> int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
> {
> - struct ocxl_link *link = (struct ocxl_link *) link_handle;
> + struct ocxl_link *link = link_handle;
> struct spa *spa = link->spa;
> struct ocxl_process_element *pe;
> int pe_handle, rc;
> @@ -666,7 +666,7 @@ int ocxl_link_update_pe(void *link_handle, int
> pasid, __u16 tid)
>
> int ocxl_link_remove_pe(void *link_handle, int pasid)
> {
> - struct ocxl_link *link = (struct ocxl_link *) link_handle;
> + struct ocxl_link *link = link_handle;
> struct spa *spa = link->spa;
> struct ocxl_process_element *pe;
> struct pe_data *pe_data;
> @@ -752,7 +752,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_remove_pe);
>
> int ocxl_link_irq_alloc(void *link_handle, int *hw_irq)
> {
> - struct ocxl_link *link = (struct ocxl_link *) link_handle;
> + struct ocxl_link *link = link_handle;
> int irq;
>
> if (atomic_dec_if_positive(>irq_available) < 0)
> @@ -771,7 +771,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_irq_alloc);
>
> void ocxl_link_free_irq(void *link_handle, int hw_irq)
> {
> - struct ocxl_link *link = (struct ocxl_link *) link_handle;
> + struct ocxl_link *link = link_handle;
>
> xive_native_free_irq(hw_irq);
> atomic_inc(>irq_available);
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] misc: ocxl: afu_irq: Remove unnecessary (void*) conversions
On Mon, 2023-11-13 at 09:22 +0800, Li zeming wrote:
> The irq pointer does not need to cast the type.
>
> Signed-off-by: Li zeming
Acked-by: Andrew Donnellan
> ---
> drivers/misc/ocxl/afu_irq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/ocxl/afu_irq.c
> b/drivers/misc/ocxl/afu_irq.c
> index a06920b7e049a..36f7379b8e2de 100644
> --- a/drivers/misc/ocxl/afu_irq.c
> +++ b/drivers/misc/ocxl/afu_irq.c
> @@ -57,7 +57,7 @@ EXPORT_SYMBOL_GPL(ocxl_irq_set_handler);
>
> static irqreturn_t afu_irq_handler(int virq, void *data)
> {
> - struct afu_irq *irq = (struct afu_irq *) data;
> + struct afu_irq *irq = data;
>
> trace_ocxl_afu_irq_receive(virq);
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] misc: ocxl: context: Remove unnecessary (void*) conversions
On Mon, 2023-11-13 at 09:15 +0800, Li zeming wrote:
> The ctx pointer does not need to cast the type.
>
> Signed-off-by: Li zeming
Acked-by: Andrew Donnellan
> ---
> drivers/misc/ocxl/context.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/ocxl/context.c
> b/drivers/misc/ocxl/context.c
> index 7f83116ae11a6..cded7d1caf328 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -55,7 +55,7 @@ EXPORT_SYMBOL_GPL(ocxl_context_alloc);
> */
> static void xsl_fault_error(void *data, u64 addr, u64 dsisr)
> {
> - struct ocxl_context *ctx = (struct ocxl_context *) data;
> + struct ocxl_context *ctx = data;
>
> mutex_lock(>xsl_error_lock);
> ctx->xsl_error.addr = addr;
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 5/5] powerpc/rtas: Remove 'extern' from function declarations in rtas.h
On Tue, 2023-11-14 at 11:22 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> This header occasionally gains new function declarations without the
> leading extern in accordance with current style rules. Leaving the
> legacy externs in place is making the header more difficult to read
> over time because of the inconsistency. Remove them, fixing up
> checkpatch issues with unnamed parameters (rtas_call) and bracket
> alignment (early_init_dt_scan_rtas) that get raised as a result of
> touching the code.
>
> Signed-off-by: Nathan Lynch '
LGTM
Reviewed-by: Andrew Donnellan
> ---
> arch/powerpc/include/asm/rtas.h | 53 ---
> --
> 1 file changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h
> b/arch/powerpc/include/asm/rtas.h
> index 1bed6be8ada3..a7110ed52e25 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -408,42 +408,41 @@ static inline bool
> rtas_function_implemented(const rtas_fn_handle_t handle)
> {
> return rtas_function_token(handle) != RTAS_UNKNOWN_SERVICE;
> }
> -extern int rtas_token(const char *service);
> -extern int rtas_call(int token, int, int, int *, ...);
> +int rtas_token(const char *service);
> +int rtas_call(int token, int nargs, int nret, int *outputs, ...);
> void rtas_call_unlocked(struct rtas_args *args, int token, int
> nargs,
> int nret, ...);
> -extern void __noreturn rtas_restart(char *cmd);
> -extern void rtas_power_off(void);
> -extern void __noreturn rtas_halt(void);
> -extern void rtas_os_term(char *str);
> +void __noreturn rtas_restart(char *cmd);
> +void rtas_power_off(void);
> +void __noreturn rtas_halt(void);
> +void rtas_os_term(char *str);
> void rtas_activate_firmware(void);
> -extern int rtas_get_sensor(int sensor, int index, int *state);
> -extern int rtas_get_sensor_fast(int sensor, int index, int *state);
> -extern int rtas_get_power_level(int powerdomain, int *level);
> -extern int rtas_set_power_level(int powerdomain, int level, int
> *setlevel);
> -extern bool rtas_indicator_present(int token, int *maxindex);
> -extern int rtas_set_indicator(int indicator, int index, int
> new_value);
> -extern int rtas_set_indicator_fast(int indicator, int index, int
> new_value);
> -extern void rtas_progress(char *s, unsigned short hex);
> +int rtas_get_sensor(int sensor, int index, int *state);
> +int rtas_get_sensor_fast(int sensor, int index, int *state);
> +int rtas_get_power_level(int powerdomain, int *level);
> +int rtas_set_power_level(int powerdomain, int level, int *setlevel);
> +bool rtas_indicator_present(int token, int *maxindex);
> +int rtas_set_indicator(int indicator, int index, int new_value);
> +int rtas_set_indicator_fast(int indicator, int index, int
> new_value);
> +void rtas_progress(char *s, unsigned short hex);
> int rtas_ibm_suspend_me(int *fw_status);
> int rtas_error_rc(int rtas_rc);
>
> struct rtc_time;
> -extern time64_t rtas_get_boot_time(void);
> -extern void rtas_get_rtc_time(struct rtc_time *rtc_time);
> -extern int rtas_set_rtc_time(struct rtc_time *rtc_time);
> +time64_t rtas_get_boot_time(void);
> +void rtas_get_rtc_time(struct rtc_time *rtc_time);
> +int rtas_set_rtc_time(struct rtc_time *rtc_time);
>
> -extern unsigned int rtas_busy_delay_time(int status);
> +unsigned int rtas_busy_delay_time(int status);
> bool rtas_busy_delay(int status);
>
> -extern int early_init_dt_scan_rtas(unsigned long node,
> - const char *uname, int depth, void *data);
> +int early_init_dt_scan_rtas(unsigned long node, const char *uname,
> int depth, void *data);
>
> -extern void pSeries_log_error(char *buf, unsigned int err_type, int
> fatal);
> +void pSeries_log_error(char *buf, unsigned int err_type, int fatal);
>
> #ifdef CONFIG_PPC_PSERIES
> extern time64_t last_rtas_event;
> -extern int clobbering_unread_rtas_event(void);
> +int clobbering_unread_rtas_event(void);
> int rtas_syscall_dispatch_ibm_suspend_me(u64 handle);
> #else
> static inline int clobbering_unread_rtas_event(void) { return 0; }
> @@ -454,7 +453,7 @@ static inline int
> rtas_syscall_dispatch_ibm_suspend_me(u64 handle)
> #endif
>
> #ifdef CONFIG_PPC_RTAS_DAEMON
> -extern void rtas_cancel_event_scan(void);
> +void rtas_cancel_event_scan(void);
> #else
> static inline void rtas_cancel_event_scan(void) { }
> #endif
> @@ -479,7 +478,7 @@ static inline void rtas_cancel_event_scan(void) {
> }
> * for all rtas calls that require an error buffer argument.
> * This includes 'check-exception' and 'rtas-last-error'.
> */
> -extern int rtas_get_error_log_max(void);
>
Re: [PATCH v2 4/5] powerpc/rtas: Remove trailing space
On Tue, 2023-11-14 at 11:22 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> Use scripts/cleanfile to remove instances of trailing space in the
> core RTAS code and header.
>
> Signed-off-by: Nathan Lynch
Thanks for the cleanup, LGTM.
Reviewed-by: Andrew Donnellan
> ---
> arch/powerpc/include/asm/rtas.h | 6 +++---
> arch/powerpc/kernel/rtas.c | 18 +-
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h
> b/arch/powerpc/include/asm/rtas.h
> index 2365668fc13e..1bed6be8ada3 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -268,7 +268,7 @@ typedef struct {
> #define RTAS_TYPE_DEALLOC0xE3
> #define RTAS_TYPE_DUMP 0xE4
> #define RTAS_TYPE_HOTPLUG0xE5
> -/* I don't add PowerMGM events right now, this is a different topic
> */
> +/* I don't add PowerMGM events right now, this is a different topic
> */
> #define RTAS_TYPE_PMGM_POWER_SW_ON 0x60
> #define RTAS_TYPE_PMGM_POWER_SW_OFF 0x61
> #define RTAS_TYPE_PMGM_LID_OPEN 0x62
> @@ -461,7 +461,7 @@ static inline void rtas_cancel_event_scan(void) {
> }
>
> /* Error types logged. */
> #define ERR_FLAG_ALREADY_LOGGED 0x0
> -#define ERR_FLAG_BOOT0x1 /* log was pulled from NVRAM
> on boot */
> +#define ERR_FLAG_BOOT0x1 /* log was pulled from NVRAM
> on boot */
> #define ERR_TYPE_RTAS_LOG0x2 /* from rtas event-scan */
> #define ERR_TYPE_KERNEL_PANIC0x4 /* from die()/panic() */
> #define ERR_TYPE_KERNEL_PANIC_GZ 0x8 /* ditto, compressed */
> @@ -471,7 +471,7 @@ static inline void rtas_cancel_event_scan(void) {
> }
> (ERR_TYPE_RTAS_LOG | ERR_TYPE_KERNEL_PANIC |
> ERR_TYPE_KERNEL_PANIC_GZ)
>
> #define RTAS_DEBUG KERN_DEBUG "RTAS: "
> -
> +
> #define RTAS_ERROR_LOG_MAX 2048
>
> /*
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index b5b340a91157..c49f078382a9 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -670,7 +670,7 @@ static void call_rtas_display_status_delay(char
> c)
> static int pending_newline = 0; /* did last write end with
> unprinted newline? */
> static int width = 16;
>
> - if (c == '\n') {
> + if (c == '\n') {
> while (width-- > 0)
> call_rtas_display_status(' ');
> width = 16;
> @@ -680,7 +680,7 @@ static void call_rtas_display_status_delay(char
> c)
> if (pending_newline) {
> call_rtas_display_status('\r');
> call_rtas_display_status('\n');
> - }
> + }
> pending_newline = 0;
> if (width--) {
> call_rtas_display_status(c);
> @@ -820,7 +820,7 @@ void rtas_progress(char *s, unsigned short hex)
> else
> rtas_call(display_character, 1, 1, NULL,
> '\r');
> }
> -
> +
> if (row_width)
> width = row_width[current_line];
> else
> @@ -840,9 +840,9 @@ void rtas_progress(char *s, unsigned short hex)
> spin_unlock(_lock);
> return;
> }
> -
> +
> /* RTAS wants CR-LF, not just LF */
> -
> +
> if (*os == '\n') {
> rtas_call(display_character, 1, 1,
> NULL, '\r');
> rtas_call(display_character, 1, 1,
> NULL, '\n');
> @@ -852,7 +852,7 @@ void rtas_progress(char *s, unsigned short hex)
> */
> rtas_call(display_character, 1, 1,
> NULL, *os);
> }
> -
> +
> if (row_width)
> width = row_width[current_line];
> else
> @@ -861,15 +861,15 @@ void rtas_progress(char *s, unsigned short hex)
> width--;
> rtas_call(display_character, 1, 1, NULL,
> *os);
> }
> -
> +
> os++;
> -
> +
> /* if we overwrite the screen length */
> if (width <= 0)
> while ((*os != 0) && (*os != '\n') && (*os
> != '\r'))
> os++;
> }
> -
> +
> spin_unlock(_lock);
> }
> EXPORT_SYMBOL_GPL(rtas_progress);/* needed by
> rtas_flash module */
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 3/5] powerpc/rtas: Move post_mobility_fixup() declaration to pseries
On Tue, 2023-11-14 at 11:22 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> This is a pseries-specific function declaration that doesn't belong
> in
> rtas.h. Move it to the pseries platform code and adjust
> pseries/suspend.c accordingly.
>
> Signed-off-by: Nathan Lynch
This looks correct to me (the other user is in mobility.c which already
has the header file included).
Reviewed-by: Andrew Donnellan
> ---
> arch/powerpc/include/asm/rtas.h | 1 -
> arch/powerpc/platforms/pseries/pseries.h | 1 +
> arch/powerpc/platforms/pseries/suspend.c | 1 +
> 3 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h
> b/arch/powerpc/include/asm/rtas.h
> index c6568a647cd0..2365668fc13e 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -444,7 +444,6 @@ extern void pSeries_log_error(char *buf, unsigned
> int err_type, int fatal);
> #ifdef CONFIG_PPC_PSERIES
> extern time64_t last_rtas_event;
> extern int clobbering_unread_rtas_event(void);
> -extern void post_mobility_fixup(void);
> int rtas_syscall_dispatch_ibm_suspend_me(u64 handle);
> #else
> static inline int clobbering_unread_rtas_event(void) { return 0; }
> diff --git a/arch/powerpc/platforms/pseries/pseries.h
> b/arch/powerpc/platforms/pseries/pseries.h
> index 8376f03f932a..1c2d736f600d 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -55,6 +55,7 @@ extern int dlpar_detach_node(struct device_node *);
> extern int dlpar_acquire_drc(u32 drc_index);
> extern int dlpar_release_drc(u32 drc_index);
> extern int dlpar_unisolate_drc(u32 drc_index);
> +void post_mobility_fixup(void);
>
> void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog);
> int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_errlog);
> diff --git a/arch/powerpc/platforms/pseries/suspend.c
> b/arch/powerpc/platforms/pseries/suspend.c
> index 5c43435472cc..382003dfdb9a 100644
> --- a/arch/powerpc/platforms/pseries/suspend.c
> +++ b/arch/powerpc/platforms/pseries/suspend.c
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include "pseries.h"
>
> static struct device suspend_dev;
>
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 2/5] powerpc/rtas: Remove unused rtas_service_present()
On Tue, 2023-11-14 at 11:22 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> rtas_service_present() has no more users.
>
> rtas_function_implemented() is now the appropriate API for
> determining
> whether a given RTAS function is available to call.
>
> Signed-off-by: Nathan Lynch
grep confirms this.
Reviewed-by: Andrew Donnellan
> ---
> arch/powerpc/include/asm/rtas.h | 1 -
> arch/powerpc/kernel/rtas.c | 5 -
> 2 files changed, 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h
> b/arch/powerpc/include/asm/rtas.h
> index 3bf7f0a4b07e..c6568a647cd0 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -409,7 +409,6 @@ static inline bool
> rtas_function_implemented(const rtas_fn_handle_t handle)
> return rtas_function_token(handle) != RTAS_UNKNOWN_SERVICE;
> }
> extern int rtas_token(const char *service);
> -extern int rtas_service_present(const char *service);
> extern int rtas_call(int token, int, int, int *, ...);
> void rtas_call_unlocked(struct rtas_args *args, int token, int
> nargs,
> int nret, ...);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index eddc031c4b95..b5b340a91157 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -900,11 +900,6 @@ int rtas_token(const char *service)
> }
> EXPORT_SYMBOL_GPL(rtas_token);
>
> -int rtas_service_present(const char *service)
> -{
> - return rtas_token(service) != RTAS_UNKNOWN_SERVICE;
> -}
> -
> #ifdef CONFIG_RTAS_ERROR_LOGGING
>
> static u32 rtas_error_log_max __ro_after_init = RTAS_ERROR_LOG_MAX;
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 1/5] powerpc/rtas: Drop declaration of undefined call_rtas() function
On Tue, 2023-11-14 at 11:22 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> The call_rtas() function has never been a part of arch/powerpc, and
> its implementation was removed from arch/ppc by commit 0a26b1364f14
> ("ppc: Remove CHRP, POWER3 and POWER4 support from arch/ppc").
>
> Signed-off-by: Nathan Lynch
grep confirms this.
Reviewed-by: Andrew Donnellan
> ---
> arch/powerpc/include/asm/rtas.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h
> b/arch/powerpc/include/asm/rtas.h
> index c697c3c74694..3bf7f0a4b07e 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -542,8 +542,6 @@ static inline void pSeries_coalesce_init(void) {
> }
> static inline void rtas_initialize(void) { }
> #endif
>
> -extern int call_rtas(const char *, int, int, unsigned long *, ...);
> -
> #ifdef CONFIG_HV_PERF_CTRS
> void read_24x7_sys_info(void);
> #else
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] cxl: Drop unused detach_spa()
On Wed, 2023-08-23 at 14:48 +1000, Michael Ellerman wrote:
> Clang warns:
> drivers/misc/cxl/native.c:272:20: error: unused function
> 'detach_spa' [-Werror,-Wunused-function]
>
> It was created as part of some refactoring in commit 05155772f642
> ("cxl:
> Allocate and release the SPA with the AFU"), but has never been
> called
> in its current form. Drop it.
>
> Signed-off-by: Michael Ellerman
Indeed it looks unused.
Acked-by: Andrew Donnellan
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] ocxl: Use pci_dev_id() to simplify the code
On Fri, 2023-08-11 at 18:20 +0800, Zheng Zengkai wrote: > PCI core API pci_dev_id() can be used to get the BDF number for a pci > device. We don't need to compose it mannually. Use pci_dev_id() to > simplify the code a little bit. > > Signed-off-by: Zheng Zengkai Acked-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 2/2] ocxl: use pci_find_next_dvsec_capability() to simplify the code
On Mon, 2023-08-07 at 11:18 +0800, Xiongfeng Wang wrote: > PCI core add pci_find_next_dvsec_capability() to query the next > DVSEC. > We can use that core API to simplify the code. Also remove the unused > macros. > > Signed-off-by: Xiongfeng Wang Reviewed-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 1/2] PCI: Add pci_find_next_dvsec_capability to find next designated VSEC
On Mon, 2023-08-07 at 11:18 +0800, Xiongfeng Wang wrote:
> Some devices may have several DVSEC(Designated Vendor-Specific
> Extended
> Capability) entries with the same DVSEC ID. Add
> pci_find_next_dvsec_capability() to find them all.
>
> Signed-off-by: Xiongfeng Wang
>
Reviewed-by: Andrew Donnellan
> ---
> drivers/pci/pci.c | 37 +
> include/linux/pci.h | 2 ++
> 2 files changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 60230da957e0..3455ca7306ae 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -749,35 +749,48 @@ u16 pci_find_vsec_capability(struct pci_dev
> *dev, u16 vendor, int cap)
> EXPORT_SYMBOL_GPL(pci_find_vsec_capability);
>
> /**
> - * pci_find_dvsec_capability - Find DVSEC for vendor
> + * pci_find_next_dvsec_capability - Find next DVSEC for vendor
> * @dev: PCI device to query
> + * @start: address at which to start looking (0 to start at
> beginning of list)
> * @vendor: Vendor ID to match for the DVSEC
> * @dvsec: Designated Vendor-specific capability ID
> *
> - * If DVSEC has Vendor ID @vendor and DVSEC ID @dvsec return the
> capability
> - * offset in config space; otherwise return 0.
> + * Returns the address of the next DVSEC if the DVSEC has Vendor ID
> @vendor and
> + * DVSEC ID @dvsec; otherwise return 0. DVSEC can occur several
> times with the
> + * same DVSEC ID for some devices, and this provides a way to find
> them all.
> */
> -u16 pci_find_dvsec_capability(struct pci_dev *dev, u16 vendor, u16
> dvsec)
> +u16 pci_find_next_dvsec_capability(struct pci_dev *dev, u16 start,
> u16 vendor,
> + u16 dvsec)
> {
> - int pos;
> + u16 pos = start;
>
> - pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DVSEC);
> - if (!pos)
> - return 0;
> -
> - while (pos) {
> + while ((pos = pci_find_next_ext_capability(dev, pos,
> +
> PCI_EXT_CAP_ID_DVSEC))) {
> u16 v, id;
>
> pci_read_config_word(dev, pos + PCI_DVSEC_HEADER1,
> );
> pci_read_config_word(dev, pos + PCI_DVSEC_HEADER2,
> );
> if (vendor == v && dvsec == id)
> return pos;
> -
> - pos = pci_find_next_ext_capability(dev, pos,
> PCI_EXT_CAP_ID_DVSEC);
> }
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(pci_find_next_dvsec_capability);
> +
> +/**
> + * pci_find_dvsec_capability - Find DVSEC for vendor
> + * @dev: PCI device to query
> + * @vendor: Vendor ID to match for the DVSEC
> + * @dvsec: Designated Vendor-specific capability ID
> + *
> + * If DVSEC has Vendor ID @vendor and DVSEC ID @dvsec return the
> capability
> + * offset in config space; otherwise return 0.
> + */
> +u16 pci_find_dvsec_capability(struct pci_dev *dev, u16 vendor, u16
> dvsec)
> +{
> + return pci_find_next_dvsec_capability(dev, 0, vendor, dvsec);
> +}
> EXPORT_SYMBOL_GPL(pci_find_dvsec_capability);
>
> /**
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c69a2cc1f412..82bb905daf72 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1168,6 +1168,8 @@ u16 pci_find_next_ext_capability(struct pci_dev
> *dev, u16 pos, int cap);
> struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
> u16 pci_find_vsec_capability(struct pci_dev *dev, u16 vendor, int
> cap);
> u16 pci_find_dvsec_capability(struct pci_dev *dev, u16 vendor, u16
> dvsec);
> +u16 pci_find_next_dvsec_capability(struct pci_dev *dev, u16 start,
> u16 vendor,
> + u16 dvsec);
>
> u64 pci_get_dsn(struct pci_dev *dev);
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [RFC PATCH] cxl: Use pci_find_vsec_capability() to simplify the code
On Fri, 2023-08-04 at 15:56 +0800, Xiongfeng Wang wrote:
> PCI core add pci_find_vsec_capability() to query VSEC. We can use
> that
> core API to simplify the code.
>
> The only logical change is that pci_find_vsec_capability check the
> Vendor ID before finding the VSEC.
>
> PCI spec rev 5.0 says in 7.9.5.2 Vendor-Specific Header:
> VSEC ID - This field is a vendor-defined ID number that indicates
> the
> nature and format of the VSEC structure
> Software must qualify the Vendor ID before interpreting this field.
>
> Signed-off-by: Xiongfeng Wang
LGTM
The cxl driver doesn't currently bind to any devices that don't have an
IBM vendor ID, and it's very unlikely to in future. If that ever
changes, this will of course need to be updated accordingly.
Reviewed-by: Andrew Donnellan
> ---
> drivers/misc/cxl/pci.c | 12 ++--
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 0ff944860dda..f3108977755d 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -150,16 +150,8 @@ static inline resource_size_t p2_size(struct
> pci_dev *dev)
>
> static int find_cxl_vsec(struct pci_dev *dev)
> {
> - int vsec = 0;
> - u16 val;
> -
> - while ((vsec = pci_find_next_ext_capability(dev, vsec,
> PCI_EXT_CAP_ID_VNDR))) {
> - pci_read_config_word(dev, vsec + 0x4, );
> - if (val == CXL_PCI_VSEC_ID)
> - return vsec;
> - }
> - return 0;
> -
> + return pci_find_vsec_capability(dev, PCI_VENDOR_ID_IBM,
> + CXL_PCI_VSEC_ID);
> }
>
> static void dump_cxl_config_space(struct pci_dev *dev)
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
[PATCH] Revert "powerpc/64s: Remove support for ELFv1 little endian userspace"
This reverts commit 606787fed7268feb256957872586370b56af697a.
ELFv1 with LE has never been a thing, and people who try to make ELFv1 LE
binaries are maniacs who need to be stopped, but unfortunately there are
ELFv1 LE binaries out there in the wild.
One such binary is the ppc64el (as Debian calls it) helper for
arch-test[0], a tool for detecting architectures that can be executed on a
given machine by means of attempting to execute helper binaries compiled
for each architecture and seeing which binaries succeed and fail. The
helpers are small snippets of assembly, and the ppc64el assembly doesn't
include the right directives to generate an ELFv2 binary.
This results in arch-test incorrectly determining that a ppc64el kernel
can't execute a ppc64el userspace, which in turn means that a number of
developer tools such as debootstrap will break (assuming arch-test is
installed).
[0] https://github.com/kilobyte/arch-test
Signed-off-by: Andrew Donnellan
Cc: Nicholas Piggin
Cc: Naveen N Rao
Cc: Christophe Leroy
Cc: Adam Borowski
---
arch/powerpc/include/asm/elf.h | 6 --
arch/powerpc/include/asm/thread_info.h | 6 +-
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index a26ca097d032..79f1c480b5eb 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -12,14 +12,8 @@
/*
* This is used to ensure we don't load something for the wrong architecture.
- * 64le only supports ELFv2 64-bit binaries (64be supports v1 and v2).
*/
-#if defined(CONFIG_PPC64) && defined(CONFIG_CPU_LITTLE_ENDIAN)
-#define elf_check_arch(x) (((x)->e_machine == ELF_ARCH) && \
- (((x)->e_flags & 0x3) == 0x2))
-#else
#define elf_check_arch(x) ((x)->e_machine == ELF_ARCH)
-#endif
#define compat_elf_check_arch(x) ((x)->e_machine == EM_PPC)
#define CORE_DUMP_USE_REGSET
diff --git a/arch/powerpc/include/asm/thread_info.h
b/arch/powerpc/include/asm/thread_info.h
index bc5d39a835fe..bf5dde1a4114 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -183,13 +183,9 @@ static inline bool test_thread_local_flags(unsigned int
flags)
#define clear_tsk_compat_task(tsk) do { } while (0)
#endif
-#ifdef CONFIG_PPC64
-#ifdef CONFIG_CPU_BIG_ENDIAN
+#if defined(CONFIG_PPC64)
#define is_elf2_task() (test_thread_flag(TIF_ELF2ABI))
#else
-#define is_elf2_task() (1)
-#endif
-#else
#define is_elf2_task() (0)
#endif
--
2.41.0
Re: [PATCH] misc: Explicitly include correct DT includes
On Fri, 2023-07-14 at 11:47 -0600, Rob Herring wrote: > The DT of_device.h and of_platform.h date back to the separate > of_platform_bus_type before it as merged into the regular platform > bus. > As part of that merge prepping Arm DT support 13 years ago, they > "temporarily" include each other. They also include platform_device.h > and of.h. As a result, there's a pretty much random mix of those > include > files used throughout the tree. In order to detangle these headers > and > replace the implicit includes with struct declarations, users need to > explicitly include the correct includes. > > Signed-off-by: Rob Herring Acked-by: Andrew Donnellan # cxl -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v3 02/12] powerpc/ptrace: Add missing include
On Fri, 2023-05-19 at 15:02 +1000, Benjamin Gray wrote: > ptrace-decl.h uses user_regset_get2_fn (among other things) from > regset.h. While all current users of ptrace-decl.h include regset.h > before it anyway, it adds an implicit ordering dependency and breaks > source tooling that tries to inspect ptrace-decl.h by itself. > > Signed-off-by: Benjamin Gray > Reviewed-by: Russell Currey Reviewed-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v3 01/12] powerpc/book3s: Add missing include
On Fri, 2023-05-19 at 15:02 +1000, Benjamin Gray wrote: > The functions here use struct thread_struct fields, so need to import > the full definition from . The header > that defines current only forward declares struct thread_struct. AFAICT, struct thread_struct is defined in arch/powerpc/include/asm/processor.h? I think you mean struct task_struct (which is what's forward declared in asm/current.h, not thread_struct). > > Failing to include this header leads to a compilation > error when a translation unit does not also include > indirectly. > > Signed-off-by: Benjamin Gray > Reviewed-by: Nicholas Piggin > Reviewed-by: Russell Currey > > --- > > v3: * Add ruscur reviewed-by > v1: * Add npiggin reviewed-by > --- > arch/powerpc/include/asm/book3s/64/kup.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/powerpc/include/asm/book3s/64/kup.h > b/arch/powerpc/include/asm/book3s/64/kup.h > index 54cf46808157..84c09e546115 100644 > --- a/arch/powerpc/include/asm/book3s/64/kup.h > +++ b/arch/powerpc/include/asm/book3s/64/kup.h > @@ -194,6 +194,7 @@ > #else /* !__ASSEMBLY__ */ > > #include > +#include > > DECLARE_STATIC_KEY_FALSE(uaccess_flush_key); > -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 4/4] powerpc/pseries: update SED for PLPKS api changes
On Fri, 2023-05-05 at 14:44 -0500, gjo...@linux.vnet.ibm.com wrote:
> From: Greg Joyce
>
> Changes to the PLPKS API require minor updates to the SED Opal
> PLPKS keystore code.
>
> Signed-off-by: Greg Joyce
[+ Nayna]
This patch will need to be squashed with patch 2.
> ---
> arch/powerpc/platforms/pseries/Kconfig | 6 +
> arch/powerpc/platforms/pseries/Makefile | 2 +-
> .../powerpc/platforms/pseries/plpks_sed_ops.c | 22 +
> --
> block/Kconfig | 1 +
> 4 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/Kconfig
> b/arch/powerpc/platforms/pseries/Kconfig
> index 21b22bf16ce6..c2f8a29e7b9b 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -163,6 +163,12 @@ config PSERIES_PLPKS
> # This option is selected by in-kernel consumers that require
> # access to the PKS.
>
> +config PSERIES_PLPKS_SED
> + depends on PPC_PSERIES
> + bool
> + # This option is selected by in-kernel consumers that require
> + # access to the SED PKS keystore.
> +
> config PAPR_SCM
> depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
> tristate "Support for the PAPR Storage Class Memory
> interface"
> diff --git a/arch/powerpc/platforms/pseries/Makefile
> b/arch/powerpc/platforms/pseries/Makefile
> index 4242aed0d5d3..1476c5e4433c 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -29,7 +29,7 @@ obj-$(CONFIG_PPC_SVM) += svm.o
> obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
> obj-$(CONFIG_PSERIES_PLPKS)+= plpks.o
> obj-$(CONFIG_PPC_SECURE_BOOT) += plpks-secvar.o
> -obj-$(CONFIG_PSERIES_PLPKS_SED)+= plpks-sed.o
> +obj-$(CONFIG_PSERIES_PLPKS_SED)+= plpks_sed_ops.o
I think you could just use obj-$(CONFIG_BLK_SED_OPAL) and then there
wouldn't be a need to introduce a new option? Unless there's going to
be a second consumer.
> obj-$(CONFIG_SUSPEND) += suspend.o
> obj-$(CONFIG_PPC_VAS) += vas.o vas-sysfs.o
>
> diff --git a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> index 086934b319a9..c1d08075e850 100644
> --- a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> +++ b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> @@ -14,7 +14,7 @@
> #include
> #include
> #include
> -#include "plpks.h"
> +#include
>
> /*
> * structure that contains all SED data
> @@ -28,9 +28,6 @@ struct plpks_sed_object_data {
> u_char key[32];
> };
>
> -#define PLPKS_PLATVAR_POLICY WORLDREADABLE
> -#define PLPKS_PLATVAR_OS_COMMON 4
> -
> #define PLPKS_SED_OBJECT_DATA_V0 0
> #define PLPKS_SED_MANGLED_LABEL "/default/pri"
> #define PLPKS_SED_COMPONENT "sed-opal"
> @@ -50,8 +47,8 @@ void plpks_init_var(struct plpks_var *var, char
> *keyname)
> var->name = PLPKS_SED_MANGLED_LABEL;
> var->namelen = strlen(keyname);
> }
> - var->policy = PLPKS_PLATVAR_POLICY;
> - var->os = PLPKS_PLATVAR_OS_COMMON;
> + var->policy = PLPKS_WORLDREADABLE;
> + var->os = PLPKS_VAR_COMMON;
> var->data = NULL;
> var->datalen = 0;
> var->component = PLPKS_SED_COMPONENT;
> @@ -64,28 +61,19 @@ int sed_read_key(char *keyname, char *key, u_int
> *keylen)
> {
> struct plpks_var var;
> struct plpks_sed_object_data data;
> - u_int offset;
> int ret;
> u_int len;
>
> plpks_init_var(, keyname);
> - var.data =
> + var.data = (u8 *)
> var.datalen = sizeof(data);
>
> ret = plpks_read_os_var();
> if (ret != 0)
> return ret;
>
> - offset = offsetof(struct plpks_sed_object_data, key);
> - if (offset > var.datalen) {
> - return -EINVAL;
> - }
> -
> - len = min(be32_to_cpu(data.key_len), *keylen);
> -
> + len = min_t(u16, be32_to_cpu(data.key_len), var.datalen);
> memcpy(key, data.key, len);
> - kfree(var.data);
> -
> key[len] = '\0';
> *keylen = len;
>
> diff --git a/block/Kconfig b/block/Kconfig
> index 76b23114fdeb..75d4db34df5a 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -182,6 +182,7 @@ config BLK_SED_OPAL
> bool "Logic for interfacing with Opal enabled SEDs"
> depends on KEYS
> select PSERIES_PLPKS if PPC_PSERIES
> + select PSERIES_PLPKS_SED if PPC_PSERIES
> help
> Builds Logic for interfacing with Opal enabled controllers.
> Enabling this option enables users to setup/unlock/lock
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [RFC PATCH 1/6] powerpc/64s: Fix assembly to support larger values of THREAD_SIZE
On Fri, 2022-11-04 at 17:51 +, Christophe Leroy wrote:
>
>
> Le 04/11/2022 à 18:27, Andrew Donnellan a écrit :
> > When CONFIG_VMAP_STACK is enabled, we set THREAD_SIZE to be at
> > least the
> > size of a page.
> >
> > There's a few bits of assembly in the book3s64 code that use
> > THREAD_SIZE in
> > immediate mode instructions, which can only take an operand of up
> > to 16
> > bits signed, which isn't quite large enough.
> >
> > Fix these spots to use a scratch register or use two immediate mode
> > instructions instead, so we can later enable VMAP_STACK.
> >
> > Signed-off-by: Andrew Donnellan
> > ---
> > arch/powerpc/include/asm/asm-compat.h | 2 ++
> > arch/powerpc/kernel/entry_64.S | 4 +++-
> > arch/powerpc/kernel/irq.c | 8 ++--
> > arch/powerpc/kernel/misc_64.S | 4 +++-
> > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 ++-
> > 5 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/asm-compat.h
> > b/arch/powerpc/include/asm/asm-compat.h
> > index 2bc53c646ccd..30dd7813bf3b 100644
> > --- a/arch/powerpc/include/asm/asm-compat.h
> > +++ b/arch/powerpc/include/asm/asm-compat.h
> > @@ -11,6 +11,7 @@
> > #define PPC_LLstringify_in_c(ld)
> > #define PPC_STL stringify_in_c(std)
> > #define PPC_STLU stringify_in_c(stdu)
> > +#define PPC_STLUX stringify_in_c(stdux)
> > #define PPC_LCMPI stringify_in_c(cmpdi)
> > #define PPC_LCMPLIstringify_in_c(cmpldi)
> > #define PPC_LCMP stringify_in_c(cmpd)
> > @@ -45,6 +46,7 @@
> > #define PPC_LLstringify_in_c(lwz)
> > #define PPC_STL stringify_in_c(stw)
> > #define PPC_STLU stringify_in_c(stwu)
> > +#define PPC_STLUX stringify_in_c(stwux)
> > #define PPC_LCMPI stringify_in_c(cmpwi)
> > #define PPC_LCMPLIstringify_in_c(cmplwi)
> > #define PPC_LCMP stringify_in_c(cmpw)
> > diff --git a/arch/powerpc/kernel/entry_64.S
> > b/arch/powerpc/kernel/entry_64.S
> > index 3e2e37e6ecab..af25db6e0205 100644
> > --- a/arch/powerpc/kernel/entry_64.S
> > +++ b/arch/powerpc/kernel/entry_64.S
> > @@ -238,7 +238,9 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
> > /* Note: this uses SWITCH_FRAME_SIZE rather than
> > INT_FRAME_SIZE
> > because we don't need to leave the 288-byte ABI gap at
> > the
> > top of the kernel stack. */
> > - addir7,r7,THREAD_SIZE-SWITCH_FRAME_SIZE
> > + li r9,0
> > + ori r9,r9,THREAD_SIZE-SWITCH_FRAME_SIZE
> > + add r7,r7,r9
>
> So you assume THREAD_SIZE is never more than 64k ? Is that a valid
> assumption ?
It looks like PPC_PAGE_SHIFT can be up to 18, which would make
THREAD_SIZE 256K, but that's only if you have 256K pages, which is a
44x specific feature. Otherwise AFAICT you can't get THREAD_SHIFT
larger than 16 and therefore THREAD_SIZE <= 64K.
>
> What about the below instead:
>
> addis r7,r7,THREAD_SIZE-SWITCH_FRAME_SIZE@ha
> addir7,r7,THREAD_SIZE-SWITCH_FRAME_SIZE@l
That looks better anyway, thanks.
>
> >
> > /*
> > * PMU interrupts in radix may come in here. They will use
> > r1, not
> > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> > index 9ede61a5a469..098cf6adceec 100644
> > --- a/arch/powerpc/kernel/irq.c
> > +++ b/arch/powerpc/kernel/irq.c
> > @@ -204,7 +204,9 @@ static __always_inline void
> > call_do_softirq(const void *sp)
> > {
> > /* Temporarily switch r1 to sp, call __do_softirq() then
> > restore r1. */
> > asm volatile (
> > - PPC_STLU " %%r1, %[offset](%[sp]) ;"
> > + "li %%r0, 0 ;"
> > + "ori%%r0, %%r0, %[offset] ;"
>
> Same, you assume offset to be max 64k, is that correct ?
>
> What about
> lis r0, offset@h
> ori r0, r0, offset@l
>
> > + PPC_STLUX "%%r1, %[sp], %%r0 ;"
> > "mr %%r1, %[sp] ;"
> > "bl %[callee] ;"
> > PPC_LL " %%r1, 0(%%r1) ;"
> > @@ -256,7 +258,9 @@ static __always_inline void call_do_irq(struct
> > pt_regs *regs, void *sp)
> >
> >
Re: [PATCH] powerpc/rtas: Replace one-element arrays with flexible arrays
On Fri, 2023-01-27 at 07:10 -0600, Nathan Lynch wrote: > > > > I see at least one place that consults the size of one of these > > > > structs, > > > > in get_pseries_errorlog(): > > > > > > > > /* Check that we understand the format */ > > > > if (ext_log_length < sizeof(struct > > > > rtas_ext_event_log_v6) > > > > || > > > > ... > > > > > > > > Don't all such sites need to be audited/adjusted for changes > > > > like > > > > this? I did actually see that site, and concluded that for the purposes of that particular check, removing a single extra byte is irrelevant (maybe it makes the check more strictly correct, what if the vendor_log is actually of length 0?) Doing a binary diff, as Kees suggests, over the object files in arch/powerpc: - there's no difference at all caused by changing rtas_ext_event_log_v6.vendor_log, which kind of surprises me given the above. - changing rtas_error_log.buffer does seem to change some code generation in arch/powerpc/platforms/pseries/ras.o, I can't quite see why. Andrew -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 06/32] powerpc/configs/64s: Add secure boot options to defconfig
On Mon, 2023-04-17 at 13:38 +1000, Michael Ellerman wrote: > > Can we add CONFIG_PPC_SECVAR_SYSFS=y as well? > > We can. > > But would it make more sense to just make PPC_SECVAR_SYSFS a hidden > symbol? Is there really any reason someone would want to turn it off? [+ Russell, Nayna, George] I think it's conceivable that you may want to build a kernel that has no ability for userspace to read/write to the key store at all as a defence in depth measure in hardened environments, but I haven't thought about this for more than 15 seconds, so opinions welcome. -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 06/32] powerpc/configs/64s: Add secure boot options to defconfig
On Fri, 2023-04-14 at 23:23 +1000, Michael Ellerman wrote: > Add the numerous options required to get secure boot enabled. > > Signed-off-by: Michael Ellerman > --- > arch/powerpc/configs/ppc64_defconfig | 17 - > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/configs/ppc64_defconfig > b/arch/powerpc/configs/ppc64_defconfig > index d98fe52a5892..f185adc128db 100644 > --- a/arch/powerpc/configs/ppc64_defconfig > +++ b/arch/powerpc/configs/ppc64_defconfig > @@ -54,6 +54,7 @@ CONFIG_CRASH_DUMP=y > CONFIG_FA_DUMP=y > CONFIG_IRQ_ALL_CPUS=y > CONFIG_SCHED_SMT=y > +CONFIG_PPC_SECURE_BOOT=y Can we add CONFIG_PPC_SECVAR_SYSFS=y as well? > CONFIG_VIRTUALIZATION=y > CONFIG_KVM_BOOK3S_64=m > CONFIG_KVM_BOOK3S_64_HV=m > @@ -335,13 +336,25 @@ CONFIG_NLS_CODEPAGE_437=y > CONFIG_NLS_ASCII=y > CONFIG_NLS_ISO8859_1=y > CONFIG_NLS_UTF8=y > +CONFIG_SECURITY=y > +CONFIG_SECURITY_LOCKDOWN_LSM=y > +CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=y > +CONFIG_INTEGRITY_SIGNATURE=y > +CONFIG_INTEGRITY_ASYMMETRIC_KEYS=y > +CONFIG_INTEGRITY_PLATFORM_KEYRING=y > +CONFIG_IMA=y > +CONFIG_IMA_KEXEC=y > +CONFIG_IMA_DEFAULT_HASH_SHA256=y > +CONFIG_IMA_WRITE_POLICY=y > +CONFIG_IMA_APPRAISE=y > +CONFIG_IMA_ARCH_POLICY=y > +CONFIG_IMA_APPRAISE_MODSIG=y > CONFIG_CRYPTO_TEST=m > CONFIG_CRYPTO_BLOWFISH=m > CONFIG_CRYPTO_CAST6=m > CONFIG_CRYPTO_SERPENT=m > CONFIG_CRYPTO_TWOFISH=m > CONFIG_CRYPTO_PCBC=m > -CONFIG_CRYPTO_HMAC=y > CONFIG_CRYPTO_MICHAEL_MIC=m > CONFIG_CRYPTO_SHA256=y > CONFIG_CRYPTO_WP512=m > @@ -352,6 +365,8 @@ CONFIG_CRYPTO_SHA1_PPC=m > CONFIG_CRYPTO_DEV_NX=y > CONFIG_CRYPTO_DEV_NX_ENCRYPT=m > CONFIG_CRYPTO_DEV_VMX=y > +CONFIG_SYSTEM_TRUSTED_KEYRING=y > +CONFIG_SYSTEM_BLACKLIST_KEYRING=y > CONFIG_PRINTK_TIME=y > CONFIG_PRINTK_CALLER=y > CONFIG_DEBUG_KERNEL=y -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 8/8] powerpc/rtas: consume retry statuses in sys_rtas()
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote: > From: Nathan Lynch > > The kernel can handle retrying RTAS function calls in response to > -2/990x in the sys_rtas() handler instead of relaying the > intermediate > status to user space. > > Justifications: > > * Currently it's nondeterministic and quite variable in practice > whether a retry status is returned for any given invocation of > sys_rtas(). Therefore user space code cannot be expecting a retry > result without already being broken. > > * This tends to significantly reduce the total number of system calls > issued by programs such as drmgr which make use of sys_rtas(), > improving the experience of tracing and debugging such > programs. This is the main motivation for me: I think this change > will make it easier for us to characterize current sys_rtas() use > cases as we move them to other interfaces over time. > > * It reduces the number of opportunities for user space to leave > complex operations, such as those associated with DLPAR, incomplete > and diffcult to recover. > > * We can expect performance improvements for existing sys_rtas() > users, not only because of overall reduction in the number of > system > calls issued, but also due to the better handling of -2/990x in the > kernel. For example, librtas still sleeps for 1ms on -2, which is > completely unnecessary. Would be good to see this fixed on the librtas side. > > Performance differences for PHB add and remove on a small P10 PowerVM > partition are included below. For add, elapsed time is slightly > reduced. For remove, there are more significant improvements: the > number of context switches is reduced by an order of magnitude, and > elapsed time is reduced by over half. > > (- before, + after): > > Performance counter stats for 'drmgr -c phb -a -s PHB 23' (5 runs): > > - 1,847.58 msec task-clock # 0.135 > CPUs utilized ( +- 14.15% ) > - 10,867 cs # 9.800 > K/sec ( +- 14.14% ) > + 1,901.15 msec task-clock # 0.148 > CPUs utilized ( +- 14.13% ) > + 10,451 cs # 9.158 > K/sec ( +- 14.14% ) > > - 13.656557 +- 0.000124 seconds time elapsed ( +- 0.00% ) > + 12.88080 +- 0.00404 seconds time elapsed ( +- 0.03% ) > > Performance counter stats for 'drmgr -c phb -r -s PHB 23' (5 runs): > > - 1,473.75 msec task-clock # 0.092 > CPUs utilized ( +- 14.15% ) > - 2,652 cs # 3.000 > K/sec ( +- 14.16% ) > + 1,444.55 msec task-clock # 0.221 > CPUs utilized ( +- 14.14% ) > + 104 cs # 119.957 > /sec ( +- 14.63% ) > > - 15.99718 +- 0.00801 seconds time elapsed ( +- 0.05% ) > + 6.54256 +- 0.00830 seconds time elapsed ( +- 0.13% ) > > Move the existing rtas_lock-guarded critical section in sys_rtas() > into a conventional rtas_busy_delay()-based loop, returning to user > space only when a final success or failure result is available. > > Signed-off-by: Nathan Lynch Should there be some kind of timeout? I'm a bit worried by sleeping in a syscall for an extended period. -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 6/8] powerpc/rtas: lockdep annotations
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote: > From: Nathan Lynch > > Add lockdep annotations for the following properties that must hold: > > * Any error log retrieval must be atomically coupled with the prior > RTAS call, without a window for another RTAS call to occur before > the > error log can be retrieved. > > * All users of the core rtas_args parameter block must hold > rtas_lock. > > Move the definitions of rtas_lock and rtas_args up in the file so > that > __do_enter_rtas_trace() can refer to them. > > Signed-off-by: Nathan Lynch I'm no lockdep expert and I haven't checked if every possible case that can be annotated has been annotated, but these changes make sense as far as I can tell from my limited inspection. Reviewed-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 7/8] powerpc/rtas: warn on unsafe argument to rtas_call_unlocked()
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> Any caller of rtas_call_unlocked() must provide an rtas_args
> parameter
> block distinct from the core rtas_args buffer used by the rtas_call()
> path. It's an unlikely error to make, but the potential consequences
> are grim, and it's trivial to check.
>
> Signed-off-by: Nathan Lynch
call_rtas_display_status() seems to do exactly this, or am I missing
something?
> ---
> arch/powerpc/kernel/rtas.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 633c925164e7..47a2aa43d7d4 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -1042,6 +1042,13 @@ void rtas_call_unlocked(struct rtas_args
> *args, int token, int nargs, int nret,
> {
> va_list list;
>
> + /*
> + * Callers must not use rtas_args; otherwise they risk
> + * corrupting the state of the rtas_call() path, which is
> + * serialized by rtas_lock.
> + */
> + WARN_ON(args == _args);
> +
> va_start(list, nret);
> va_rtas_call(args, token, nargs, nret, list);
> va_end(list);
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 5/8] powerpc/rtas: rename va_rtas_call_unlocked() to va_rtas_call()
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> The function name va_rtas_call_unlocked() is confusing: it may be
> called with or without rtas_lock held. Rename it to va_rtas_call().
>
> Signed-off-by: Nathan Lynch
Not a huge fan of the name, the va_ suggests that the only difference
between this function and rtas_call() is the varargs handling. Perhaps
something like __rtas_call()?
> ---
> arch/powerpc/kernel/rtas.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index c29c38b1a55a..96a10a0abe3a 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -996,9 +996,8 @@ static void __init init_error_log_max(void) {}
> #endif
>
>
> -static void
> -va_rtas_call_unlocked(struct rtas_args *args, int token, int nargs,
> int nret,
> - va_list list)
> +static void va_rtas_call(struct rtas_args *args, int token, int
> nargs, int nret,
> + va_list list)
> {
> int i;
>
> @@ -1038,7 +1037,7 @@ void rtas_call_unlocked(struct rtas_args *args,
> int token, int nargs, int nret,
> va_list list;
>
> va_start(list, nret);
> - va_rtas_call_unlocked(args, token, nargs, nret, list);
> + va_rtas_call(args, token, nargs, nret, list);
> va_end(list);
> }
>
> @@ -1138,7 +1137,7 @@ int rtas_call(int token, int nargs, int nret,
> int *outputs, ...)
> args = _args;
>
> va_start(list, outputs);
> - va_rtas_call_unlocked(args, token, nargs, nret, list);
> + va_rtas_call(args, token, nargs, nret, list);
> va_end(list);
>
> /* A -1 return code indicates that the last command couldn't
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 3/8] powerpc/rtas: rtas_call_unlocked() kerneldoc
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote: > From: Nathan Lynch > > Add documentation for rtas_call_unlocked(), including details on how > it differs from rtas_call(). > > Signed-off-by: Nathan Lynch Reviewed-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 2/8] powerpc/rtas: use memmove for potentially overlapping buffer copy
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> Using memcpy() isn't safe when buf is identical to rtas_err_buf,
> which
> can happen during boot before slab is up. Full context which may not
> be obvious from the diff:
>
> if (altbuf) {
> buf = altbuf;
> } else {
> buf = rtas_err_buf;
> if (slab_is_available())
> buf = kmalloc(RTAS_ERROR_LOG_MAX,
> GFP_ATOMIC);
> }
> if (buf)
> memcpy(buf, rtas_err_buf, RTAS_ERROR_LOG_MAX);
>
> This was found by inspection and I'm not aware of it causing problems
> in practice. It appears to have been introduced by commit
> 033ef338b6e0 ("powerpc: Merge rtas.c into arch/powerpc/kernel"); the
> old ppc64 version of this code did not have this problem.
>
> Use memmove() instead.
>
> Fixes: 033ef338b6e0 ("powerpc: Merge rtas.c into
> arch/powerpc/kernel")
> Signed-off-by: Nathan Lynch
Reviewed-by: Andrew Donnellan
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 1/8] powerpc/rtas: ensure 8-byte alignment for struct rtas_args
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote:
> > From: Nathan Lynch
> >
> > CHRP and PAPR agree: "In order to make an RTAS call, the operating
> > system must construct an argument call buffer aligned on an eight
> > byte
> > boundary in physically contiguous real memory [...]." (7.2.7
> > Calling
> > Mechanism and Conventions).
> >
> > struct rtas_args is the type used for this argument call buffer.
> > The
> > unarchitected 'rets' member happens to produce 8-byte alignment for
> > the struct on 64-bit targets in practice. But without an alignment
> > directive the structure will have only 4-byte alignment on 32-bit
> > targets:
> >
> > $ nm b/{before,after}/chrp32/vmlinux | grep rtas_args
> > c096881c b rtas_args
> > c0968820 b rtas_args
> >
> > Add an alignment directive to the struct rtas_args declaration so
> > all
> > instances have the alignment required by the specs. rtas-types.h no
> > longer refers to any spinlock types, so drop the spinlock_types.h
> > inclusion while we're here.
> >
> > Signed-off-by: Nathan Lynch
Reviewed-by: Andrew Donnellan
> > ---
> > arch/powerpc/include/asm/rtas-types.h | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/rtas-types.h
> > b/arch/powerpc/include/asm/rtas-types.h
> > index f2ad4a96cbc5..861145c8a021 100644
> > --- a/arch/powerpc/include/asm/rtas-types.h
> > +++ b/arch/powerpc/include/asm/rtas-types.h
> > @@ -2,7 +2,8 @@
> > #ifndef _ASM_POWERPC_RTAS_TYPES_H
> > #define _ASM_POWERPC_RTAS_TYPES_H
> >
> > -#include
> > +#include
> > +#include
> >
> > typedef __be32 rtas_arg_t;
> >
> > @@ -12,7 +13,7 @@ struct rtas_args {
> > __be32 nret;
> > rtas_arg_t args[16];
> > rtas_arg_t *rets; /* Pointer to return values in
> > args[].
> > */
> > -};
> > +} __aligned(SZ_8);
Nowhere else in the kernel uses __aligned(SZ_8) over just __aligned(8),
which I suppose would also save an include, but I don't care either
way.
> >
> > struct rtas_t {
> > unsigned long entry;/* physical address pointer
> > */
> >
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 4/8] powerpc/rtas: fix miswording in rtas_function kerneldoc
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch
>
> The 'filter' member is a pointer, not a bool; fix the wording
> accordingly.
>
> Signed-off-by: Nathan Lynch
Reviewed-by: Andrew Donnellan
> ---
> arch/powerpc/kernel/rtas.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index c73b01d722f6..c29c38b1a55a 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -68,7 +68,7 @@ struct rtas_filter {
> * functions are believed to have no
> users on
> * ppc64le, and we want to keep it that
> way. It does
> * not make sense for this to be set when
> @filter
> - * is false.
> + * is NULL.
> */
> struct rtas_function {
> s32 token;
>
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] powerpc/iommu: Fix notifiers being shared by PCI and VIO buses
On Wed, 2023-03-22 at 14:53 +1100, Russell Currey wrote:
> fail_iommu_setup() registers the fail_iommu_bus_notifier struct to
> both
> PCI and VIO buses. struct notifier_block is a linked list node, so
> this
> causes any notifiers later registered to either bus type to also be
> registered to the other since they share the same node.
>
> This causes issues in (at least) the vgaarb code, which registers a
> notifier for PCI buses. pci_notify() ends up being called on a vio
> device, converted with to_pci_dev() even though it's not a PCI
> device,
> and finally makes a bad access in vga_arbiter_add_pci_device() as
> discovered with KASAN:
>
> BUG: KASAN: slab-out-of-bounds in
> vga_arbiter_add_pci_device+0x60/0xe00
> Read of size 4 at addr c00264c26fdc by task swapper/0/1
>
> Call Trace:
> [c00263607520] [c00010f7023c] dump_stack_lvl+0x1bc/0x2b8
> (unreliable)
> [c00263607560] [cf142a64] print_report+0x3f4/0xc60
> [c00263607640] [cf142144] kasan_report+0x244/0x698
> [c00263607740] [cf1460e8] __asan_load4+0xe8/0x250
> [c00263607760] [cff4b850]
> vga_arbiter_add_pci_device+0x60/0xe00
> [c00263607850] [cff4c678] pci_notify+0x88/0x444
> [c002636078b0] [ce94dfc4]
> notifier_call_chain+0x104/0x320
> [c00263607950] [ce94f050]
> blocking_notifier_call_chain+0xa0/0x140
> [c00263607990] [c000100cb3b8] device_add+0xac8/0x1d30
> [c00263607aa0] [c000100ccd98] device_register+0x58/0x80
> [c00263607ad0] [ce84247c]
> vio_register_device_node+0x9ac/0xce0
> [c00263607ba0] [c000126c95d8]
> vio_bus_scan_register_devices+0xc4/0x13c
> [c00263607bd0] [c000126c96e4]
> __machine_initcall_pseries_vio_device_init+0x94/0xf0
> [c00263607c00] [ce69467c] do_one_initcall+0x12c/0xaa8
> [c00263607cf0] [c0001268b8a8]
> kernel_init_freeable+0xa48/0xba8
> [c00263607dd0] [ce695f24] kernel_init+0x64/0x400
> [c00263607e50] [ce68e0e4]
> ret_from_kernel_thread+0x5c/0x64
>
> Fix this by creating separate notifier_block structs for each bus
> type.
>
> Fixes: d6b9a81b2a45 ("powerpc: IOMMU fault injection")
> Reported-by: Nageswara R Sastry
> Signed-off-by: Russell Currey
Reviewed-by: Andrew Donnellan
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
[PATCH] powerpc/pseries: Add FW_FEATURE_PLPKS feature flag
Add a firmware feature flag, FW_FEATURE_PLPKS, to indicate availability of
Platform KeyStore related hcalls.
Check this flag in plpks_is_available() and pseries_plpks_init() before
trying to make an hcall.
Suggested-by: Michael Ellerman
Signed-off-by: Andrew Donnellan
---
arch/powerpc/include/asm/firmware.h | 4 +++-
arch/powerpc/platforms/pseries/firmware.c | 1 +
arch/powerpc/platforms/pseries/plpks.c| 5 -
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/firmware.h
b/arch/powerpc/include/asm/firmware.h
index ed6db13a1d7c..69ae9cf57d50 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -56,6 +56,7 @@
#define FW_FEATURE_FORM2_AFFINITY ASM_CONST(0x0200)
#define FW_FEATURE_ENERGY_SCALE_INFO ASM_CONST(0x0400)
#define FW_FEATURE_WATCHDOGASM_CONST(0x0800)
+#define FW_FEATURE_PLPKS ASM_CONST(0x1000)
#ifndef __ASSEMBLY__
@@ -77,7 +78,8 @@ enum {
FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR |
FW_FEATURE_RPT_INVALIDATE | FW_FEATURE_FORM2_AFFINITY |
- FW_FEATURE_ENERGY_SCALE_INFO | FW_FEATURE_WATCHDOG,
+ FW_FEATURE_ENERGY_SCALE_INFO | FW_FEATURE_WATCHDOG |
+ FW_FEATURE_PLPKS,
FW_FEATURE_PSERIES_ALWAYS = 0,
FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL | FW_FEATURE_ULTRAVISOR,
FW_FEATURE_POWERNV_ALWAYS = 0,
diff --git a/arch/powerpc/platforms/pseries/firmware.c
b/arch/powerpc/platforms/pseries/firmware.c
index 080108d129ed..18447e5fa17d 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -68,6 +68,7 @@ hypertas_fw_features_table[] = {
{FW_FEATURE_RPT_INVALIDATE, "hcall-rpt-invalidate"},
{FW_FEATURE_ENERGY_SCALE_INFO, "hcall-energy-scale-info"},
{FW_FEATURE_WATCHDOG, "hcall-watchdog"},
+ {FW_FEATURE_PLPKS, "hcall-pks"},
};
/* Build up the firmware features bitmask using the contents of
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 6f7bf3fc3aea..b0658ea3eccb 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -378,7 +378,7 @@ bool plpks_is_available(void)
{
int rc;
- if (!firmware_has_feature(FW_FEATURE_LPAR))
+ if (!firmware_has_feature(FW_FEATURE_PLPKS))
return false;
rc = _plpks_get_config();
@@ -690,6 +690,9 @@ static __init int pseries_plpks_init(void)
{
int rc;
+ if (!firmware_has_feature(FW_FEATURE_PLPKS))
+ return -ENODEV;
+
rc = _plpks_get_config();
if (rc) {
--
2.39.2
[PATCH] powerpc/pseries: Fix endianness issue when parsing PLPKS secvar flags
When a user updates a variable through the PLPKS secvar interface, we take
the first 8 bytes of the data written to the update attribute to pass
through to the H_PKS_SIGNED_UPDATE hcall as flags. These bytes are always
written in big-endian format.
Currently, the flags bytes are memcpy()ed into a u64, which is then loaded
into a register to pass as part of the hcall. This means that on LE
systems, the bytes are in the wrong order.
Use be64_to_cpup() instead, to ensure the flags bytes are byteswapped if
necessary.
Reported-by: Stefan Berger
Fixes: ccadf154cb00 ("powerpc/pseries: Implement secvars for dynamic secure
boot")
Signed-off-by: Andrew Donnellan
---
arch/powerpc/platforms/pseries/plpks-secvar.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c
b/arch/powerpc/platforms/pseries/plpks-secvar.c
index f6cf4076..257fd1f8bc19 100644
--- a/arch/powerpc/platforms/pseries/plpks-secvar.c
+++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
@@ -135,7 +135,8 @@ static int plpks_set_variable(const char *key, u64 key_len,
u8 *data,
goto err;
var.namelen = rc * 2;
- memcpy(, data, sizeof(flags));
+ // Flags are contained in the first 8 bytes of the buffer, and are
always big-endian
+ flags = be64_to_cpup((__be64 *)data);
var.datalen = data_size - sizeof(flags);
var.data = data + sizeof(flags);
--
2.39.1
Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot
On Mon, 2023-02-13 at 22:32 +1100, Michael Ellerman wrote: > > > + memcpy(, data, sizeof(flags)); > > > > conversion from bytestream to integer: I think in this case it > > would be better to use > > > > flags = cpu_to_be64p((__u64*)data); > > > > so that the flags always in hypervisor/big endian format > > I don't think it's correct to byte swap the flags here. They must be > in > big endian format, but that's up to the caller. That was what I initially thought, until I went and tested it properly and found it was indeed broken (at least in our qemu environment, this is slightly tricky for me to test right now on real hardware with real PowerVM) depending on kernel endianness. - Userspace writes the flags into the buffer in BE order - The first 8 bytes of the buffer are memcpy()ed, in BE order, into flags (a u64) - plpar_hcall9() is called with flags as an argument, loaded into r9 - r9 is moved to r8 before jumping into the hypervisor On a BE system, this works fine. On an LE system, this results in the bytes in the flags variable being loaded into the register in LE order, so the conversion is necessary. > The powernv secvar backend doesn't byte swap the flags, if the > pseries > one did then the final content of the variable, written either by > phyp > or OPAL, would differ depending on which backend is active. > > Or am I missing something? The powernv secvar backend doesn't have a notion of flags at all. (The flags interface for pseries is a little ugly, but it gets the job done - userspace can read the format attribute to discover what it needs to do.) -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot
On Mon, 2023-02-13 at 16:26 +1100, Michael Ellerman wrote: > Andrew Donnellan writes: > > On Fri, 2023-02-10 at 16:28 -0500, Stefan Berger wrote: > > > > > +err: > > > > > + kfree(var.data); > > > > > > > > remove the kfree() > > > > > > Actually don't remove it but it should probably be > > > > > > if (var.data != ) > > > kfree(var.data); > > > > > > > Argh, thanks for catching this. > > > > I don't think the condition is needed - we can assume the var.data > > is > > unmodified. > > > > mpe, are you able to fix this up in merge? > > Yeah, can you reply here with the delta you want applied. > > cheers diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c index 98d0f2b31e0d..bdfe63bc8705 100644 --- a/arch/powerpc/platforms/pseries/plpks-secvar.c +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c @@ -135,7 +135,7 @@ static int plpks_set_variable(const char *key, u64 key_len, u8 *data, goto err; var.namelen = rc * 2; - memcpy(, data, sizeof(flags)); + flags = cpu_to_be64p((__u64 *)data); var.datalen = data_size - sizeof(flags); var.data = data + sizeof(flags); @@ -184,7 +184,6 @@ static ssize_t plpks_secvar_format(char *buf, size_t bufsize) ret = snprintf(buf, bufsize, "ibm,plpks-sb-v%hhu", version); err: - kfree(var.data); return ret; } -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2023-02-10 at 16:23 -0500, Stefan Berger wrote: > > + memcpy(, data, sizeof(flags)); > > conversion from bytestream to integer: I think in this case it would > be better to use > > flags = cpu_to_be64p((__u64*)data); > > so that the flags always in hypervisor/big endian format Thanks for catching this - it turns out we weren't properly testing the one flag that exists (append vs replace) in our test script, so I didn't notice this. -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2023-02-10 at 16:28 -0500, Stefan Berger wrote: > > > +err: > > > + kfree(var.data); > > > > remove the kfree() > > Actually don't remove it but it should probably be > > if (var.data != ) > kfree(var.data); > Argh, thanks for catching this. I don't think the condition is needed - we can assume the var.data is unmodified. mpe, are you able to fix this up in merge? -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v6 21/26] powerpc/pseries: Clarify warning when PLPKS password already set
On Fri, 2023-02-10 at 15:47 -0500, Stefan Berger wrote: > > > On 2/10/23 03:03, Andrew Donnellan wrote: > > When the H_PKS_GEN_PASSWORD hcall returns H_IN_USE, operations that > > require > > authentication (i.e. anything other than reading a world-readable > > variable) > > will not work. > > > > The current error message doesn't explain this clearly enough. > > Reword it > > to emphasise that authenticated operations will fail. > > typo: -> emphasize This commit message was written in en_AU. ;) -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
[PATCH v6 20/26] powerpc/pseries: Turn PSERIES_PLPKS into a hidden option
It seems a bit unnecessary for the PLPKS code to have a user-visible config option when it doesn't do anything on its own, and there's existing options for enabling Secure Boot-related features. It should be enabled by PPC_SECURE_BOOT, which will eventually be what uses PLPKS to populate keyrings. However, we can't get of the separate option completely, because it will also be used for SED Opal purposes. Change PSERIES_PLPKS into a hidden option, which is selected by PPC_SECURE_BOOT. Signed-off-by: Andrew Donnellan Signed-off-by: Russell Currey Reviewed-by: Stefan Berger --- v3: New patch v5: Change the previous description into a comment (npiggin) --- arch/powerpc/Kconfig | 1 + arch/powerpc/platforms/pseries/Kconfig | 19 +-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index b8c4ac56bddc..d4ed46101bec 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -1029,6 +1029,7 @@ config PPC_SECURE_BOOT depends on PPC_POWERNV || PPC_PSERIES depends on IMA_ARCH_POLICY imply IMA_SECURE_AND_OR_TRUSTED_BOOT + select PSERIES_PLPKS if PPC_PSERIES help Systems with firmware secure boot enabled need to define security policies to extend secure boot to the OS. This config allows a user diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index a3b4d99567cb..e51d65969318 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -151,16 +151,15 @@ config IBMEBUS config PSERIES_PLPKS depends on PPC_PSERIES - bool "Support for the Platform Key Storage" - help - PowerVM provides an isolated Platform Keystore(PKS) storage - allocation for each LPAR with individually managed access - controls to store sensitive information securely. It can be - used to store asymmetric public keys or secrets as required - by different usecases. Select this config to enable - operating system interface to hypervisor to access this space. - - If unsure, select N. + bool + # PowerVM provides an isolated Platform Keystore (PKS) storage + # allocation for each LPAR with individually managed access + # controls to store sensitive information securely. It can be + # used to store asymmetric public keys or secrets as required + # by different usecases. + # + # This option is selected by in-kernel consumers that require + # access to the PKS. config PAPR_SCM depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM -- 2.39.1
[PATCH v6 01/26] powerpc/pseries: Fix handling of PLPKS object flushing timeout
plpks_confirm_object_flushed() uses the H_PKS_CONFIRM_OBJECT_FLUSHED hcall
to check whether changes to an object in the Platform KeyStore have been
flushed to non-volatile storage.
The hcall returns two output values, the return code and the flush status.
plpks_confirm_object_flushed() polls the hcall until either the flush
status has updated, the return code is an error, or a timeout has been
exceeded.
While we're still polling, the hcall is returning H_SUCCESS (0) as the
return code. In the timeout case, this means that upon exiting the polling
loop, rc is 0, and therefore 0 is returned to the user.
Handle the timeout case separately and return ETIMEDOUT if triggered.
Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore")
Reported-by: Benjamin Gray
Signed-off-by: Andrew Donnellan
Tested-by: Russell Currey
Reviewed-by: Russell Currey
Signed-off-by: Russell Currey
---
v3: Merge plpks fixes and signed update series with secvar series
Neaten how we return at the end of the function (ruscur)
v4: Move up in series (npiggin)
---
arch/powerpc/platforms/pseries/plpks.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 4edd1585e245..9e85b6d85b0b 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -248,6 +248,7 @@ static int plpks_confirm_object_flushed(struct label *label,
struct plpks_auth *auth)
{
unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+ bool timed_out = true;
u64 timeout = 0;
u8 status;
int rc;
@@ -259,22 +260,26 @@ static int plpks_confirm_object_flushed(struct label
*label,
status = retbuf[0];
if (rc) {
+ timed_out = false;
if (rc == H_NOT_FOUND && status == 1)
rc = 0;
break;
}
- if (!rc && status == 1)
+ if (!rc && status == 1) {
+ timed_out = false;
break;
+ }
usleep_range(PKS_FLUSH_SLEEP,
PKS_FLUSH_SLEEP + PKS_FLUSH_SLEEP_RANGE);
timeout = timeout + PKS_FLUSH_SLEEP;
} while (timeout < PKS_FLUSH_MAX_TIMEOUT);
- rc = pseries_status_to_err(rc);
+ if (timed_out)
+ return -ETIMEDOUT;
- return rc;
+ return pseries_status_to_err(rc);
}
int plpks_write_var(struct plpks_var var)
--
2.39.1
[PATCH v6 14/26] powerpc/pseries: Move plpks.h to include directory
From: Russell Currey
Move plpks.h from platforms/pseries/ to include/asm/. This is necessary
for later patches to make use of the PLPKS from code in other subsystems.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v3: New patch
---
.../powerpc/{platforms/pseries => include/asm}/plpks.h | 10 +++---
arch/powerpc/platforms/pseries/plpks.c | 3 +--
2 files changed, 8 insertions(+), 5 deletions(-)
rename arch/powerpc/{platforms/pseries => include/asm}/plpks.h (89%)
diff --git a/arch/powerpc/platforms/pseries/plpks.h
b/arch/powerpc/include/asm/plpks.h
similarity index 89%
rename from arch/powerpc/platforms/pseries/plpks.h
rename to arch/powerpc/include/asm/plpks.h
index 275ccd86bfb5..8295502ee93b 100644
--- a/arch/powerpc/platforms/pseries/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -6,8 +6,10 @@
* Platform keystore for pseries LPAR(PLPKS).
*/
-#ifndef _PSERIES_PLPKS_H
-#define _PSERIES_PLPKS_H
+#ifndef _ASM_POWERPC_PLPKS_H
+#define _ASM_POWERPC_PLPKS_H
+
+#ifdef CONFIG_PSERIES_PLPKS
#include
#include
@@ -68,4 +70,6 @@ int plpks_read_fw_var(struct plpks_var *var);
*/
int plpks_read_bootloader_var(struct plpks_var *var);
-#endif
+#endif // CONFIG_PSERIES_PLPKS
+
+#endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index a01cf2ff140a..13e6daadb179 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -18,8 +18,7 @@
#include
#include
#include
-
-#include "plpks.h"
+#include
#define PKS_FW_OWNER0x1
#define PKS_BOOTLOADER_OWNER 0x2
--
2.39.1
[PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot
From: Russell Currey The pseries platform can support dynamic secure boot (i.e. secure boot using user-defined keys) using variables contained with the PowerVM LPAR Platform KeyStore (PLPKS). Using the powerpc secvar API, expose the relevant variables for pseries dynamic secure boot through the existing secvar filesystem layout. The relevant variables for dynamic secure boot are signed in the keystore, and can only be modified using the H_PKS_SIGNED_UPDATE hcall. Object labels in the keystore are encoded using ucs2 format. With our fixed variable names we don't have to care about encoding outside of the necessary byte padding. When a user writes to a variable, the first 8 bytes of data must contain the signed update flags as defined by the hypervisor. When a user reads a variable, the first 4 bytes of data contain the policies defined for the object. Limitations exist due to the underlying implementation of sysfs binary attributes, as is the case for the OPAL secvar implementation - partial writes are unsupported and writes cannot be larger than PAGE_SIZE. (Even when using bin_attributes, which can be larger than a single page, sysfs only gives us one page's worth of write buffer at a time, and the hypervisor does not expose an interface for partial writes.) Co-developed-by: Nayna Jain Signed-off-by: Nayna Jain Co-developed-by: Andrew Donnellan Signed-off-by: Andrew Donnellan Signed-off-by: Russell Currey --- v2: Remove unnecessary config vars from sysfs and document the others, thanks to review from Greg. If we end up needing to expose more, we can add them later and update the docs. Use sysfs_emit() instead of sprintf(), thanks to Greg. Change the size of the sysfs binary attributes to include the 8-byte flags header, preventing truncation of large writes. v3: plpks_set_variable(): pass var to plpks_signed_update_var() as a pointer (mpe) Update copyright date (ajd) Consistent comment style (ajd) Change device_initcall() to machine_arch_initcall(pseries...) so we don't try to load on powernv and kill the machine (mpe) Add config attributes into plpks_secvar_ops (mpe) Get rid of PLPKS_SECVAR_COUNT macro (mpe) Reworded descriptions in ABI documentation (mpe) Switch to using secvar_ops->var_names rather than secvar_ops->get_next() (ajd/mpe) Optimise allocation/copying of buffers (mpe) Elaborate the comment documenting the "format" string (mpe) Return -EIO on errors in the read case (mpe) Add "grubdbx" variable (Sudhakar Kuppusamy) Use utf8s_to_utf16s() rather than our own "UCS-2" conversion code (mpe) Change uint64_t to u64 (mpe) Fix SB_VERSION data length (ruscur) Stop prepending policy data on read (ruscur) Enforce max format length on format string (not strictly needed, but makes the length limit clear) (ajd) Update include of plpks.h to reflect new path (ruscur) Consistent constant naming scheme (ruscur) v4: Return set_secvar_ops() return code Pass buffer size to plpks_secvar_format() (stefanb, npiggin) Add missing null check (stefanb) Add comment to commit message explaining PAGE_SIZE write limit (joel) v5: Add comment explaining why we use "key_len - 1" (npiggin) Use strlen(var.name) instead of hardcoding 10 as length of "SB_VERSION" (npiggin) Improve comments about use of SB_VERSION and format string (npiggin) Change "+ 8" to "+ sizeof(u64)" when accounting for flags size in working out file's max size (npiggin) Compile plpks-secvar.c based on CONFIG_PPC_SECURE_BOOT, not CONFIG_PPC_SECVAR_SYSFS, as the secvar backend is needed for loading keys into keyrings even if the sysfs interface is disabled (ajd) v6: Update date in ABI docs (stefanb) Get rid of 1 byte kzalloc (npiggin) --- Documentation/ABI/testing/sysfs-secvar| 75 +- arch/powerpc/platforms/pseries/Makefile | 4 +- arch/powerpc/platforms/pseries/plpks-secvar.c | 218 ++ 3 files changed, 294 insertions(+), 3 deletions(-) create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar index feebb8c57294..857cf12b0904 100644 --- a/Documentation/ABI/testing/sysfs-secvar +++ b/Documentation/ABI/testing/sysfs-secvar @@ -18,6 +18,14 @@ Description: A string indicating which backend is in use by the firmware. This determines the format of the variable and the accepted format of variable updates. + On powernv/OPAL, this value is provided by the OPAL firmware + and is expected to be "ibm,edk2-compat-v1". + + On pseries/PLPKS, this is generated by the kernel based on the + version number in the SB_VERSION variable in the keystore, and +
[PATCH v6 17/26] powerpc/pseries: Implement signed update for PLPKS objects
From: Nayna Jain
The Platform Keystore provides a signed update interface which can be used
to create, replace or append to certain variables in the PKS in a secure
fashion, with the hypervisor requiring that the update be signed using the
Platform Key.
Implement an interface to the H_PKS_SIGNED_UPDATE hcall in the plpks
driver to allow signed updates to PKS objects.
(The plpks driver doesn't need to do any cryptography or otherwise handle
the actual signed variable contents - that will be handled by userspace
tooling.)
Signed-off-by: Nayna Jain
[ajd: split patch, add timeout handling and misc cleanups]
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
Reviewed-by: Stefan Berger
---
v3: Merge plpks fixes and signed update series with secvar series
Fix error code handling in plpks_confirm_object_flushed() (ruscur)
Pass plpks_var struct to plpks_signed_update_var() by reference (mpe)
Consistent constant naming scheme (ruscur)
v4: Fix MAX_HCALL_OPCODE rebasing issue (npiggin)
v5: Drop the EXPORT_SYMBOL since we don't need it (npiggin)
Return an error if plpks_signed_update_var() is called with non-NULL
component (npiggin)
---
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/include/asm/plpks.h | 5 ++
arch/powerpc/platforms/pseries/plpks.c | 74 --
3 files changed, 75 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h
b/arch/powerpc/include/asm/hvcall.h
index 95fd7f9485d5..c099780385dd 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -335,6 +335,7 @@
#define H_RPT_INVALIDATE 0x448
#define H_SCM_FLUSH0x44C
#define H_GET_ENERGY_SCALE_INFO0x450
+#define H_PKS_SIGNED_UPDATE0x454
#define H_WATCHDOG 0x45C
#define MAX_HCALL_OPCODE H_WATCHDOG
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 7c5f51a9af7c..e7204e6c0ca4 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -68,6 +68,11 @@ struct plpks_var_name_list {
struct plpks_var_name varlist[];
};
+/**
+ * Updates the authenticated variable. It expects NULL as the component.
+ */
+int plpks_signed_update_var(struct plpks_var *var, u64 flags);
+
/**
* Writes the specified var and its data to PKS.
* Any caller of PKS driver should present a valid component type for
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 1189246b03dc..cee06fb9a370 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -81,6 +81,12 @@ static int pseries_status_to_err(int rc)
err = -ENOENT;
break;
case H_BUSY:
+ case H_LONG_BUSY_ORDER_1_MSEC:
+ case H_LONG_BUSY_ORDER_10_MSEC:
+ case H_LONG_BUSY_ORDER_100_MSEC:
+ case H_LONG_BUSY_ORDER_1_SEC:
+ case H_LONG_BUSY_ORDER_10_SEC:
+ case H_LONG_BUSY_ORDER_100_SEC:
err = -EBUSY;
break;
case H_AUTHORITY:
@@ -184,14 +190,17 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
u16 namelen)
{
struct label *label;
- size_t slen;
+ size_t slen = 0;
if (!name || namelen > PLPKS_MAX_NAME_SIZE)
return ERR_PTR(-EINVAL);
- slen = strlen(component);
- if (component && slen > sizeof(label->attr.prefix))
- return ERR_PTR(-EINVAL);
+ // Support NULL component for signed updates
+ if (component) {
+ slen = strlen(component);
+ if (slen > sizeof(label->attr.prefix))
+ return ERR_PTR(-EINVAL);
+ }
// The label structure must not cross a page boundary, so we align to
the next power of 2
label = kzalloc(roundup_pow_of_two(sizeof(*label)), GFP_KERNEL);
@@ -397,6 +406,61 @@ static int plpks_confirm_object_flushed(struct label
*label,
return pseries_status_to_err(rc);
}
+int plpks_signed_update_var(struct plpks_var *var, u64 flags)
+{
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
+ int rc;
+ struct label *label;
+ struct plpks_auth *auth;
+ u64 continuetoken = 0;
+ u64 timeout = 0;
+
+ if (!var->data || var->datalen <= 0 || var->namelen >
PLPKS_MAX_NAME_SIZE)
+ return -EINVAL;
+
+ if (!(var->policy & PLPKS_SIGNEDUPDATE))
+ return -EINVAL;
+
+ // Signed updates need the component to be NULL.
+ if (var->component)
+ return -EINVAL;
+
+ auth = construct_auth(PLPKS_OS_OWNER);
+ if (IS_ERR(auth))
+ return PTR_ERR(auth);
+
+ label = construct_label(var->component, var->os, var->name,
var->namelen);
+
[PATCH v6 26/26] integrity/powerpc: Support loading keys from PLPKS
From: Russell Currey
Add support for loading keys from the PLPKS on pseries machines, with the
"ibm,plpks-sb-v1" format.
The object format is expected to be the same, so there shouldn't be any
functional differences between objects retrieved on powernv or pseries.
Unlike on powernv, on pseries the format string isn't contained in the
device tree. Use secvar_ops->format() to fetch the format string in a
generic manner, rather than searching the device tree ourselves.
(The current code searches the device tree for a node compatible with
"ibm,edk2-compat-v1". This patch switches to calling secvar_ops->format(),
which in the case of OPAL/powernv means opal_secvar_format(), which
searches the device tree for a node compatible with "ibm,secvar-backend"
and checks its "format" property. These are equivalent, as skiboot creates
a node with both "ibm,edk2-compat-v1" and "ibm,secvar-backend" as
compatible strings.)
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v3: New patch
v4: Pass format buffer size (stefanb, npiggin)
v5: Use sizeof(buf) rather than stating the size twice (npiggin)
Clarify change to DT compatible strings in commit message (zohar)
Reword commit message a bit (ajd)
---
.../integrity/platform_certs/load_powerpc.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/security/integrity/platform_certs/load_powerpc.c
b/security/integrity/platform_certs/load_powerpc.c
index dee51606d5f4..b9de70b90826 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -10,7 +10,6 @@
#include
#include
#include
-#include
#include
#include
#include "keyring_handler.h"
@@ -59,16 +58,22 @@ static int __init load_powerpc_certs(void)
void *db = NULL, *dbx = NULL;
u64 dbsize = 0, dbxsize = 0;
int rc = 0;
- struct device_node *node;
+ ssize_t len;
+ char buf[32];
if (!secvar_ops)
return -ENODEV;
- /* The following only applies for the edk2-compat backend. */
- node = of_find_compatible_node(NULL, NULL, "ibm,edk2-compat-v1");
- if (!node)
+ len = secvar_ops->format(buf, sizeof(buf));
+ if (len <= 0)
return -ENODEV;
+ // Check for known secure boot implementations from OPAL or PLPKS
+ if (strcmp("ibm,edk2-compat-v1", buf) && strcmp("ibm,plpks-sb-v1",
buf)) {
+ pr_err("Unsupported secvar implementation \"%s\", not loading
certs\n", buf);
+ return -ENODEV;
+ }
+
/*
* Get db, and dbx. They might not exist, so it isn't an error if we
* can't get them.
@@ -103,8 +108,6 @@ static int __init load_powerpc_certs(void)
kfree(dbx);
}
- of_node_put(node);
-
return rc;
}
late_initcall(load_powerpc_certs);
--
2.39.1
[PATCH v6 10/26] powerpc/secvar: Extend sysfs to include config vars
From: Russell Currey
The forthcoming pseries consumer of the secvar API wants to expose a
number of config variables. Allowing secvar implementations to provide
their own sysfs attributes makes it easy for consumers to expose what
they need to.
This is not being used by the OPAL secvar implementation at present, and
the config directory will not be created if no attributes are set.
Signed-off-by: Russell Currey
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v3: Remove unnecessary "secvar:" prefix from error messages (ajd)
Merge config attributes into secvar_operations (mpe)
---
arch/powerpc/include/asm/secvar.h | 2 ++
arch/powerpc/kernel/secvar-sysfs.c | 33 +-
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index bf396215903d..011a53a8076c 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -10,6 +10,7 @@
#include
#include
+#include
extern const struct secvar_operations *secvar_ops;
@@ -19,6 +20,7 @@ struct secvar_operations {
int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
ssize_t (*format)(char *buf, size_t bufsize);
int (*max_size)(u64 *max_size);
+ const struct attribute **config_attrs;
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 8f3deff94009..7df32be86507 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -144,6 +144,19 @@ static int update_kobj_size(void)
return 0;
}
+static int secvar_sysfs_config(struct kobject *kobj)
+{
+ struct attribute_group config_group = {
+ .name = "config",
+ .attrs = (struct attribute **)secvar_ops->config_attrs,
+ };
+
+ if (secvar_ops->config_attrs)
+ return sysfs_create_group(kobj, _group);
+
+ return 0;
+}
+
static int secvar_sysfs_load(void)
{
struct kobject *kobj;
@@ -208,26 +221,36 @@ static int secvar_sysfs_init(void)
rc = sysfs_create_file(secvar_kobj, _attr.attr);
if (rc) {
- kobject_put(secvar_kobj);
- return -ENOMEM;
+ pr_err("Failed to create format object\n");
+ rc = -ENOMEM;
+ goto err;
}
secvar_kset = kset_create_and_add("vars", NULL, secvar_kobj);
if (!secvar_kset) {
pr_err("sysfs kobject registration failed\n");
- kobject_put(secvar_kobj);
- return -ENOMEM;
+ rc = -ENOMEM;
+ goto err;
}
rc = update_kobj_size();
if (rc) {
pr_err("Cannot read the size of the attribute\n");
- return rc;
+ goto err;
+ }
+
+ rc = secvar_sysfs_config(secvar_kobj);
+ if (rc) {
+ pr_err("Failed to create config directory\n");
+ goto err;
}
secvar_sysfs_load();
return 0;
+err:
+ kobject_put(secvar_kobj);
+ return rc;
}
late_initcall(secvar_sysfs_init);
--
2.39.1
[PATCH v6 22/26] powerpc/pseries: Add helper to get PLPKS password length
From: Russell Currey
Add helper function to get the PLPKS password length. This will be used
in a later patch to support passing the password between kernels over
kexec.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v3: New patch
v5: Drop plpks_get_password() since we no longer need to expose it.
---
arch/powerpc/include/asm/plpks.h | 5 +
arch/powerpc/platforms/pseries/plpks.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 0c49969b0864..757313e00521 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -171,6 +171,11 @@ u32 plpks_get_maxlargeobjectsize(void);
*/
u64 plpks_get_signedupdatealgorithms(void);
+/**
+ * Returns the length of the PLPKS password in bytes.
+ */
+u16 plpks_get_passwordlen(void);
+
#endif // CONFIG_PSERIES_PLPKS
#endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 01ae919b4497..671a10acaebf 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -359,6 +359,11 @@ u64 plpks_get_signedupdatealgorithms(void)
return signedupdatealgorithms;
}
+u16 plpks_get_passwordlen(void)
+{
+ return ospasswordlength;
+}
+
bool plpks_is_available(void)
{
int rc;
--
2.39.1
[PATCH v6 19/26] powerpc/pseries: Make caller pass buffer to plpks_read_var()
Currently, plpks_read_var() allocates a buffer to pass to the
H_PKS_READ_OBJECT hcall, then allocates another buffer into which the data
is copied, and returns that buffer to the caller.
This is a bit over the top - while we probably still want to allocate a
separate buffer to pass to the hypervisor in the hcall, we can let the
caller allocate the final buffer and specify the size.
Don't allocate var->data in plpks_read_var(), instead expect the caller to
allocate it. If the caller needs to discover the size, it can set
var->data to NULL and var->datalen will be populated. Update header file
to document this.
Suggested-by: Michael Ellerman
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
Reviewed-by: Stefan Berger
---
v3: New patch (mpe)
v6: Reword commit message (stefanb)
---
arch/powerpc/include/asm/plpks.h | 12
arch/powerpc/platforms/pseries/plpks.c | 11 ---
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index e7204e6c0ca4..0c49969b0864 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -88,16 +88,28 @@ int plpks_remove_var(char *component, u8 varos,
/**
* Returns the data for the specified os variable.
+ *
+ * Caller must allocate a buffer in var->data with length in var->datalen.
+ * If no buffer is provided, var->datalen will be populated with the object's
+ * size.
*/
int plpks_read_os_var(struct plpks_var *var);
/**
* Returns the data for the specified firmware variable.
+ *
+ * Caller must allocate a buffer in var->data with length in var->datalen.
+ * If no buffer is provided, var->datalen will be populated with the object's
+ * size.
*/
int plpks_read_fw_var(struct plpks_var *var);
/**
* Returns the data for the specified bootloader variable.
+ *
+ * Caller must allocate a buffer in var->data with length in var->datalen.
+ * If no buffer is provided, var->datalen will be populated with the object's
+ * size.
*/
int plpks_read_bootloader_var(struct plpks_var *var);
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index e5755443d4a4..926b6a927326 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -581,17 +581,14 @@ static int plpks_read_var(u8 consumer, struct plpks_var
*var)
goto out_free_output;
}
- if (var->datalen == 0 || var->datalen > retbuf[0])
+ if (!var->data || var->datalen > retbuf[0])
var->datalen = retbuf[0];
- var->data = kzalloc(var->datalen, GFP_KERNEL);
- if (!var->data) {
- rc = -ENOMEM;
- goto out_free_output;
- }
var->policy = retbuf[1];
- memcpy(var->data, output, var->datalen);
+ if (var->data)
+ memcpy(var->data, output, var->datalen);
+
rc = 0;
out_free_output:
--
2.39.1
[PATCH v6 15/26] powerpc/pseries: Move PLPKS constants to header file
From: Russell Currey
Move the constants defined in plpks.c to plpks.h, and standardise their
naming, so that PLPKS consumers can make use of them later on.
Signed-off-by: Russell Currey
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v3: New patch
---
arch/powerpc/include/asm/plpks.h | 36 +---
arch/powerpc/platforms/pseries/plpks.c | 57 ++
2 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 8295502ee93b..6466aadd7145 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -14,14 +14,40 @@
#include
#include
-#define OSSECBOOTAUDIT 0x4000
-#define OSSECBOOTENFORCE 0x2000
-#define WORLDREADABLE 0x0800
-#define SIGNEDUPDATE 0x0100
+// Object policy flags from supported_policies
+#define PLPKS_OSSECBOOTAUDIT PPC_BIT32(1) // OS secure boot must be
audit/enforce
+#define PLPKS_OSSECBOOTENFORCE PPC_BIT32(2) // OS secure boot must be enforce
+#define PLPKS_PWSETPPC_BIT32(3) // No access without password set
+#define PLPKS_WORLDREADABLEPPC_BIT32(4) // Readable without authentication
+#define PLPKS_IMMUTABLEPPC_BIT32(5) // Once written, object
cannot be removed
+#define PLPKS_TRANSIENTPPC_BIT32(6) // Object does not persist
through reboot
+#define PLPKS_SIGNEDUPDATE PPC_BIT32(7) // Object can only be modified by
signed updates
+#define PLPKS_HVPROVISIONEDPPC_BIT32(28) // Hypervisor has provisioned
this object
-#define PLPKS_VAR_LINUX0x02
+// Signature algorithm flags from signed_update_algorithms
+#define PLPKS_ALG_RSA2048 PPC_BIT(0)
+#define PLPKS_ALG_RSA4096 PPC_BIT(1)
+
+// Object label OS metadata flags
+#define PLPKS_VAR_LINUX0x02
#define PLPKS_VAR_COMMON 0x04
+// Flags for which consumer owns an object is owned by
+#define PLPKS_FW_OWNER 0x1
+#define PLPKS_BOOTLOADER_OWNER 0x2
+#define PLPKS_OS_OWNER 0x3
+
+// Flags for label metadata fields
+#define PLPKS_LABEL_VERSION0
+#define PLPKS_MAX_LABEL_ATTR_SIZE 16
+#define PLPKS_MAX_NAME_SIZE239
+#define PLPKS_MAX_DATA_SIZE4000
+
+// Timeouts for PLPKS operations
+#define PLPKS_MAX_TIMEOUT 5000 // msec
+#define PLPKS_FLUSH_SLEEP 10 // msec
+#define PLPKS_FLUSH_SLEEP_RANGE400
+
struct plpks_var {
char *component;
u8 *name;
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 13e6daadb179..91f3f623a2c7 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -20,19 +20,6 @@
#include
#include
-#define PKS_FW_OWNER0x1
-#define PKS_BOOTLOADER_OWNER 0x2
-#define PKS_OS_OWNER0x3
-
-#define LABEL_VERSION 0
-#define MAX_LABEL_ATTR_SIZE 16
-#define MAX_NAME_SIZE 239
-#define MAX_DATA_SIZE 4000
-
-#define PKS_FLUSH_MAX_TIMEOUT 5000 //msec
-#define PKS_FLUSH_SLEEP 10 //msec
-#define PKS_FLUSH_SLEEP_RANGE 400
-
static u8 *ospassword;
static u16 ospasswordlength;
@@ -59,7 +46,7 @@ struct label_attr {
struct label {
struct label_attr attr;
- u8 name[MAX_NAME_SIZE];
+ u8 name[PLPKS_MAX_NAME_SIZE];
size_t size;
};
@@ -122,7 +109,7 @@ static int pseries_status_to_err(int rc)
static int plpks_gen_password(void)
{
unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
- u8 *password, consumer = PKS_OS_OWNER;
+ u8 *password, consumer = PLPKS_OS_OWNER;
int rc;
// The password must not cross a page boundary, so we align to the next
power of 2
@@ -159,7 +146,7 @@ static struct plpks_auth *construct_auth(u8 consumer)
{
struct plpks_auth *auth;
- if (consumer > PKS_OS_OWNER)
+ if (consumer > PLPKS_OS_OWNER)
return ERR_PTR(-EINVAL);
// The auth structure must not cross a page boundary and must be
@@ -171,7 +158,7 @@ static struct plpks_auth *construct_auth(u8 consumer)
auth->version = 1;
auth->consumer = consumer;
- if (consumer == PKS_FW_OWNER || consumer == PKS_BOOTLOADER_OWNER)
+ if (consumer == PLPKS_FW_OWNER || consumer == PLPKS_BOOTLOADER_OWNER)
return auth;
memcpy(auth->password, ospassword, ospasswordlength);
@@ -191,7 +178,7 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
struct label *label;
size_t slen;
- if (!name || namelen > MAX_NAME_SIZE)
+ if (!name || namelen > PLPKS_MAX_NAME_SIZE)
return ERR_PTR(-EINVAL);
slen = strlen(component);
@@ -206,9 +193,9 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
if (component)
memcpy(&g
[PATCH v6 23/26] powerpc/pseries: Pass PLPKS password on kexec
From: Russell Currey
Before interacting with the PLPKS, we ask the hypervisor to generate a
password for the current boot, which is then required for most further
PLPKS operations.
If we kexec into a new kernel, the new kernel will try and fail to
generate a new password, as the password has already been set.
Pass the password through to the new kernel via the device tree, in
/chosen/ibm,plpks-pw. Check for the presence of this property before
trying to generate a new password - if it exists, use the existing
password and remove it from the device tree.
This only works with the kexec_file_load() syscall, not the older
kexec_load() syscall, however if you're using Secure Boot then you want
to be using kexec_file_load() anyway.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
v3: New patch
v4: Fix compile when CONFIG_PSERIES_PLPKS=n (snowpatch)
Fix error handling on fdt_path_offset() call (ruscur)
v5: Fix DT property name in commit message (npiggin)
Clear prop in FDT during init to prevent password exposure (mpe)
Rework to remove ifdefs from C code (npiggin)
v6: Rebase on top of 7294194b47e994753a86eee8cf1c61f3f36458a3 and
fc546faa559538fb312c77e055243ece18ab3288
Whitespace (stefanb)
Use more const (stefanb)
Get rid of FDT extra space allocation for node overhead, as it
shouldn't be necessary (ruscur)
Note kexec_file_load() restriction in commit message
---
arch/powerpc/include/asm/plpks.h | 14 ++
arch/powerpc/kernel/prom.c | 4 ++
arch/powerpc/kexec/file_load_64.c | 18 +---
arch/powerpc/platforms/pseries/plpks.c | 61 ++
4 files changed, 92 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 757313e00521..23b77027c916 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -176,6 +176,20 @@ u64 plpks_get_signedupdatealgorithms(void);
*/
u16 plpks_get_passwordlen(void);
+/**
+ * Called in early init to retrieve and clear the PLPKS password from the DT.
+ */
+void plpks_early_init_devtree(void);
+
+/**
+ * Populates the FDT with the PLPKS password to prepare for kexec.
+ */
+int plpks_populate_fdt(void *fdt);
+#else // CONFIG_PSERIES_PLPKS
+static inline bool plpks_is_available(void) { return false; }
+static inline u16 plpks_get_passwordlen(void) { BUILD_BUG(); }
+static inline void plpks_early_init_devtree(void) { }
+static inline int plpks_populate_fdt(void *fdt) { BUILD_BUG(); }
#endif // CONFIG_PSERIES_PLPKS
#endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4f1c920aa13e..8a13b378770f 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
@@ -893,6 +894,9 @@ void __init early_init_devtree(void *params)
powerpc_firmware_features |= FW_FEATURE_PS3_POSSIBLE;
#endif
+ /* If kexec left a PLPKS password in the DT, get it and clear it */
+ plpks_early_init_devtree();
+
tm_init();
DBG(" <- early_init_devtree()\n");
diff --git a/arch/powerpc/kexec/file_load_64.c
b/arch/powerpc/kexec/file_load_64.c
index 52085751f5f4..8a9469e1ce71 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
struct umem_info {
u64 *buf; /* data buffer for usable-memory property */
@@ -977,12 +978,17 @@ static unsigned int cpu_node_size(void)
*/
unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
{
- unsigned int cpu_nodes, extra_size;
+ unsigned int cpu_nodes, extra_size = 0;
struct device_node *dn;
u64 usm_entries;
+ // Budget some space for the password blob. There's already extra space
+ // for the key name
+ if (plpks_is_available())
+ extra_size += (unsigned int)plpks_get_passwordlen();
+
if (image->type != KEXEC_TYPE_CRASH)
- return 0;
+ return extra_size;
/*
* For kdump kernel, account for linux,usable-memory and
@@ -992,9 +998,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage
*image)
if (drmem_lmb_size()) {
usm_entries = ((memory_hotplug_max() / drmem_lmb_size()) +
(2 * (resource_size(_res) /
drmem_lmb_size(;
- extra_size = (unsigned int)(usm_entries * sizeof(u64));
- } else {
- extra_size = 0;
+ extra_size += (unsigned int)(usm_entries * sizeof(u64));
}
/*
@@ -1233,6 +1237,10 @@ int setup_new_fdt_ppc64(const struct kimage *image, void
*fdt,
}
}
+ // If we have PLPKS active, we need to provide the password to the new
kernel
+ if (plpks_is_available())
+
[PATCH v6 25/26] integrity/powerpc: Improve error handling & reporting when loading certs
From: Russell Currey
A few improvements to load_powerpc.c:
- include integrity.h for the pr_fmt()
- move all error reporting out of get_cert_list()
- use ERR_PTR() to better preserve error detail
- don't use pr_err() for missing keys
Reviewed-by: Mimi Zohar
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
v3: New patch
---
.../integrity/platform_certs/load_powerpc.c | 26 ++-
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/security/integrity/platform_certs/load_powerpc.c
b/security/integrity/platform_certs/load_powerpc.c
index 1e4f80a4e71c..dee51606d5f4 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -14,9 +14,15 @@
#include
#include
#include "keyring_handler.h"
+#include "../integrity.h"
/*
* Get a certificate list blob from the named secure variable.
+ *
+ * Returns:
+ * - a pointer to a kmalloc'd buffer containing the cert list on success
+ * - NULL if the key does not exist
+ * - an ERR_PTR on error
*/
static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size)
{
@@ -25,19 +31,19 @@ static __init void *get_cert_list(u8 *key, unsigned long
keylen, u64 *size)
rc = secvar_ops->get(key, keylen, NULL, size);
if (rc) {
- pr_err("Couldn't get size: %d\n", rc);
- return NULL;
+ if (rc == -ENOENT)
+ return NULL;
+ return ERR_PTR(rc);
}
db = kmalloc(*size, GFP_KERNEL);
if (!db)
- return NULL;
+ return ERR_PTR(-ENOMEM);
rc = secvar_ops->get(key, keylen, db, size);
if (rc) {
kfree(db);
- pr_err("Error reading %s var: %d\n", key, rc);
- return NULL;
+ return ERR_PTR(rc);
}
return db;
@@ -69,7 +75,11 @@ static int __init load_powerpc_certs(void)
*/
db = get_cert_list("db", 3, );
if (!db) {
- pr_err("Couldn't get db list from firmware\n");
+ pr_info("Couldn't get db list from firmware\n");
+ } else if (IS_ERR(db)) {
+ rc = PTR_ERR(db);
+ pr_err("Error reading db from firmware: %d\n", rc);
+ return rc;
} else {
rc = parse_efi_signature_list("powerpc:db", db, dbsize,
get_handler_for_db);
@@ -81,6 +91,10 @@ static int __init load_powerpc_certs(void)
dbx = get_cert_list("dbx", 4, );
if (!dbx) {
pr_info("Couldn't get dbx list from firmware\n");
+ } else if (IS_ERR(dbx)) {
+ rc = PTR_ERR(dbx);
+ pr_err("Error reading dbx from firmware: %d\n", rc);
+ return rc;
} else {
rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize,
get_handler_for_dbx);
--
2.39.1
[PATCH v6 21/26] powerpc/pseries: Clarify warning when PLPKS password already set
When the H_PKS_GEN_PASSWORD hcall returns H_IN_USE, operations that require
authentication (i.e. anything other than reading a world-readable variable)
will not work.
The current error message doesn't explain this clearly enough. Reword it
to emphasise that authenticated operations will fail.
Signed-off-by: Andrew Donnellan
---
v6: New patch
---
arch/powerpc/platforms/pseries/plpks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 926b6a927326..01ae919b4497 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -146,7 +146,7 @@ static int plpks_gen_password(void)
memcpy(ospassword, password, ospasswordlength);
} else {
if (rc == H_IN_USE) {
- pr_warn("Password is already set for POWER LPAR
Platform KeyStore\n");
+ pr_warn("Password already set - authenticated
operations will fail\n");
rc = 0;
} else {
goto out;
--
2.39.1
[PATCH v6 16/26] powerpc/pseries: Expose PLPKS config values, support additional fields
From: Nayna Jain
The plpks driver uses the H_PKS_GET_CONFIG hcall to retrieve configuration
and status information about the PKS from the hypervisor.
Update _plpks_get_config() to handle some additional fields. Add getter
functions to allow the PKS configuration information to be accessed from
other files. Validate that the values we're getting comply with the spec.
While we're here, move the config struct in _plpks_get_config() off the
stack - it's getting large and we also need to make sure it doesn't cross
a page boundary.
Signed-off-by: Nayna Jain
[ajd: split patch, extend to support additional v3 API fields, minor fixes]
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
Reviewed-by: Stefan Berger
---
v3: Merge plpks fixes and signed update series with secvar series
Refresh config values in plpks_get_usedspace() (ajd)
Validate the config values being returned comply with spec (ruscur)
Return maxobjlabelsize as is (ruscur)
Move plpks.h to include/asm (ruscur)
Fix checkpatch checks (ruscur)
---
arch/powerpc/include/asm/plpks.h | 58 ++
arch/powerpc/platforms/pseries/plpks.c | 149 +++--
2 files changed, 195 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 6466aadd7145..7c5f51a9af7c 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -96,6 +96,64 @@ int plpks_read_fw_var(struct plpks_var *var);
*/
int plpks_read_bootloader_var(struct plpks_var *var);
+/**
+ * Returns if PKS is available on this LPAR.
+ */
+bool plpks_is_available(void);
+
+/**
+ * Returns version of the Platform KeyStore.
+ */
+u8 plpks_get_version(void);
+
+/**
+ * Returns hypervisor storage overhead per object, not including the size of
+ * the object or label. Only valid for config version >= 2
+ */
+u16 plpks_get_objoverhead(void);
+
+/**
+ * Returns maximum password size. Must be >= 32 bytes
+ */
+u16 plpks_get_maxpwsize(void);
+
+/**
+ * Returns maximum object size supported by Platform KeyStore.
+ */
+u16 plpks_get_maxobjectsize(void);
+
+/**
+ * Returns maximum object label size supported by Platform KeyStore.
+ */
+u16 plpks_get_maxobjectlabelsize(void);
+
+/**
+ * Returns total size of the configured Platform KeyStore.
+ */
+u32 plpks_get_totalsize(void);
+
+/**
+ * Returns used space from the total size of the Platform KeyStore.
+ */
+u32 plpks_get_usedspace(void);
+
+/**
+ * Returns bitmask of policies supported by the hypervisor.
+ */
+u32 plpks_get_supportedpolicies(void);
+
+/**
+ * Returns maximum byte size of a single object supported by the hypervisor.
+ * Only valid for config version >= 3
+ */
+u32 plpks_get_maxlargeobjectsize(void);
+
+/**
+ * Returns bitmask of signature algorithms supported for signed updates.
+ * Only valid for config version >= 3
+ */
+u64 plpks_get_signedupdatealgorithms(void);
+
#endif // CONFIG_PSERIES_PLPKS
#endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 91f3f623a2c7..1189246b03dc 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -24,8 +24,16 @@ static u8 *ospassword;
static u16 ospasswordlength;
// Retrieved with H_PKS_GET_CONFIG
+static u8 version;
+static u16 objoverhead;
static u16 maxpwsize;
static u16 maxobjsize;
+static s16 maxobjlabelsize;
+static u32 totalsize;
+static u32 usedspace;
+static u32 supportedpolicies;
+static u32 maxlargeobjectsize;
+static u64 signedupdatealgorithms;
struct plpks_auth {
u8 version;
@@ -206,32 +214,149 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
static int _plpks_get_config(void)
{
unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
- struct {
+ struct config {
u8 version;
u8 flags;
- __be32 rsvd0;
+ __be16 rsvd0;
+ __be16 objoverhead;
__be16 maxpwsize;
__be16 maxobjlabelsize;
__be16 maxobjsize;
__be32 totalsize;
__be32 usedspace;
__be32 supportedpolicies;
- __be64 rsvd1;
- } __packed config;
+ __be32 maxlargeobjectsize;
+ __be64 signedupdatealgorithms;
+ u8 rsvd1[476];
+ } __packed * config;
size_t size;
- int rc;
+ int rc = 0;
+
+ size = sizeof(*config);
+
+ // Config struct must not cross a page boundary. So long as the struct
+ // size is a power of 2, this should be fine as alignment is guaranteed
+ config = kzalloc(size, GFP_KERNEL);
+ if (!config) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ rc = plpar_hcall(H_PKS_GET_CONFIG, retbuf, virt_to_phys(config), size);
+
+ if (rc
[PATCH v6 18/26] powerpc/pseries: Log hcall return codes for PLPKS debug
From: Russell Currey
The plpks code converts hypervisor return codes into their Linux
equivalents so that users can understand them. Having access to the
original return codes is really useful for debugging, so add a
pr_debug() so we don't lose information from the conversion.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
arch/powerpc/platforms/pseries/plpks.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index cee06fb9a370..e5755443d4a4 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -117,6 +117,8 @@ static int pseries_status_to_err(int rc)
err = -EINVAL;
}
+ pr_debug("Converted hypervisor code %d to Linux %d\n", rc, err);
+
return err;
}
--
2.39.1
[PATCH v6 13/26] powerpc/secvar: Don't print error on ENOENT when reading variables
If attempting to read the size or data attributes of a non-existent
variable (which will be possible after a later patch to expose the PLPKS
via the secvar interface), don't spam the kernel log with error messages.
Only print errors for return codes that aren't ENOENT.
Reported-by: Sudhakar Kuppusamy
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v3: New patch
---
arch/powerpc/kernel/secvar-sysfs.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 6ba23b2bb9da..eb3c053f323f 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -43,8 +43,8 @@ static ssize_t size_show(struct kobject *kobj, struct
kobj_attribute *attr,
rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, );
if (rc) {
- pr_err("Error retrieving %s variable size %d\n", kobj->name,
- rc);
+ if (rc != -ENOENT)
+ pr_err("Error retrieving %s variable size %d\n",
kobj->name, rc);
return rc;
}
@@ -61,7 +61,8 @@ static ssize_t data_read(struct file *filep, struct kobject
*kobj,
rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, );
if (rc) {
- pr_err("Error getting %s variable size %d\n", kobj->name, rc);
+ if (rc != -ENOENT)
+ pr_err("Error getting %s variable size %d\n",
kobj->name, rc);
return rc;
}
pr_debug("dsize is %llu\n", dsize);
--
2.39.1
[PATCH v6 11/26] powerpc/secvar: Allow backend to populate static list of variable names
Currently, the list of variables is populated by calling
secvar_ops->get_next() repeatedly, which is explicitly modelled on the
OPAL API (including the keylen parameter).
For the upcoming PLPKS backend, we have a static list of variable names.
It is messy to fit that into get_next(), so instead, let the backend put
a NULL-terminated array of variable names into secvar_ops->var_names,
which will be used if get_next() is undefined.
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
Reviewed-by: Stefan Berger
---
v3: New patch (ajd/mpe)
v6: Add newlines for better aesthetics (stefanb)
---
arch/powerpc/include/asm/secvar.h | 4 ++
arch/powerpc/kernel/secvar-sysfs.c | 69 +-
2 files changed, 52 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index 011a53a8076c..4828e0ab7e3c 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -21,6 +21,10 @@ struct secvar_operations {
ssize_t (*format)(char *buf, size_t bufsize);
int (*max_size)(u64 *max_size);
const struct attribute **config_attrs;
+
+ // NULL-terminated array of fixed variable names
+ // Only used if get_next() isn't provided
+ const char * const *var_names;
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 7df32be86507..bfb19f22c6ba 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -157,9 +157,31 @@ static int secvar_sysfs_config(struct kobject *kobj)
return 0;
}
-static int secvar_sysfs_load(void)
+static int add_var(const char *name)
{
struct kobject *kobj;
+ int rc;
+
+ kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
+ if (!kobj)
+ return -ENOMEM;
+
+ kobject_init(kobj, _ktype);
+
+ rc = kobject_add(kobj, _kset->kobj, "%s", name);
+ if (rc) {
+ pr_warn("kobject_add error %d for attribute: %s\n", rc,
+ name);
+ kobject_put(kobj);
+ return rc;
+ }
+
+ kobject_uevent(kobj, KOBJ_ADD);
+ return 0;
+}
+
+static int secvar_sysfs_load(void)
+{
u64 namesize = 0;
char *name;
int rc;
@@ -179,31 +201,28 @@ static int secvar_sysfs_load(void)
break;
}
- kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
- if (!kobj) {
- rc = -ENOMEM;
- break;
- }
-
- kobject_init(kobj, _ktype);
-
- rc = kobject_add(kobj, _kset->kobj, "%s", name);
- if (rc) {
- pr_warn("kobject_add error %d for attribute: %s\n", rc,
- name);
- kobject_put(kobj);
- kobj = NULL;
- }
-
- if (kobj)
- kobject_uevent(kobj, KOBJ_ADD);
-
+ rc = add_var(name);
} while (!rc);
kfree(name);
return rc;
}
+static int secvar_sysfs_load_static(void)
+{
+ const char * const *name_ptr = secvar_ops->var_names;
+ int rc;
+
+ while (*name_ptr) {
+ rc = add_var(*name_ptr);
+ if (rc)
+ return rc;
+ name_ptr++;
+ }
+
+ return 0;
+}
+
static int secvar_sysfs_init(void)
{
int rc;
@@ -245,7 +264,15 @@ static int secvar_sysfs_init(void)
goto err;
}
- secvar_sysfs_load();
+ if (secvar_ops->get_next)
+ rc = secvar_sysfs_load();
+ else
+ rc = secvar_sysfs_load_static();
+
+ if (rc) {
+ pr_err("Failed to create variable attributes\n");
+ goto err;
+ }
return 0;
err:
--
2.39.1
[PATCH v6 12/26] powerpc/secvar: Warn when PAGE_SIZE is smaller than max object size
Due to sysfs constraints, when writing to a variable, we can only handle
writes of up to PAGE_SIZE.
It's possible that the maximum object size is larger than PAGE_SIZE, in
which case, print a warning on boot so that the user is aware.
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
Reviewed-by: Stefan Berger
---
v3: New patch (ajd)
---
arch/powerpc/kernel/secvar-sysfs.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index bfb19f22c6ba..6ba23b2bb9da 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -225,6 +225,7 @@ static int secvar_sysfs_load_static(void)
static int secvar_sysfs_init(void)
{
+ u64 max_size;
int rc;
if (!secvar_ops) {
@@ -274,6 +275,14 @@ static int secvar_sysfs_init(void)
goto err;
}
+ // Due to sysfs limitations, we will only ever get a write buffer of
+ // up to 1 page in size. Print a warning if this is potentially going
+ // to cause problems, so that the user is aware.
+ secvar_ops->max_size(_size);
+ if (max_size > PAGE_SIZE)
+ pr_warn_ratelimited("PAGE_SIZE (%lu) is smaller than maximum
object size (%llu), writes are limited to PAGE_SIZE\n",
+ PAGE_SIZE, max_size);
+
return 0;
err:
kobject_put(secvar_kobj);
--
2.39.1
[PATCH v6 05/26] powerpc/secvar: Warn and error if multiple secvar ops are set
From: Russell Currey
The secvar code only supports one consumer at a time.
Multiple consumers aren't possible at this point in time, but we'd want
it to be obvious if it ever could happen.
Signed-off-by: Russell Currey
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
---
v4: Return an error and don't actually try to set secvar_operations if the
warning is triggered (npiggin)
v5: Drop "extern" to fix a checkpatch check (snowpatch)
v6: Return -EBUSY rather than -1 (stefanb)
---
arch/powerpc/include/asm/secvar.h| 4 ++--
arch/powerpc/kernel/secvar-ops.c | 10 --
arch/powerpc/platforms/powernv/opal-secvar.c | 4 +---
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index 07ba36f868a7..a2b5f2203dc5 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -21,11 +21,11 @@ struct secvar_operations {
#ifdef CONFIG_PPC_SECURE_BOOT
-extern void set_secvar_ops(const struct secvar_operations *ops);
+int set_secvar_ops(const struct secvar_operations *ops);
#else
-static inline void set_secvar_ops(const struct secvar_operations *ops) { }
+static inline int set_secvar_ops(const struct secvar_operations *ops) { return
0; }
#endif
diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c
index 6a29777d6a2d..19172a2804f0 100644
--- a/arch/powerpc/kernel/secvar-ops.c
+++ b/arch/powerpc/kernel/secvar-ops.c
@@ -8,10 +8,16 @@
#include
#include
+#include
-const struct secvar_operations *secvar_ops __ro_after_init;
+const struct secvar_operations *secvar_ops __ro_after_init = NULL;
-void set_secvar_ops(const struct secvar_operations *ops)
+int set_secvar_ops(const struct secvar_operations *ops)
{
+ if (WARN_ON_ONCE(secvar_ops))
+ return -EBUSY;
+
secvar_ops = ops;
+
+ return 0;
}
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c
b/arch/powerpc/platforms/powernv/opal-secvar.c
index ef89861569e0..4c0a3b030fe0 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -113,9 +113,7 @@ static int opal_secvar_probe(struct platform_device *pdev)
return -ENODEV;
}
- set_secvar_ops(_secvar_ops);
-
- return 0;
+ return set_secvar_ops(_secvar_ops);
}
static const struct of_device_id opal_secvar_match[] = {
--
2.39.1
[PATCH v6 07/26] powerpc/secvar: Handle format string in the consumer
From: Russell Currey
The code that handles the format string in secvar-sysfs.c is entirely
OPAL specific, so create a new "format" op in secvar_operations to make
the secvar code more generic. No functional change.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v2: Use sysfs_emit() instead of sprintf() (gregkh)
v3: Enforce format string size limit (ruscur)
v4: Pass the buffer size as an argument, not using a macro (stefanb,
npiggin)
Fix error reporting (npiggin)
---
arch/powerpc/include/asm/secvar.h| 1 +
arch/powerpc/kernel/secvar-sysfs.c | 27 +++-
arch/powerpc/platforms/powernv/opal-secvar.c | 25 ++
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index a2b5f2203dc5..1a2c696a48ad 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -17,6 +17,7 @@ struct secvar_operations {
int (*get)(const char *key, u64 key_len, u8 *data, u64 *data_size);
int (*get_next)(const char *key, u64 *key_len, u64 keybufsize);
int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
+ ssize_t (*format)(char *buf, size_t bufsize);
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index b786d1005027..e4661559c855 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -21,26 +21,17 @@ static struct kset *secvar_kset;
static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
- ssize_t rc = 0;
- struct device_node *node;
- const char *format;
-
- node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
- if (!of_device_is_available(node)) {
- rc = -ENODEV;
- goto out;
- }
+ char tmp[32];
+ ssize_t len = secvar_ops->format(tmp, sizeof(tmp));
- rc = of_property_read_string(node, "format", );
- if (rc)
- goto out;
+ if (len > 0)
+ return sysfs_emit(buf, "%s\n", tmp);
+ else if (len < 0)
+ pr_err("Error %zd reading format string\n", len);
+ else
+ pr_err("Got empty format string from backend\n");
- rc = sysfs_emit(buf, "%s\n", format);
-
-out:
- of_node_put(node);
-
- return rc;
+ return -EIO;
}
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c
b/arch/powerpc/platforms/powernv/opal-secvar.c
index 4c0a3b030fe0..e33bb703ecbc 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -98,10 +98,35 @@ static int opal_set_variable(const char *key, u64 ksize, u8
*data, u64 dsize)
return opal_status_to_err(rc);
}
+static ssize_t opal_secvar_format(char *buf, size_t bufsize)
+{
+ ssize_t rc = 0;
+ struct device_node *node;
+ const char *format;
+
+ node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
+ if (!of_device_is_available(node)) {
+ rc = -ENODEV;
+ goto out;
+ }
+
+ rc = of_property_read_string(node, "format", );
+ if (rc)
+ goto out;
+
+ rc = snprintf(buf, bufsize, "%s", format);
+
+out:
+ of_node_put(node);
+
+ return rc;
+}
+
static const struct secvar_operations opal_secvar_ops = {
.get = opal_get_variable,
.get_next = opal_get_next_variable,
.set = opal_set_variable,
+ .format = opal_secvar_format,
};
static int opal_secvar_probe(struct platform_device *pdev)
--
2.39.1
[PATCH v6 08/26] powerpc/secvar: Handle max object size in the consumer
From: Russell Currey
Currently the max object size is handled in the core secvar code with an
entirely OPAL-specific implementation, so create a new max_size() op and
move the existing implementation into the powernv platform. Should be
no functional change.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v3: Change uint64_t type to u64 (mpe)
v4: Return immediately if node is NULL (gjoyce)
---
arch/powerpc/include/asm/secvar.h| 1 +
arch/powerpc/kernel/secvar-sysfs.c | 17 +++
arch/powerpc/platforms/powernv/opal-secvar.c | 22
3 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index 1a2c696a48ad..bf396215903d 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -18,6 +18,7 @@ struct secvar_operations {
int (*get_next)(const char *key, u64 *key_len, u64 keybufsize);
int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
ssize_t (*format)(char *buf, size_t bufsize);
+ int (*max_size)(u64 *max_size);
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index e4661559c855..0966806f28c7 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -132,27 +132,16 @@ static struct kobj_type secvar_ktype = {
static int update_kobj_size(void)
{
- struct device_node *node;
u64 varsize;
- int rc = 0;
+ int rc = secvar_ops->max_size();
- node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
- if (!of_device_is_available(node)) {
- rc = -ENODEV;
- goto out;
- }
-
- rc = of_property_read_u64(node, "max-var-size", );
if (rc)
- goto out;
+ return rc;
data_attr.size = varsize;
update_attr.size = varsize;
-out:
- of_node_put(node);
-
- return rc;
+ return 0;
}
static int secvar_sysfs_load(void)
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c
b/arch/powerpc/platforms/powernv/opal-secvar.c
index e33bb703ecbc..a8436bf35e2f 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -122,11 +122,33 @@ static ssize_t opal_secvar_format(char *buf, size_t
bufsize)
return rc;
}
+static int opal_secvar_max_size(u64 *max_size)
+{
+ int rc;
+ struct device_node *node;
+
+ node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
+ if (!node)
+ return -ENODEV;
+
+ if (!of_device_is_available(node)) {
+ rc = -ENODEV;
+ goto out;
+ }
+
+ rc = of_property_read_u64(node, "max-var-size", max_size);
+
+out:
+ of_node_put(node);
+ return rc;
+}
+
static const struct secvar_operations opal_secvar_ops = {
.get = opal_get_variable,
.get_next = opal_get_next_variable,
.set = opal_set_variable,
.format = opal_secvar_format,
+ .max_size = opal_secvar_max_size,
};
static int opal_secvar_probe(struct platform_device *pdev)
--
2.39.1
[PATCH v6 09/26] powerpc/secvar: Clean up init error messages
Remove unnecessary prefixes from error messages in secvar_sysfs_init()
(the file defines pr_fmt, so putting "secvar:" in every message is
unnecessary). Make capitalisation and punctuation more consistent.
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
Reviewed-by: Stefan Berger
---
v3: New patch (ajd)
---
arch/powerpc/kernel/secvar-sysfs.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 0966806f28c7..8f3deff94009 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -196,13 +196,13 @@ static int secvar_sysfs_init(void)
int rc;
if (!secvar_ops) {
- pr_warn("secvar: failed to retrieve secvar operations.\n");
+ pr_warn("Failed to retrieve secvar operations\n");
return -ENODEV;
}
secvar_kobj = kobject_create_and_add("secvar", firmware_kobj);
if (!secvar_kobj) {
- pr_err("secvar: Failed to create firmware kobj\n");
+ pr_err("Failed to create firmware kobj\n");
return -ENOMEM;
}
@@ -214,7 +214,7 @@ static int secvar_sysfs_init(void)
secvar_kset = kset_create_and_add("vars", NULL, secvar_kobj);
if (!secvar_kset) {
- pr_err("secvar: sysfs kobject registration failed.\n");
+ pr_err("sysfs kobject registration failed\n");
kobject_put(secvar_kobj);
return -ENOMEM;
}
--
2.39.1
[PATCH v6 06/26] powerpc/secvar: Use sysfs_emit() instead of sprintf()
From: Russell Currey The secvar format string and object size sysfs files are both ASCII text, and should use sysfs_emit(). No functional change. Suggested-by: Greg Kroah-Hartman Signed-off-by: Russell Currey Signed-off-by: Andrew Donnellan Reviewed-by: Stefan Berger --- v2: New patch (gregkh) --- arch/powerpc/kernel/secvar-sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c index 702044edf14d..b786d1005027 100644 --- a/arch/powerpc/kernel/secvar-sysfs.c +++ b/arch/powerpc/kernel/secvar-sysfs.c @@ -35,7 +35,7 @@ static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr, if (rc) goto out; - rc = sprintf(buf, "%s\n", format); + rc = sysfs_emit(buf, "%s\n", format); out: of_node_put(node); @@ -57,7 +57,7 @@ static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr, return rc; } - return sprintf(buf, "%llu\n", dsize); + return sysfs_emit(buf, "%llu\n", dsize); } static ssize_t data_read(struct file *filep, struct kobject *kobj, -- 2.39.1
[PATCH v6 02/26] powerpc/pseries: Fix alignment of PLPKS structures and buffers
A number of structures and buffers passed to PKS hcalls have alignment
requirements, which could on occasion cause problems:
- Authorisation structures must be 16-byte aligned and must not cross a
page boundary
- Label structures must not cross page boundaries
- Password output buffers must not cross page boundaries
To ensure correct alignment, we adjust the allocation size of each of
these structures/buffers to be the closest power of 2 that is at least the
size of the structure/buffer (since kmalloc() guarantees that an
allocation of a power of 2 size will be aligned to at least that size).
Reported-by: Benjamin Gray
Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore")
Signed-off-by: Andrew Donnellan
Reviewed-by: Russell Currey
Signed-off-by: Russell Currey
---
v3: Merge plpks fixes and signed update series with secvar series
v4: Fix typo in commit message
Move up in series (npiggin)
v5: Reword commit message to better explain alignment guarantee (mpe)
---
arch/powerpc/platforms/pseries/plpks.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 9e85b6d85b0b..a01cf2ff140a 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -126,7 +126,8 @@ static int plpks_gen_password(void)
u8 *password, consumer = PKS_OS_OWNER;
int rc;
- password = kzalloc(maxpwsize, GFP_KERNEL);
+ // The password must not cross a page boundary, so we align to the next
power of 2
+ password = kzalloc(roundup_pow_of_two(maxpwsize), GFP_KERNEL);
if (!password)
return -ENOMEM;
@@ -162,7 +163,9 @@ static struct plpks_auth *construct_auth(u8 consumer)
if (consumer > PKS_OS_OWNER)
return ERR_PTR(-EINVAL);
- auth = kzalloc(struct_size(auth, password, maxpwsize), GFP_KERNEL);
+ // The auth structure must not cross a page boundary and must be
+ // 16 byte aligned. We align to the next largest power of 2
+ auth = kzalloc(roundup_pow_of_two(struct_size(auth, password,
maxpwsize)), GFP_KERNEL);
if (!auth)
return ERR_PTR(-ENOMEM);
@@ -196,7 +199,8 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
if (component && slen > sizeof(label->attr.prefix))
return ERR_PTR(-EINVAL);
- label = kzalloc(sizeof(*label), GFP_KERNEL);
+ // The label structure must not cross a page boundary, so we align to
the next power of 2
+ label = kzalloc(roundup_pow_of_two(sizeof(*label)), GFP_KERNEL);
if (!label)
return ERR_PTR(-ENOMEM);
--
2.39.1
[PATCH v6 00/26] pSeries dynamic secure boot secvar interface + platform keyring loading
pe) Make the data buffer in plpks_read_var() caller-allocated to reduce number of allocations/copies (mpe) Rework the Kconfig options so that PSERIES_PLPKS is a hidden option, turned on by enabling PPC_SECURE_BOOT, and the PLPKS secvar code is activated by PPC_SECVAR_SYSFS to match powernv (ajd) Use machine_arch_initcall() rather than device_initcall() so we don't break powernv (mpe) Improve ABI documentation (mpe) Return -EIO on most read errors (mpe) Add "grubdbx" variable (Sudhakar) Use utf8s_to_utf16s() rather than our own "UCS-2" conversion code (mpe) Fix SB_VERSION data length (ruscur) Stop prepending policy data on read (ruscur) Don't print errors to the kernel log when reading non-existent variables (Sudhakar) Miscellaneous code style, checkpatch cleanups Changes in v2: Remove unnecessary config vars from sysfs and document the others, thanks to review from Greg. If we end up needing to expose more, we can add them later and update the docs. Use sysfs_emit() instead of sprintf() for all sysfs strings Change the size of the sysfs binary attributes to include the 8-byte flags header, preventing truncation of large writes. Andrew Donnellan (9): powerpc/pseries: Fix handling of PLPKS object flushing timeout powerpc/pseries: Fix alignment of PLPKS structures and buffers powerpc/secvar: Clean up init error messages powerpc/secvar: Allow backend to populate static list of variable names powerpc/secvar: Warn when PAGE_SIZE is smaller than max object size powerpc/secvar: Don't print error on ENOENT when reading variables powerpc/pseries: Make caller pass buffer to plpks_read_var() powerpc/pseries: Turn PSERIES_PLPKS into a hidden option powerpc/pseries: Clarify warning when PLPKS password already set Michael Ellerman (1): powerpc/secvar: Use u64 in secvar_operations Nayna Jain (2): powerpc/pseries: Expose PLPKS config values, support additional fields powerpc/pseries: Implement signed update for PLPKS objects Russell Currey (14): powerpc/secvar: Fix incorrect return in secvar_sysfs_load() powerpc/secvar: Warn and error if multiple secvar ops are set powerpc/secvar: Use sysfs_emit() instead of sprintf() powerpc/secvar: Handle format string in the consumer powerpc/secvar: Handle max object size in the consumer powerpc/secvar: Extend sysfs to include config vars powerpc/pseries: Move plpks.h to include directory powerpc/pseries: Move PLPKS constants to header file powerpc/pseries: Log hcall return codes for PLPKS debug powerpc/pseries: Add helper to get PLPKS password length powerpc/pseries: Pass PLPKS password on kexec powerpc/pseries: Implement secvars for dynamic secure boot integrity/powerpc: Improve error handling & reporting when loading certs integrity/powerpc: Support loading keys from PLPKS Documentation/ABI/testing/sysfs-secvar| 75 +++- arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/hvcall.h | 1 + arch/powerpc/include/asm/plpks.h | 195 + arch/powerpc/include/asm/secvar.h | 21 +- arch/powerpc/kernel/prom.c| 4 + arch/powerpc/kernel/secvar-ops.c | 10 +- arch/powerpc/kernel/secvar-sysfs.c| 178 arch/powerpc/kexec/file_load_64.c | 18 +- arch/powerpc/platforms/powernv/opal-secvar.c | 60 ++- arch/powerpc/platforms/pseries/Kconfig| 19 +- arch/powerpc/platforms/pseries/Makefile | 4 +- arch/powerpc/platforms/pseries/plpks-secvar.c | 218 ++ arch/powerpc/platforms/pseries/plpks.c| 381 +++--- arch/powerpc/platforms/pseries/plpks.h| 71 .../integrity/platform_certs/load_powerpc.c | 47 ++- 16 files changed, 1046 insertions(+), 257 deletions(-) create mode 100644 arch/powerpc/include/asm/plpks.h create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c delete mode 100644 arch/powerpc/platforms/pseries/plpks.h -- 2.39.1
[PATCH v6 03/26] powerpc/secvar: Fix incorrect return in secvar_sysfs_load()
From: Russell Currey
secvar_ops->get_next() returns -ENOENT when there are no more variables
to return, which is expected behaviour.
Fix this by returning 0 if get_next() returns -ENOENT.
This fixes an issue introduced in commit bd5d9c743d38 ("powerpc: expose
secure variables to userspace via sysfs"), but the return code of
secvar_sysfs_load() was never checked so this issue never mattered.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
Reviewed-by: Stefan Berger
---
v5: New patch
---
arch/powerpc/kernel/secvar-sysfs.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 1ee4640a2641..7fa5f8ed9542 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -179,8 +179,10 @@ static int secvar_sysfs_load(void)
rc = secvar_ops->get_next(name, , NAME_MAX_SIZE);
if (rc) {
if (rc != -ENOENT)
- pr_err("error getting secvar from firmware
%d\n",
- rc);
+ pr_err("error getting secvar from firmware
%d\n", rc);
+ else
+ rc = 0;
+
break;
}
--
2.39.1
[PATCH v6 04/26] powerpc/secvar: Use u64 in secvar_operations
From: Michael Ellerman
There's no reason for secvar_operations to use uint64_t vs the more
common kernel type u64.
The types are compatible, but they require different printk format
strings which can lead to confusion.
Change all the secvar related routines to use u64.
Signed-off-by: Michael Ellerman
Reviewed-by: Russell Currey
Reviewed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
---
v3: Include new patch
---
arch/powerpc/include/asm/secvar.h| 9 +++--
arch/powerpc/kernel/secvar-sysfs.c | 8
arch/powerpc/platforms/powernv/opal-secvar.c | 9 +++--
security/integrity/platform_certs/load_powerpc.c | 4 ++--
4 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index 4cc35b58b986..07ba36f868a7 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -14,12 +14,9 @@
extern const struct secvar_operations *secvar_ops;
struct secvar_operations {
- int (*get)(const char *key, uint64_t key_len, u8 *data,
- uint64_t *data_size);
- int (*get_next)(const char *key, uint64_t *key_len,
- uint64_t keybufsize);
- int (*set)(const char *key, uint64_t key_len, u8 *data,
- uint64_t data_size);
+ int (*get)(const char *key, u64 key_len, u8 *data, u64 *data_size);
+ int (*get_next)(const char *key, u64 *key_len, u64 keybufsize);
+ int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 7fa5f8ed9542..702044edf14d 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -47,7 +47,7 @@ static ssize_t format_show(struct kobject *kobj, struct
kobj_attribute *attr,
static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
- uint64_t dsize;
+ u64 dsize;
int rc;
rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, );
@@ -64,8 +64,8 @@ static ssize_t data_read(struct file *filep, struct kobject
*kobj,
struct bin_attribute *attr, char *buf, loff_t off,
size_t count)
{
- uint64_t dsize;
char *data;
+ u64 dsize;
int rc;
rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, );
@@ -166,9 +166,9 @@ static int update_kobj_size(void)
static int secvar_sysfs_load(void)
{
- char *name;
- uint64_t namesize = 0;
struct kobject *kobj;
+ u64 namesize = 0;
+ char *name;
int rc;
name = kzalloc(NAME_MAX_SIZE, GFP_KERNEL);
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c
b/arch/powerpc/platforms/powernv/opal-secvar.c
index 14133e120bdd..ef89861569e0 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -54,8 +54,7 @@ static int opal_status_to_err(int rc)
return err;
}
-static int opal_get_variable(const char *key, uint64_t ksize,
-u8 *data, uint64_t *dsize)
+static int opal_get_variable(const char *key, u64 ksize, u8 *data, u64 *dsize)
{
int rc;
@@ -71,8 +70,7 @@ static int opal_get_variable(const char *key, uint64_t ksize,
return opal_status_to_err(rc);
}
-static int opal_get_next_variable(const char *key, uint64_t *keylen,
- uint64_t keybufsize)
+static int opal_get_next_variable(const char *key, u64 *keylen, u64 keybufsize)
{
int rc;
@@ -88,8 +86,7 @@ static int opal_get_next_variable(const char *key, uint64_t
*keylen,
return opal_status_to_err(rc);
}
-static int opal_set_variable(const char *key, uint64_t ksize, u8 *data,
-uint64_t dsize)
+static int opal_set_variable(const char *key, u64 ksize, u8 *data, u64 dsize)
{
int rc;
diff --git a/security/integrity/platform_certs/load_powerpc.c
b/security/integrity/platform_certs/load_powerpc.c
index a2900cb85357..1e4f80a4e71c 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -18,7 +18,7 @@
/*
* Get a certificate list blob from the named secure variable.
*/
-static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t
*size)
+static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size)
{
int rc;
void *db;
@@ -51,7 +51,7 @@ static __init void *get_cert_list(u8 *key, unsigned long
keylen, uint64_t *size)
static int __init load_powerpc_certs(void)
{
void *db = NULL, *dbx = NULL;
- uint64_t dbsize = 0, dbxsize = 0;
+ u64 dbsize = 0, dbxsize = 0;
int rc = 0;
struct device_node *node;
--
2.39.1
Re: [PATCH v5 19/25] powerpc/pseries: Make caller pass buffer to plpks_read_var()
On Tue, 2023-01-31 at 11:38 -0500, Stefan Berger wrote: > > > On 1/31/23 01:39, Andrew Donnellan wrote: > > Currently, plpks_read_var() allocates a buffer to pass to the > > H_PKS_READ_OBJECT hcall, then allocates another buffer, of the > > caller's > > > -> buffer of the (no comma) I'll just remove that clause entirely, it's not really necessary -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 4/4] powerpc/rtas: upgrade internal arch spinlocks
On Tue, 2023-01-24 at 08:04 -0600, Nathan Lynch wrote:
> At the time commit f97bb36f705d ("powerpc/rtas: Turn rtas lock into a
> raw spinlock") was written, the spinlock lockup detection code called
> __delay(), which will not make progress if the timebase is not
> advancing. Since the interprocessor timebase synchronization sequence
> for chrp, cell, and some now-unsupported Power models can temporarily
> freeze the timebase through an RTAS function (freeze-time-base), the
> lock that serializes most RTAS calls was converted to arch_spinlock_t
> to prevent kernel hangs in the lockup detection code.
>
> However, commit bc88c10d7e69 ("locking/spinlock/debug: Remove
> spinlock
> lockup detection code") removed that inconvenient property from the
> lock debug code several years ago. So now it should be safe to
> reintroduce generic locks into the RTAS support code, primarily to
> increase lockdep coverage.
>
> Making rtas_lock a spinlock_t would violate lock type nesting rules
> because it can be acquired while holding raw locks, e.g. pci_lock and
> irq_desc->lock. So convert it to raw_spinlock_t. There's no apparent
> reason not to upgrade timebase_lock as well.
>
> Signed-off-by: Nathan Lynch
I'm no locking expert but this looks reasonable from a quick read-
through.
Reviewed-by: Andrew Donnellan
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 3/4] powerpc/rtas: remove lock and args fields from global rtas struct
On Tue, 2023-01-24 at 08:04 -0600, Nathan Lynch wrote: > Only code internal to the RTAS subsystem needs access to the central > lock and parameter block. Remove these from the globally visible > 'rtas' struct and make them file-static in rtas.c. > > Some changed lines in rtas_call() lack appropriate spacing around > operators and cause checkpatch errors; fix these as well. > > Suggested-by: Laurent Dufour > Signed-off-by: Nathan Lynch Reviewed-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 2/4] powerpc/rtas: make all exports GPL
On Tue, 2023-01-24 at 08:04 -0600, Nathan Lynch wrote: > The first symbol exports of RTAS functions and data came with the > (now > removed) scanlog driver in 2003: > > https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=f92e361842d5251e50562b09664082dcbd0548bb > > At the time this was applied, EXPORT_SYMBOL_GPL() was very new, and > the exports of rtas_call() etc have remained non-GPL. As new APIs > have > been added to the RTAS subsystem, their symbol exports have followed > the convention set by existing code. > > However, the historical evidence is that RTAS function exports have > been added over time only to satisfy the needs of in-kernel users, > and > these clients must have fairly intimate knowledge of how the APIs > work > to use them safely. No out of tree users are known, and future ones > seem unlikely. > > Arguably the default for RTAS symbols should have become > EXPORT_SYMBOL_GPL once it was available. Let's make it so now, and > exceptions can be evaluated as needed. > > Signed-off-by: Nathan Lynch Agreed. Reviewed-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 1/4] powerpc/rtas: unexport 'rtas' symbol
On Tue, 2023-01-24 at 08:04 -0600, Nathan Lynch wrote: > No modular code needs access to the 'rtas' struct, so remove the > symbol export. > > Suggested-by: Michael Ellerman > Signed-off-by: Nathan Lynch A quick grep suggests that this is correct, and any modular code that wants access to the rtas struct would clearly be doing something extremely cursed that ought to be stopped. Reviewed-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v5 23/25] powerpc/pseries: Implement secvars for dynamic secure boot
On Tue, 2023-01-31 at 12:11 -0500, Stefan Berger wrote:
> > +static int plpks_get_variable(const char *key, u64 key_len, u8
> > *data,
> > + u64 *data_size)
> > +{
> > + struct plpks_var var = {0};
> > + int rc = 0;
> > +
> > + // We subtract 1 from key_len because we don't need to
> > include the
> > + // null terminator at the end of the string
> > + var.name = kcalloc(key_len - 1, sizeof(wchar_t),
> > GFP_KERNEL);
> > + if (!var.name)
> > + return -ENOMEM;
> > + rc = utf8s_to_utf16s(key, key_len - 1, UTF16_LITTLE_ENDIAN,
> > (wchar_t *)var.name,
> > + key_len - 1);
> > + if (rc < 0)
> > + goto err;
> > + var.namelen = rc * 2;
>
> Are you sure this is not just supposed to be 'rc'?
namelen is a length in bytes, not characters, while utf8s_to_utf16s()
returns the number of characters. I suppose this could be clearer by
changing 2 to sizeof(wchar_t).
>
> > +
> > + var.os = PLPKS_VAR_LINUX;
> > + if (data) {
> > + var.data = data;
> > + var.datalen = *data_size;
> > + }
> > + rc = plpks_read_os_var();
> > +
> > + if (rc)
> > + goto err;
> > +
> > + *data_size = var.datalen;
> > +
> > +err:
> > + kfree(var.name);
> > + if (rc && rc != -ENOENT) {
> > + pr_err("Failed to read variable '%s': %d\n", key,
> > rc);
> > + // Return -EIO since userspace probably doesn't
> > care about the
> > + // specific error
> > + rc = -EIO;
> > + }
> > + return rc;
> > +}
> > +
> > +static int plpks_set_variable(const char *key, u64 key_len, u8
> > *data,
> > + u64 data_size)
> > +{
> > + struct plpks_var var = {0};
> > + int rc = 0;
> > + u64 flags;
> > +
> > + // Secure variables need to be prefixed with 8 bytes of
> > flags.
> > + // We only want to perform the write if we have at least
> > one byte of data.
> > + if (data_size <= sizeof(flags))
> > + return -EINVAL;
> > +
> > + // We subtract 1 from key_len because we don't need to
> > include the
> > + // null terminator at the end of the string
> > + var.name = kcalloc(key_len - 1, sizeof(wchar_t),
> > GFP_KERNEL);
> here I would think that it should be key_len * 2 - 1 since
> utf8s_to_utf16s presumably makes the string longer
No, wchar_t is u16, so this allocates (key_len - 1)*sizeof(u16) =
(key_len - 1)*2 bytes.
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v4 22/24] powerpc/pseries: Implement secvars for dynamic secure boot
On Tue, 2023-01-31 at 18:55 +1000, Nicholas Piggin wrote: > > > > + var.datalen = 1; > > > > + var.data = kzalloc(1, GFP_KERNEL); > > > > > > This could just point to a u8 on stack I think? > > > > Until we get VMAP_STACK and we'll have to switch back. > > AFAIKS plpks_read_var does not require linear map, so should not be > required. IMO that's the right way to go for an external API, so > actually plpks_write_var is the outlier there and should be changed > to follow read and remove in not requiring special pointers. Indeed, I was confused - in the read case, the buffer doesn't get directly passed to the hcall. I'll wait a little bit longer for more feedback on v5 of this series and maybe fix this in v6 if mpe thinks I should respin it again. -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
[PATCH v5 05/25] powerpc/secvar: Warn and error if multiple secvar ops are set
From: Russell Currey
The secvar code only supports one consumer at a time.
Multiple consumers aren't possible at this point in time, but we'd want
it to be obvious if it ever could happen.
Signed-off-by: Russell Currey
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
---
v4: Return an error and don't actually try to set secvar_operations if the
warning is triggered (npiggin)
v5: Drop "extern" to fix a checkpatch check (snowpatch)
---
arch/powerpc/include/asm/secvar.h| 4 ++--
arch/powerpc/kernel/secvar-ops.c | 8 ++--
arch/powerpc/platforms/powernv/opal-secvar.c | 4 +---
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index 07ba36f868a7..a2b5f2203dc5 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -21,11 +21,11 @@ struct secvar_operations {
#ifdef CONFIG_PPC_SECURE_BOOT
-extern void set_secvar_ops(const struct secvar_operations *ops);
+int set_secvar_ops(const struct secvar_operations *ops);
#else
-static inline void set_secvar_ops(const struct secvar_operations *ops) { }
+static inline int set_secvar_ops(const struct secvar_operations *ops) { return
0; }
#endif
diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c
index 6a29777d6a2d..9c8dd4e7c270 100644
--- a/arch/powerpc/kernel/secvar-ops.c
+++ b/arch/powerpc/kernel/secvar-ops.c
@@ -8,10 +8,14 @@
#include
#include
+#include
-const struct secvar_operations *secvar_ops __ro_after_init;
+const struct secvar_operations *secvar_ops __ro_after_init = NULL;
-void set_secvar_ops(const struct secvar_operations *ops)
+int set_secvar_ops(const struct secvar_operations *ops)
{
+ if (WARN_ON_ONCE(secvar_ops))
+ return -1;
secvar_ops = ops;
+ return 0;
}
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c
b/arch/powerpc/platforms/powernv/opal-secvar.c
index ef89861569e0..4c0a3b030fe0 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -113,9 +113,7 @@ static int opal_secvar_probe(struct platform_device *pdev)
return -ENODEV;
}
- set_secvar_ops(_secvar_ops);
-
- return 0;
+ return set_secvar_ops(_secvar_ops);
}
static const struct of_device_id opal_secvar_match[] = {
--
2.39.1
[PATCH v5 12/25] powerpc/secvar: Warn when PAGE_SIZE is smaller than max object size
Due to sysfs constraints, when writing to a variable, we can only handle
writes of up to PAGE_SIZE.
It's possible that the maximum object size is larger than PAGE_SIZE, in
which case, print a warning on boot so that the user is aware.
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
---
v3: New patch (ajd)
---
arch/powerpc/kernel/secvar-sysfs.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 2cbc60b37e4e..9b6be63b7b36 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -223,6 +223,7 @@ static int secvar_sysfs_load_static(void)
static int secvar_sysfs_init(void)
{
+ u64 max_size;
int rc;
if (!secvar_ops) {
@@ -272,6 +273,14 @@ static int secvar_sysfs_init(void)
goto err;
}
+ // Due to sysfs limitations, we will only ever get a write buffer of
+ // up to 1 page in size. Print a warning if this is potentially going
+ // to cause problems, so that the user is aware.
+ secvar_ops->max_size(_size);
+ if (max_size > PAGE_SIZE)
+ pr_warn_ratelimited("PAGE_SIZE (%lu) is smaller than maximum
object size (%llu), writes are limited to PAGE_SIZE\n",
+ PAGE_SIZE, max_size);
+
return 0;
err:
kobject_put(secvar_kobj);
--
2.39.1
[PATCH v5 08/25] powerpc/secvar: Handle max object size in the consumer
From: Russell Currey
Currently the max object size is handled in the core secvar code with an
entirely OPAL-specific implementation, so create a new max_size() op and
move the existing implementation into the powernv platform. Should be
no functional change.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
v3: Change uint64_t type to u64 (mpe)
v4: Return immediately if node is NULL (gjoyce)
---
arch/powerpc/include/asm/secvar.h| 1 +
arch/powerpc/kernel/secvar-sysfs.c | 17 +++
arch/powerpc/platforms/powernv/opal-secvar.c | 22
3 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index 1a2c696a48ad..bf396215903d 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -18,6 +18,7 @@ struct secvar_operations {
int (*get_next)(const char *key, u64 *key_len, u64 keybufsize);
int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
ssize_t (*format)(char *buf, size_t bufsize);
+ int (*max_size)(u64 *max_size);
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index e4661559c855..0966806f28c7 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -132,27 +132,16 @@ static struct kobj_type secvar_ktype = {
static int update_kobj_size(void)
{
- struct device_node *node;
u64 varsize;
- int rc = 0;
+ int rc = secvar_ops->max_size();
- node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
- if (!of_device_is_available(node)) {
- rc = -ENODEV;
- goto out;
- }
-
- rc = of_property_read_u64(node, "max-var-size", );
if (rc)
- goto out;
+ return rc;
data_attr.size = varsize;
update_attr.size = varsize;
-out:
- of_node_put(node);
-
- return rc;
+ return 0;
}
static int secvar_sysfs_load(void)
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c
b/arch/powerpc/platforms/powernv/opal-secvar.c
index e33bb703ecbc..a8436bf35e2f 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -122,11 +122,33 @@ static ssize_t opal_secvar_format(char *buf, size_t
bufsize)
return rc;
}
+static int opal_secvar_max_size(u64 *max_size)
+{
+ int rc;
+ struct device_node *node;
+
+ node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
+ if (!node)
+ return -ENODEV;
+
+ if (!of_device_is_available(node)) {
+ rc = -ENODEV;
+ goto out;
+ }
+
+ rc = of_property_read_u64(node, "max-var-size", max_size);
+
+out:
+ of_node_put(node);
+ return rc;
+}
+
static const struct secvar_operations opal_secvar_ops = {
.get = opal_get_variable,
.get_next = opal_get_next_variable,
.set = opal_set_variable,
.format = opal_secvar_format,
+ .max_size = opal_secvar_max_size,
};
static int opal_secvar_probe(struct platform_device *pdev)
--
2.39.1
[PATCH v5 19/25] powerpc/pseries: Make caller pass buffer to plpks_read_var()
Currently, plpks_read_var() allocates a buffer to pass to the
H_PKS_READ_OBJECT hcall, then allocates another buffer, of the caller's
preferred size if necessary, into which the data is copied, and returns
that buffer to the caller.
This is a bit over the top - while we probably still want to allocate a
separate buffer to pass to the hypervisor in the hcall, we can let the
caller allocate the final buffer and specify the size.
Don't allocate var->data in plpks_read_var(), instead expect the caller to
allocate it. If the caller needs to discover the size, it can set
var->data to NULL and var->datalen will be populated. Update header file
to document this.
Suggested-by: Michael Ellerman
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
---
v3: New patch (mpe)
---
arch/powerpc/include/asm/plpks.h | 12
arch/powerpc/platforms/pseries/plpks.c | 11 ---
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index e7204e6c0ca4..0c49969b0864 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -88,16 +88,28 @@ int plpks_remove_var(char *component, u8 varos,
/**
* Returns the data for the specified os variable.
+ *
+ * Caller must allocate a buffer in var->data with length in var->datalen.
+ * If no buffer is provided, var->datalen will be populated with the object's
+ * size.
*/
int plpks_read_os_var(struct plpks_var *var);
/**
* Returns the data for the specified firmware variable.
+ *
+ * Caller must allocate a buffer in var->data with length in var->datalen.
+ * If no buffer is provided, var->datalen will be populated with the object's
+ * size.
*/
int plpks_read_fw_var(struct plpks_var *var);
/**
* Returns the data for the specified bootloader variable.
+ *
+ * Caller must allocate a buffer in var->data with length in var->datalen.
+ * If no buffer is provided, var->datalen will be populated with the object's
+ * size.
*/
int plpks_read_bootloader_var(struct plpks_var *var);
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index e5755443d4a4..926b6a927326 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -581,17 +581,14 @@ static int plpks_read_var(u8 consumer, struct plpks_var
*var)
goto out_free_output;
}
- if (var->datalen == 0 || var->datalen > retbuf[0])
+ if (!var->data || var->datalen > retbuf[0])
var->datalen = retbuf[0];
- var->data = kzalloc(var->datalen, GFP_KERNEL);
- if (!var->data) {
- rc = -ENOMEM;
- goto out_free_output;
- }
var->policy = retbuf[1];
- memcpy(var->data, output, var->datalen);
+ if (var->data)
+ memcpy(var->data, output, var->datalen);
+
rc = 0;
out_free_output:
--
2.39.1
[PATCH v5 07/25] powerpc/secvar: Handle format string in the consumer
From: Russell Currey
The code that handles the format string in secvar-sysfs.c is entirely
OPAL specific, so create a new "format" op in secvar_operations to make
the secvar code more generic. No functional change.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
v2: Use sysfs_emit() instead of sprintf() (gregkh)
v3: Enforce format string size limit (ruscur)
v4: Pass the buffer size as an argument, not using a macro (stefanb,
npiggin)
Fix error reporting (npiggin)
---
arch/powerpc/include/asm/secvar.h| 1 +
arch/powerpc/kernel/secvar-sysfs.c | 27 +++-
arch/powerpc/platforms/powernv/opal-secvar.c | 25 ++
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index a2b5f2203dc5..1a2c696a48ad 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -17,6 +17,7 @@ struct secvar_operations {
int (*get)(const char *key, u64 key_len, u8 *data, u64 *data_size);
int (*get_next)(const char *key, u64 *key_len, u64 keybufsize);
int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
+ ssize_t (*format)(char *buf, size_t bufsize);
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index b786d1005027..e4661559c855 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -21,26 +21,17 @@ static struct kset *secvar_kset;
static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
- ssize_t rc = 0;
- struct device_node *node;
- const char *format;
-
- node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
- if (!of_device_is_available(node)) {
- rc = -ENODEV;
- goto out;
- }
+ char tmp[32];
+ ssize_t len = secvar_ops->format(tmp, sizeof(tmp));
- rc = of_property_read_string(node, "format", );
- if (rc)
- goto out;
+ if (len > 0)
+ return sysfs_emit(buf, "%s\n", tmp);
+ else if (len < 0)
+ pr_err("Error %zd reading format string\n", len);
+ else
+ pr_err("Got empty format string from backend\n");
- rc = sysfs_emit(buf, "%s\n", format);
-
-out:
- of_node_put(node);
-
- return rc;
+ return -EIO;
}
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c
b/arch/powerpc/platforms/powernv/opal-secvar.c
index 4c0a3b030fe0..e33bb703ecbc 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -98,10 +98,35 @@ static int opal_set_variable(const char *key, u64 ksize, u8
*data, u64 dsize)
return opal_status_to_err(rc);
}
+static ssize_t opal_secvar_format(char *buf, size_t bufsize)
+{
+ ssize_t rc = 0;
+ struct device_node *node;
+ const char *format;
+
+ node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
+ if (!of_device_is_available(node)) {
+ rc = -ENODEV;
+ goto out;
+ }
+
+ rc = of_property_read_string(node, "format", );
+ if (rc)
+ goto out;
+
+ rc = snprintf(buf, bufsize, "%s", format);
+
+out:
+ of_node_put(node);
+
+ return rc;
+}
+
static const struct secvar_operations opal_secvar_ops = {
.get = opal_get_variable,
.get_next = opal_get_next_variable,
.set = opal_set_variable,
+ .format = opal_secvar_format,
};
static int opal_secvar_probe(struct platform_device *pdev)
--
2.39.1
[PATCH v5 11/25] powerpc/secvar: Allow backend to populate static list of variable names
Currently, the list of variables is populated by calling
secvar_ops->get_next() repeatedly, which is explicitly modelled on the
OPAL API (including the keylen parameter).
For the upcoming PLPKS backend, we have a static list of variable names.
It is messy to fit that into get_next(), so instead, let the backend put
a NULL-terminated array of variable names into secvar_ops->var_names,
which will be used if get_next() is undefined.
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
---
v3: New patch (ajd/mpe)
---
arch/powerpc/include/asm/secvar.h | 4 ++
arch/powerpc/kernel/secvar-sysfs.c | 67 --
2 files changed, 50 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index 011a53a8076c..4828e0ab7e3c 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -21,6 +21,10 @@ struct secvar_operations {
ssize_t (*format)(char *buf, size_t bufsize);
int (*max_size)(u64 *max_size);
const struct attribute **config_attrs;
+
+ // NULL-terminated array of fixed variable names
+ // Only used if get_next() isn't provided
+ const char * const *var_names;
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 7df32be86507..2cbc60b37e4e 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -157,9 +157,31 @@ static int secvar_sysfs_config(struct kobject *kobj)
return 0;
}
-static int secvar_sysfs_load(void)
+static int add_var(const char *name)
{
struct kobject *kobj;
+ int rc;
+
+ kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
+ if (!kobj)
+ return -ENOMEM;
+
+ kobject_init(kobj, _ktype);
+
+ rc = kobject_add(kobj, _kset->kobj, "%s", name);
+ if (rc) {
+ pr_warn("kobject_add error %d for attribute: %s\n", rc,
+ name);
+ kobject_put(kobj);
+ return rc;
+ }
+
+ kobject_uevent(kobj, KOBJ_ADD);
+ return 0;
+}
+
+static int secvar_sysfs_load(void)
+{
u64 namesize = 0;
char *name;
int rc;
@@ -179,31 +201,26 @@ static int secvar_sysfs_load(void)
break;
}
- kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
- if (!kobj) {
- rc = -ENOMEM;
- break;
- }
-
- kobject_init(kobj, _ktype);
-
- rc = kobject_add(kobj, _kset->kobj, "%s", name);
- if (rc) {
- pr_warn("kobject_add error %d for attribute: %s\n", rc,
- name);
- kobject_put(kobj);
- kobj = NULL;
- }
-
- if (kobj)
- kobject_uevent(kobj, KOBJ_ADD);
-
+ rc = add_var(name);
} while (!rc);
kfree(name);
return rc;
}
+static int secvar_sysfs_load_static(void)
+{
+ const char * const *name_ptr = secvar_ops->var_names;
+ int rc;
+ while (*name_ptr) {
+ rc = add_var(*name_ptr);
+ if (rc)
+ return rc;
+ name_ptr++;
+ }
+ return 0;
+}
+
static int secvar_sysfs_init(void)
{
int rc;
@@ -245,7 +262,15 @@ static int secvar_sysfs_init(void)
goto err;
}
- secvar_sysfs_load();
+ if (secvar_ops->get_next)
+ rc = secvar_sysfs_load();
+ else
+ rc = secvar_sysfs_load_static();
+
+ if (rc) {
+ pr_err("Failed to create variable attributes\n");
+ goto err;
+ }
return 0;
err:
--
2.39.1
[PATCH v5 17/25] powerpc/pseries: Implement signed update for PLPKS objects
From: Nayna Jain
The Platform Keystore provides a signed update interface which can be used
to create, replace or append to certain variables in the PKS in a secure
fashion, with the hypervisor requiring that the update be signed using the
Platform Key.
Implement an interface to the H_PKS_SIGNED_UPDATE hcall in the plpks
driver to allow signed updates to PKS objects.
(The plpks driver doesn't need to do any cryptography or otherwise handle
the actual signed variable contents - that will be handled by userspace
tooling.)
Signed-off-by: Nayna Jain
[ajd: split patch, add timeout handling and misc cleanups]
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
---
v3: Merge plpks fixes and signed update series with secvar series
Fix error code handling in plpks_confirm_object_flushed() (ruscur)
Pass plpks_var struct to plpks_signed_update_var() by reference (mpe)
Consistent constant naming scheme (ruscur)
v4: Fix MAX_HCALL_OPCODE rebasing issue (npiggin)
v5: Drop the EXPORT_SYMBOL since we don't need it (npiggin)
Return an error if plpks_signed_update_var() is called with non-NULL
component (npiggin)
---
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/include/asm/plpks.h | 5 ++
arch/powerpc/platforms/pseries/plpks.c | 74 --
3 files changed, 75 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h
b/arch/powerpc/include/asm/hvcall.h
index 95fd7f9485d5..c099780385dd 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -335,6 +335,7 @@
#define H_RPT_INVALIDATE 0x448
#define H_SCM_FLUSH0x44C
#define H_GET_ENERGY_SCALE_INFO0x450
+#define H_PKS_SIGNED_UPDATE0x454
#define H_WATCHDOG 0x45C
#define MAX_HCALL_OPCODE H_WATCHDOG
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 7c5f51a9af7c..e7204e6c0ca4 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -68,6 +68,11 @@ struct plpks_var_name_list {
struct plpks_var_name varlist[];
};
+/**
+ * Updates the authenticated variable. It expects NULL as the component.
+ */
+int plpks_signed_update_var(struct plpks_var *var, u64 flags);
+
/**
* Writes the specified var and its data to PKS.
* Any caller of PKS driver should present a valid component type for
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 1189246b03dc..cee06fb9a370 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -81,6 +81,12 @@ static int pseries_status_to_err(int rc)
err = -ENOENT;
break;
case H_BUSY:
+ case H_LONG_BUSY_ORDER_1_MSEC:
+ case H_LONG_BUSY_ORDER_10_MSEC:
+ case H_LONG_BUSY_ORDER_100_MSEC:
+ case H_LONG_BUSY_ORDER_1_SEC:
+ case H_LONG_BUSY_ORDER_10_SEC:
+ case H_LONG_BUSY_ORDER_100_SEC:
err = -EBUSY;
break;
case H_AUTHORITY:
@@ -184,14 +190,17 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
u16 namelen)
{
struct label *label;
- size_t slen;
+ size_t slen = 0;
if (!name || namelen > PLPKS_MAX_NAME_SIZE)
return ERR_PTR(-EINVAL);
- slen = strlen(component);
- if (component && slen > sizeof(label->attr.prefix))
- return ERR_PTR(-EINVAL);
+ // Support NULL component for signed updates
+ if (component) {
+ slen = strlen(component);
+ if (slen > sizeof(label->attr.prefix))
+ return ERR_PTR(-EINVAL);
+ }
// The label structure must not cross a page boundary, so we align to
the next power of 2
label = kzalloc(roundup_pow_of_two(sizeof(*label)), GFP_KERNEL);
@@ -397,6 +406,61 @@ static int plpks_confirm_object_flushed(struct label
*label,
return pseries_status_to_err(rc);
}
+int plpks_signed_update_var(struct plpks_var *var, u64 flags)
+{
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
+ int rc;
+ struct label *label;
+ struct plpks_auth *auth;
+ u64 continuetoken = 0;
+ u64 timeout = 0;
+
+ if (!var->data || var->datalen <= 0 || var->namelen >
PLPKS_MAX_NAME_SIZE)
+ return -EINVAL;
+
+ if (!(var->policy & PLPKS_SIGNEDUPDATE))
+ return -EINVAL;
+
+ // Signed updates need the component to be NULL.
+ if (var->component)
+ return -EINVAL;
+
+ auth = construct_auth(PLPKS_OS_OWNER);
+ if (IS_ERR(auth))
+ return PTR_ERR(auth);
+
+ label = construct_label(var->component, var->os, var->name,
var->namelen);
+ if (IS_ERR(label)) {
+
[PATCH v5 15/25] powerpc/pseries: Move PLPKS constants to header file
From: Russell Currey
Move the constants defined in plpks.c to plpks.h, and standardise their
naming, so that PLPKS consumers can make use of them later on.
Signed-off-by: Russell Currey
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
---
v3: New patch
---
arch/powerpc/include/asm/plpks.h | 36 +---
arch/powerpc/platforms/pseries/plpks.c | 57 ++
2 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 8295502ee93b..6466aadd7145 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -14,14 +14,40 @@
#include
#include
-#define OSSECBOOTAUDIT 0x4000
-#define OSSECBOOTENFORCE 0x2000
-#define WORLDREADABLE 0x0800
-#define SIGNEDUPDATE 0x0100
+// Object policy flags from supported_policies
+#define PLPKS_OSSECBOOTAUDIT PPC_BIT32(1) // OS secure boot must be
audit/enforce
+#define PLPKS_OSSECBOOTENFORCE PPC_BIT32(2) // OS secure boot must be enforce
+#define PLPKS_PWSETPPC_BIT32(3) // No access without password set
+#define PLPKS_WORLDREADABLEPPC_BIT32(4) // Readable without authentication
+#define PLPKS_IMMUTABLEPPC_BIT32(5) // Once written, object
cannot be removed
+#define PLPKS_TRANSIENTPPC_BIT32(6) // Object does not persist
through reboot
+#define PLPKS_SIGNEDUPDATE PPC_BIT32(7) // Object can only be modified by
signed updates
+#define PLPKS_HVPROVISIONEDPPC_BIT32(28) // Hypervisor has provisioned
this object
-#define PLPKS_VAR_LINUX0x02
+// Signature algorithm flags from signed_update_algorithms
+#define PLPKS_ALG_RSA2048 PPC_BIT(0)
+#define PLPKS_ALG_RSA4096 PPC_BIT(1)
+
+// Object label OS metadata flags
+#define PLPKS_VAR_LINUX0x02
#define PLPKS_VAR_COMMON 0x04
+// Flags for which consumer owns an object is owned by
+#define PLPKS_FW_OWNER 0x1
+#define PLPKS_BOOTLOADER_OWNER 0x2
+#define PLPKS_OS_OWNER 0x3
+
+// Flags for label metadata fields
+#define PLPKS_LABEL_VERSION0
+#define PLPKS_MAX_LABEL_ATTR_SIZE 16
+#define PLPKS_MAX_NAME_SIZE239
+#define PLPKS_MAX_DATA_SIZE4000
+
+// Timeouts for PLPKS operations
+#define PLPKS_MAX_TIMEOUT 5000 // msec
+#define PLPKS_FLUSH_SLEEP 10 // msec
+#define PLPKS_FLUSH_SLEEP_RANGE400
+
struct plpks_var {
char *component;
u8 *name;
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 13e6daadb179..91f3f623a2c7 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -20,19 +20,6 @@
#include
#include
-#define PKS_FW_OWNER0x1
-#define PKS_BOOTLOADER_OWNER 0x2
-#define PKS_OS_OWNER0x3
-
-#define LABEL_VERSION 0
-#define MAX_LABEL_ATTR_SIZE 16
-#define MAX_NAME_SIZE 239
-#define MAX_DATA_SIZE 4000
-
-#define PKS_FLUSH_MAX_TIMEOUT 5000 //msec
-#define PKS_FLUSH_SLEEP 10 //msec
-#define PKS_FLUSH_SLEEP_RANGE 400
-
static u8 *ospassword;
static u16 ospasswordlength;
@@ -59,7 +46,7 @@ struct label_attr {
struct label {
struct label_attr attr;
- u8 name[MAX_NAME_SIZE];
+ u8 name[PLPKS_MAX_NAME_SIZE];
size_t size;
};
@@ -122,7 +109,7 @@ static int pseries_status_to_err(int rc)
static int plpks_gen_password(void)
{
unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
- u8 *password, consumer = PKS_OS_OWNER;
+ u8 *password, consumer = PLPKS_OS_OWNER;
int rc;
// The password must not cross a page boundary, so we align to the next
power of 2
@@ -159,7 +146,7 @@ static struct plpks_auth *construct_auth(u8 consumer)
{
struct plpks_auth *auth;
- if (consumer > PKS_OS_OWNER)
+ if (consumer > PLPKS_OS_OWNER)
return ERR_PTR(-EINVAL);
// The auth structure must not cross a page boundary and must be
@@ -171,7 +158,7 @@ static struct plpks_auth *construct_auth(u8 consumer)
auth->version = 1;
auth->consumer = consumer;
- if (consumer == PKS_FW_OWNER || consumer == PKS_BOOTLOADER_OWNER)
+ if (consumer == PLPKS_FW_OWNER || consumer == PLPKS_BOOTLOADER_OWNER)
return auth;
memcpy(auth->password, ospassword, ospasswordlength);
@@ -191,7 +178,7 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
struct label *label;
size_t slen;
- if (!name || namelen > MAX_NAME_SIZE)
+ if (!name || namelen > PLPKS_MAX_NAME_SIZE)
return ERR_PTR(-EINVAL);
slen = strlen(component);
@@ -206,9 +193,9 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
if (component)
memcpy(>attr.prefix, component
[PATCH v5 23/25] powerpc/pseries: Implement secvars for dynamic secure boot
From: Russell Currey The pseries platform can support dynamic secure boot (i.e. secure boot using user-defined keys) using variables contained with the PowerVM LPAR Platform KeyStore (PLPKS). Using the powerpc secvar API, expose the relevant variables for pseries dynamic secure boot through the existing secvar filesystem layout. The relevant variables for dynamic secure boot are signed in the keystore, and can only be modified using the H_PKS_SIGNED_UPDATE hcall. Object labels in the keystore are encoded using ucs2 format. With our fixed variable names we don't have to care about encoding outside of the necessary byte padding. When a user writes to a variable, the first 8 bytes of data must contain the signed update flags as defined by the hypervisor. When a user reads a variable, the first 4 bytes of data contain the policies defined for the object. Limitations exist due to the underlying implementation of sysfs binary attributes, as is the case for the OPAL secvar implementation - partial writes are unsupported and writes cannot be larger than PAGE_SIZE. (Even when using bin_attributes, which can be larger than a single page, sysfs only gives us one page's worth of write buffer at a time, and the hypervisor does not expose an interface for partial writes.) Co-developed-by: Nayna Jain Signed-off-by: Nayna Jain Co-developed-by: Andrew Donnellan Signed-off-by: Andrew Donnellan Signed-off-by: Russell Currey --- v2: Remove unnecessary config vars from sysfs and document the others, thanks to review from Greg. If we end up needing to expose more, we can add them later and update the docs. Use sysfs_emit() instead of sprintf(), thanks to Greg. Change the size of the sysfs binary attributes to include the 8-byte flags header, preventing truncation of large writes. v3: plpks_set_variable(): pass var to plpks_signed_update_var() as a pointer (mpe) Update copyright date (ajd) Consistent comment style (ajd) Change device_initcall() to machine_arch_initcall(pseries...) so we don't try to load on powernv and kill the machine (mpe) Add config attributes into plpks_secvar_ops (mpe) Get rid of PLPKS_SECVAR_COUNT macro (mpe) Reworded descriptions in ABI documentation (mpe) Switch to using secvar_ops->var_names rather than secvar_ops->get_next() (ajd/mpe) Optimise allocation/copying of buffers (mpe) Elaborate the comment documenting the "format" string (mpe) Return -EIO on errors in the read case (mpe) Add "grubdbx" variable (Sudhakar Kuppusamy) Use utf8s_to_utf16s() rather than our own "UCS-2" conversion code (mpe) Change uint64_t to u64 (mpe) Fix SB_VERSION data length (ruscur) Stop prepending policy data on read (ruscur) Enforce max format length on format string (not strictly needed, but makes the length limit clear) (ajd) Update include of plpks.h to reflect new path (ruscur) Consistent constant naming scheme (ruscur) v4: Return set_secvar_ops() return code Pass buffer size to plpks_secvar_format() (stefanb, npiggin) Add missing null check (stefanb) Add comment to commit message explaining PAGE_SIZE write limit (joel) v5: Add comment explaining why we use "key_len - 1" (npiggin) Use strlen(var.name) instead of hardcoding 10 as length of "SB_VERSION" (npiggin) Improve comments about use of SB_VERSION and format string (npiggin) Change "+ 8" to "+ sizeof(u64)" when accounting for flags size in working out file's max size (npiggin) Compile plpks-secvar.c based on CONFIG_PPC_SECURE_BOOT, not CONFIG_PPC_SECVAR_SYSFS, as the secvar backend is needed for loading keys into keyrings even if the sysfs interface is disabled (ajd) --- Documentation/ABI/testing/sysfs-secvar| 75 +- arch/powerpc/platforms/pseries/Makefile | 4 +- arch/powerpc/platforms/pseries/plpks-secvar.c | 219 ++ 3 files changed, 295 insertions(+), 3 deletions(-) create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar index feebb8c57294..a19f4d5fcec6 100644 --- a/Documentation/ABI/testing/sysfs-secvar +++ b/Documentation/ABI/testing/sysfs-secvar @@ -18,6 +18,14 @@ Description: A string indicating which backend is in use by the firmware. This determines the format of the variable and the accepted format of variable updates. + On powernv/OPAL, this value is provided by the OPAL firmware + and is expected to be "ibm,edk2-compat-v1". + + On pseries/PLPKS, this is generated by the kernel based on the + version number in the SB_VERSION variable in the keystore, and + has the form "ibm,plpks-sb-v", or + "ibm,plp
[PATCH v5 25/25] integrity/powerpc: Support loading keys from PLPKS
From: Russell Currey
Add support for loading keys from the PLPKS on pseries machines, with the
"ibm,plpks-sb-v1" format.
The object format is expected to be the same, so there shouldn't be any
functional differences between objects retrieved on powernv or pseries.
Unlike on powernv, on pseries the format string isn't contained in the
device tree. Use secvar_ops->format() to fetch the format string in a
generic manner, rather than searching the device tree ourselves.
(The current code searches the device tree for a node compatible with
"ibm,edk2-compat-v1". This patch switches to calling secvar_ops->format(),
which in the case of OPAL/powernv means opal_secvar_format(), which
searches the device tree for a node compatible with "ibm,secvar-backend"
and checks its "format" property. These are equivalent, as skiboot creates
a node with both "ibm,edk2-compat-v1" and "ibm,secvar-backend" as
compatible strings.)
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
v3: New patch
v4: Pass format buffer size (stefanb, npiggin)
v5: Use sizeof(buf) rather than stating the size twice (npiggin)
Clarify change to DT compatible strings in commit message (zohar)
Reword commit message a bit (ajd)
---
.../integrity/platform_certs/load_powerpc.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/security/integrity/platform_certs/load_powerpc.c
b/security/integrity/platform_certs/load_powerpc.c
index dee51606d5f4..b9de70b90826 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -10,7 +10,6 @@
#include
#include
#include
-#include
#include
#include
#include "keyring_handler.h"
@@ -59,16 +58,22 @@ static int __init load_powerpc_certs(void)
void *db = NULL, *dbx = NULL;
u64 dbsize = 0, dbxsize = 0;
int rc = 0;
- struct device_node *node;
+ ssize_t len;
+ char buf[32];
if (!secvar_ops)
return -ENODEV;
- /* The following only applies for the edk2-compat backend. */
- node = of_find_compatible_node(NULL, NULL, "ibm,edk2-compat-v1");
- if (!node)
+ len = secvar_ops->format(buf, sizeof(buf));
+ if (len <= 0)
return -ENODEV;
+ // Check for known secure boot implementations from OPAL or PLPKS
+ if (strcmp("ibm,edk2-compat-v1", buf) && strcmp("ibm,plpks-sb-v1",
buf)) {
+ pr_err("Unsupported secvar implementation \"%s\", not loading
certs\n", buf);
+ return -ENODEV;
+ }
+
/*
* Get db, and dbx. They might not exist, so it isn't an error if we
* can't get them.
@@ -103,8 +108,6 @@ static int __init load_powerpc_certs(void)
kfree(dbx);
}
- of_node_put(node);
-
return rc;
}
late_initcall(load_powerpc_certs);
--
2.39.1
[PATCH v5 18/25] powerpc/pseries: Log hcall return codes for PLPKS debug
From: Russell Currey
The plpks code converts hypervisor return codes into their Linux
equivalents so that users can understand them. Having access to the
original return codes is really useful for debugging, so add a
pr_debug() so we don't lose information from the conversion.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
arch/powerpc/platforms/pseries/plpks.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index cee06fb9a370..e5755443d4a4 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -117,6 +117,8 @@ static int pseries_status_to_err(int rc)
err = -EINVAL;
}
+ pr_debug("Converted hypervisor code %d to Linux %d\n", rc, err);
+
return err;
}
--
2.39.1
[PATCH v5 22/25] powerpc/pseries: Pass PLPKS password on kexec
From: Russell Currey
Before interacting with the PLPKS, we ask the hypervisor to generate a
password for the current boot, which is then required for most further
PLPKS operations.
If we kexec into a new kernel, the new kernel will try and fail to
generate a new password, as the password has already been set.
Pass the password through to the new kernel via the device tree, in
/chosen/ibm,plpks-pw. Check for the presence of this property before
trying to generate a new password - if it exists, use the existing
password and remove it from the device tree.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
v3: New patch
v4: Fix compile when CONFIG_PSERIES_PLPKS=n (snowpatch)
Fix error handling on fdt_path_offset() call (ruscur)
v5: Fix DT property name in commit message (npiggin)
Clear prop in FDT during init to prevent password exposure (mpe)
Rework to remove ifdefs from C code (npiggin)
---
arch/powerpc/include/asm/plpks.h | 14 ++
arch/powerpc/kernel/prom.c | 4 ++
arch/powerpc/kexec/file_load_64.c | 15 +--
arch/powerpc/platforms/pseries/plpks.c | 60 ++
4 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 757313e00521..23b77027c916 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -176,6 +176,20 @@ u64 plpks_get_signedupdatealgorithms(void);
*/
u16 plpks_get_passwordlen(void);
+/**
+ * Called in early init to retrieve and clear the PLPKS password from the DT.
+ */
+void plpks_early_init_devtree(void);
+
+/**
+ * Populates the FDT with the PLPKS password to prepare for kexec.
+ */
+int plpks_populate_fdt(void *fdt);
+#else // CONFIG_PSERIES_PLPKS
+static inline bool plpks_is_available(void) { return false; }
+static inline u16 plpks_get_passwordlen(void) { BUILD_BUG(); }
+static inline void plpks_early_init_devtree(void) { }
+static inline int plpks_populate_fdt(void *fdt) { BUILD_BUG(); }
#endif // CONFIG_PSERIES_PLPKS
#endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4f1c920aa13e..8a13b378770f 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
@@ -893,6 +894,9 @@ void __init early_init_devtree(void *params)
powerpc_firmware_features |= FW_FEATURE_PS3_POSSIBLE;
#endif
+ /* If kexec left a PLPKS password in the DT, get it and clear it */
+ plpks_early_init_devtree();
+
tm_init();
DBG(" <- early_init_devtree()\n");
diff --git a/arch/powerpc/kexec/file_load_64.c
b/arch/powerpc/kexec/file_load_64.c
index af8854f9eae3..3f5740fb01a4 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
struct umem_info {
u64 *buf; /* data buffer for usable-memory property */
@@ -977,12 +978,16 @@ static unsigned int cpu_node_size(void)
*/
unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
{
- unsigned int cpu_nodes, extra_size;
+ unsigned int cpu_nodes, extra_size = 0;
struct device_node *dn;
u64 usm_entries;
+ // Make room for the PLPKS password, plus node overhead for
ibm,plpks-pw.
+ if (plpks_is_available())
+ extra_size += (unsigned int)plpks_get_passwordlen() + 32;
+
if (image->type != KEXEC_TYPE_CRASH)
- return 0;
+ return extra_size;
/*
* For kdump kernel, account for linux,usable-memory and
@@ -992,7 +997,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage
*image)
usm_entries = ((memblock_end_of_DRAM() / drmem_lmb_size()) +
(2 * (resource_size(_res) / drmem_lmb_size(;
- extra_size = (unsigned int)(usm_entries * sizeof(u64));
+ extra_size += (unsigned int)(usm_entries * sizeof(u64));
/*
* Get the number of CPU nodes in the current DT. This allows to
@@ -1230,6 +1235,10 @@ int setup_new_fdt_ppc64(const struct kimage *image, void
*fdt,
}
}
+ // If we have PLPKS active, we need to provide the password to the new
kernel
+ if (plpks_is_available())
+ ret = plpks_populate_fdt(fdt);
+
out:
kfree(rmem);
kfree(umem);
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 6940280ae94a..481a669845c5 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -16,6 +16,9 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
#include
@@ -128,6 +131,12 @@ static int plpks_gen_password(void)
u8 *password, consumer = PLPKS_OS_OWNER;
int rc;
+ // If we boote
[PATCH v5 20/25] powerpc/pseries: Turn PSERIES_PLPKS into a hidden option
It seems a bit unnecessary for the PLPKS code to have a user-visible config option when it doesn't do anything on its own, and there's existing options for enabling Secure Boot-related features. It should be enabled by PPC_SECURE_BOOT, which will eventually be what uses PLPKS to populate keyrings. However, we can't get of the separate option completely, because it will also be used for SED Opal purposes. Change PSERIES_PLPKS into a hidden option, which is selected by PPC_SECURE_BOOT. Signed-off-by: Andrew Donnellan Signed-off-by: Russell Currey --- v3: New patch v5: Change the previous description into a comment (npiggin) --- arch/powerpc/Kconfig | 1 + arch/powerpc/platforms/pseries/Kconfig | 19 +-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index b8c4ac56bddc..d4ed46101bec 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -1029,6 +1029,7 @@ config PPC_SECURE_BOOT depends on PPC_POWERNV || PPC_PSERIES depends on IMA_ARCH_POLICY imply IMA_SECURE_AND_OR_TRUSTED_BOOT + select PSERIES_PLPKS if PPC_PSERIES help Systems with firmware secure boot enabled need to define security policies to extend secure boot to the OS. This config allows a user diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index a3b4d99567cb..e51d65969318 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -151,16 +151,15 @@ config IBMEBUS config PSERIES_PLPKS depends on PPC_PSERIES - bool "Support for the Platform Key Storage" - help - PowerVM provides an isolated Platform Keystore(PKS) storage - allocation for each LPAR with individually managed access - controls to store sensitive information securely. It can be - used to store asymmetric public keys or secrets as required - by different usecases. Select this config to enable - operating system interface to hypervisor to access this space. - - If unsure, select N. + bool + # PowerVM provides an isolated Platform Keystore (PKS) storage + # allocation for each LPAR with individually managed access + # controls to store sensitive information securely. It can be + # used to store asymmetric public keys or secrets as required + # by different usecases. + # + # This option is selected by in-kernel consumers that require + # access to the PKS. config PAPR_SCM depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM -- 2.39.1
[PATCH v5 21/25] powerpc/pseries: Add helper to get PLPKS password length
From: Russell Currey
Add helper function to get the PLPKS password length. This will be used
in a later patch to support passing the password between kernels over
kexec.
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
v3: New patch
v5: Drop plpks_get_password() since we no longer need to expose it.
---
arch/powerpc/include/asm/plpks.h | 5 +
arch/powerpc/platforms/pseries/plpks.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 0c49969b0864..757313e00521 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -171,6 +171,11 @@ u32 plpks_get_maxlargeobjectsize(void);
*/
u64 plpks_get_signedupdatealgorithms(void);
+/**
+ * Returns the length of the PLPKS password in bytes.
+ */
+u16 plpks_get_passwordlen(void);
+
#endif // CONFIG_PSERIES_PLPKS
#endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 926b6a927326..6940280ae94a 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -359,6 +359,11 @@ u64 plpks_get_signedupdatealgorithms(void)
return signedupdatealgorithms;
}
+u16 plpks_get_passwordlen(void)
+{
+ return ospasswordlength;
+}
+
bool plpks_is_available(void)
{
int rc;
--
2.39.1
[PATCH v5 24/25] integrity/powerpc: Improve error handling & reporting when loading certs
From: Russell Currey
A few improvements to load_powerpc.c:
- include integrity.h for the pr_fmt()
- move all error reporting out of get_cert_list()
- use ERR_PTR() to better preserve error detail
- don't use pr_err() for missing keys
Reviewed-by: Mimi Zohar
Signed-off-by: Russell Currey
Signed-off-by: Andrew Donnellan
---
v3: New patch
---
.../integrity/platform_certs/load_powerpc.c | 26 ++-
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/security/integrity/platform_certs/load_powerpc.c
b/security/integrity/platform_certs/load_powerpc.c
index 1e4f80a4e71c..dee51606d5f4 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -14,9 +14,15 @@
#include
#include
#include "keyring_handler.h"
+#include "../integrity.h"
/*
* Get a certificate list blob from the named secure variable.
+ *
+ * Returns:
+ * - a pointer to a kmalloc'd buffer containing the cert list on success
+ * - NULL if the key does not exist
+ * - an ERR_PTR on error
*/
static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size)
{
@@ -25,19 +31,19 @@ static __init void *get_cert_list(u8 *key, unsigned long
keylen, u64 *size)
rc = secvar_ops->get(key, keylen, NULL, size);
if (rc) {
- pr_err("Couldn't get size: %d\n", rc);
- return NULL;
+ if (rc == -ENOENT)
+ return NULL;
+ return ERR_PTR(rc);
}
db = kmalloc(*size, GFP_KERNEL);
if (!db)
- return NULL;
+ return ERR_PTR(-ENOMEM);
rc = secvar_ops->get(key, keylen, db, size);
if (rc) {
kfree(db);
- pr_err("Error reading %s var: %d\n", key, rc);
- return NULL;
+ return ERR_PTR(rc);
}
return db;
@@ -69,7 +75,11 @@ static int __init load_powerpc_certs(void)
*/
db = get_cert_list("db", 3, );
if (!db) {
- pr_err("Couldn't get db list from firmware\n");
+ pr_info("Couldn't get db list from firmware\n");
+ } else if (IS_ERR(db)) {
+ rc = PTR_ERR(db);
+ pr_err("Error reading db from firmware: %d\n", rc);
+ return rc;
} else {
rc = parse_efi_signature_list("powerpc:db", db, dbsize,
get_handler_for_db);
@@ -81,6 +91,10 @@ static int __init load_powerpc_certs(void)
dbx = get_cert_list("dbx", 4, );
if (!dbx) {
pr_info("Couldn't get dbx list from firmware\n");
+ } else if (IS_ERR(dbx)) {
+ rc = PTR_ERR(dbx);
+ pr_err("Error reading dbx from firmware: %d\n", rc);
+ return rc;
} else {
rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize,
get_handler_for_dbx);
--
2.39.1
[PATCH v5 16/25] powerpc/pseries: Expose PLPKS config values, support additional fields
From: Nayna Jain
The plpks driver uses the H_PKS_GET_CONFIG hcall to retrieve configuration
and status information about the PKS from the hypervisor.
Update _plpks_get_config() to handle some additional fields. Add getter
functions to allow the PKS configuration information to be accessed from
other files. Validate that the values we're getting comply with the spec.
While we're here, move the config struct in _plpks_get_config() off the
stack - it's getting large and we also need to make sure it doesn't cross
a page boundary.
Signed-off-by: Nayna Jain
[ajd: split patch, extend to support additional v3 API fields, minor fixes]
Co-developed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
Signed-off-by: Russell Currey
---
v3: Merge plpks fixes and signed update series with secvar series
Refresh config values in plpks_get_usedspace() (ajd)
Validate the config values being returned comply with spec (ruscur)
Return maxobjlabelsize as is (ruscur)
Move plpks.h to include/asm (ruscur)
Fix checkpatch checks (ruscur)
---
arch/powerpc/include/asm/plpks.h | 58 ++
arch/powerpc/platforms/pseries/plpks.c | 149 +++--
2 files changed, 195 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 6466aadd7145..7c5f51a9af7c 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -96,6 +96,64 @@ int plpks_read_fw_var(struct plpks_var *var);
*/
int plpks_read_bootloader_var(struct plpks_var *var);
+/**
+ * Returns if PKS is available on this LPAR.
+ */
+bool plpks_is_available(void);
+
+/**
+ * Returns version of the Platform KeyStore.
+ */
+u8 plpks_get_version(void);
+
+/**
+ * Returns hypervisor storage overhead per object, not including the size of
+ * the object or label. Only valid for config version >= 2
+ */
+u16 plpks_get_objoverhead(void);
+
+/**
+ * Returns maximum password size. Must be >= 32 bytes
+ */
+u16 plpks_get_maxpwsize(void);
+
+/**
+ * Returns maximum object size supported by Platform KeyStore.
+ */
+u16 plpks_get_maxobjectsize(void);
+
+/**
+ * Returns maximum object label size supported by Platform KeyStore.
+ */
+u16 plpks_get_maxobjectlabelsize(void);
+
+/**
+ * Returns total size of the configured Platform KeyStore.
+ */
+u32 plpks_get_totalsize(void);
+
+/**
+ * Returns used space from the total size of the Platform KeyStore.
+ */
+u32 plpks_get_usedspace(void);
+
+/**
+ * Returns bitmask of policies supported by the hypervisor.
+ */
+u32 plpks_get_supportedpolicies(void);
+
+/**
+ * Returns maximum byte size of a single object supported by the hypervisor.
+ * Only valid for config version >= 3
+ */
+u32 plpks_get_maxlargeobjectsize(void);
+
+/**
+ * Returns bitmask of signature algorithms supported for signed updates.
+ * Only valid for config version >= 3
+ */
+u64 plpks_get_signedupdatealgorithms(void);
+
#endif // CONFIG_PSERIES_PLPKS
#endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/platforms/pseries/plpks.c
b/arch/powerpc/platforms/pseries/plpks.c
index 91f3f623a2c7..1189246b03dc 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -24,8 +24,16 @@ static u8 *ospassword;
static u16 ospasswordlength;
// Retrieved with H_PKS_GET_CONFIG
+static u8 version;
+static u16 objoverhead;
static u16 maxpwsize;
static u16 maxobjsize;
+static s16 maxobjlabelsize;
+static u32 totalsize;
+static u32 usedspace;
+static u32 supportedpolicies;
+static u32 maxlargeobjectsize;
+static u64 signedupdatealgorithms;
struct plpks_auth {
u8 version;
@@ -206,32 +214,149 @@ static struct label *construct_label(char *component, u8
varos, u8 *name,
static int _plpks_get_config(void)
{
unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
- struct {
+ struct config {
u8 version;
u8 flags;
- __be32 rsvd0;
+ __be16 rsvd0;
+ __be16 objoverhead;
__be16 maxpwsize;
__be16 maxobjlabelsize;
__be16 maxobjsize;
__be32 totalsize;
__be32 usedspace;
__be32 supportedpolicies;
- __be64 rsvd1;
- } __packed config;
+ __be32 maxlargeobjectsize;
+ __be64 signedupdatealgorithms;
+ u8 rsvd1[476];
+ } __packed * config;
size_t size;
- int rc;
+ int rc = 0;
+
+ size = sizeof(*config);
+
+ // Config struct must not cross a page boundary. So long as the struct
+ // size is a power of 2, this should be fine as alignment is guaranteed
+ config = kzalloc(size, GFP_KERNEL);
+ if (!config) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ rc = plpar_hcall(H_PKS_GET_CONFIG, retbuf, virt_to_phys(config), size);
+
+ if (rc != H_SUCCESS) {
+
[PATCH v5 04/25] powerpc/secvar: Use u64 in secvar_operations
From: Michael Ellerman
There's no reason for secvar_operations to use uint64_t vs the more
common kernel type u64.
The types are compatible, but they require different printk format
strings which can lead to confusion.
Change all the secvar related routines to use u64.
Signed-off-by: Michael Ellerman
Reviewed-by: Russell Currey
Reviewed-by: Andrew Donnellan
Signed-off-by: Andrew Donnellan
---
v3: Include new patch
---
arch/powerpc/include/asm/secvar.h| 9 +++--
arch/powerpc/kernel/secvar-sysfs.c | 8
arch/powerpc/platforms/powernv/opal-secvar.c | 9 +++--
security/integrity/platform_certs/load_powerpc.c | 4 ++--
4 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
index 4cc35b58b986..07ba36f868a7 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -14,12 +14,9 @@
extern const struct secvar_operations *secvar_ops;
struct secvar_operations {
- int (*get)(const char *key, uint64_t key_len, u8 *data,
- uint64_t *data_size);
- int (*get_next)(const char *key, uint64_t *key_len,
- uint64_t keybufsize);
- int (*set)(const char *key, uint64_t key_len, u8 *data,
- uint64_t data_size);
+ int (*get)(const char *key, u64 key_len, u8 *data, u64 *data_size);
+ int (*get_next)(const char *key, u64 *key_len, u64 keybufsize);
+ int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
};
#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
index 7fa5f8ed9542..702044edf14d 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -47,7 +47,7 @@ static ssize_t format_show(struct kobject *kobj, struct
kobj_attribute *attr,
static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
- uint64_t dsize;
+ u64 dsize;
int rc;
rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, );
@@ -64,8 +64,8 @@ static ssize_t data_read(struct file *filep, struct kobject
*kobj,
struct bin_attribute *attr, char *buf, loff_t off,
size_t count)
{
- uint64_t dsize;
char *data;
+ u64 dsize;
int rc;
rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, );
@@ -166,9 +166,9 @@ static int update_kobj_size(void)
static int secvar_sysfs_load(void)
{
- char *name;
- uint64_t namesize = 0;
struct kobject *kobj;
+ u64 namesize = 0;
+ char *name;
int rc;
name = kzalloc(NAME_MAX_SIZE, GFP_KERNEL);
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c
b/arch/powerpc/platforms/powernv/opal-secvar.c
index 14133e120bdd..ef89861569e0 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -54,8 +54,7 @@ static int opal_status_to_err(int rc)
return err;
}
-static int opal_get_variable(const char *key, uint64_t ksize,
-u8 *data, uint64_t *dsize)
+static int opal_get_variable(const char *key, u64 ksize, u8 *data, u64 *dsize)
{
int rc;
@@ -71,8 +70,7 @@ static int opal_get_variable(const char *key, uint64_t ksize,
return opal_status_to_err(rc);
}
-static int opal_get_next_variable(const char *key, uint64_t *keylen,
- uint64_t keybufsize)
+static int opal_get_next_variable(const char *key, u64 *keylen, u64 keybufsize)
{
int rc;
@@ -88,8 +86,7 @@ static int opal_get_next_variable(const char *key, uint64_t
*keylen,
return opal_status_to_err(rc);
}
-static int opal_set_variable(const char *key, uint64_t ksize, u8 *data,
-uint64_t dsize)
+static int opal_set_variable(const char *key, u64 ksize, u8 *data, u64 dsize)
{
int rc;
diff --git a/security/integrity/platform_certs/load_powerpc.c
b/security/integrity/platform_certs/load_powerpc.c
index a2900cb85357..1e4f80a4e71c 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -18,7 +18,7 @@
/*
* Get a certificate list blob from the named secure variable.
*/
-static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t
*size)
+static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size)
{
int rc;
void *db;
@@ -51,7 +51,7 @@ static __init void *get_cert_list(u8 *key, unsigned long
keylen, uint64_t *size)
static int __init load_powerpc_certs(void)
{
void *db = NULL, *dbx = NULL;
- uint64_t dbsize = 0, dbxsize = 0;
+ u64 dbsize = 0, dbxsize = 0;
int rc = 0;
struct device_node *node;
--
2.39.1
