The device-tree property to check secure and trusted boot state is
different for guests(pseries) compared to baremetal(powernv).
This patch updates the existing is_ppc_secureboot_enabled() and
is_ppc_trustedboot_enabled() function to add support for pseries.
Signed-off-by: Nayna Jain <[email protected]>
Reviewed-by: Daniel Axtens <[email protected]>
---
v2:
* included Michael Ellerman's feedback.
* added Daniel Axtens's Reviewed-by.
arch/powerpc/kernel/secure_boot.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c
index 4b982324d368..efb325cbd42f 100644
--- a/arch/powerpc/kernel/secure_boot.c
+++ b/arch/powerpc/kernel/secure_boot.c
@@ -6,6 +6,7 @@
#include <linux/types.h>
#include <linux/of.h>
#include <asm/secure_boot.h>
+#include <asm/machdep.h>
static struct device_node *get_ppc_fw_sb_node(void)
{
@@ -23,12 +24,21 @@ bool is_ppc_secureboot_enabled(void)
{
struct device_node *node;
bool enabled = false;
+ u32 secureboot;
node = get_ppc_fw_sb_node();
enabled = of_property_read_bool(node, "os-secureboot-enforcing");
-
of_node_put(node);
+ if (enabled)
+ goto out;
+
+ if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) {
+ if (secureboot)
+ enabled = (secureboot > 1) ? true : false;
+ }
+
+out:
pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled");
return enabled;
@@ -38,12 +48,21 @@ bool is_ppc_trustedboot_enabled(void)
{
struct device_node *node;
bool enabled = false;
+ u32 trustedboot;
node = get_ppc_fw_sb_node();
enabled = of_property_read_bool(node, "trusted-enabled");
-
of_node_put(node);
+ if (enabled)
+ goto out;
+
+ if (!of_property_read_u32(of_root, "ibm,trusted-boot", &trustedboot)) {
+ if (trustedboot)
+ enabled = (trustedboot > 0) ? true : false;
+ }
+
+out:
pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled");
return enabled;
--
2.26.2
Hi Nayna,
Thanks! Would you be able to fold in some of the information from my
reply to v1 into the changelog? Until we have public PAPR release with
it, that information is the extent of the public documentation. It would
be good to get it into the git log rather than just floating around in
the mail archives!
A couple of small nits:
> + if (enabled)
> + goto out;
> +
> + if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) {
> + if (secureboot)
> + enabled = (secureboot > 1) ? true : false;
Your tests double up here - you don't need both the 'if' statement and
the 'secureboot > 1' ternary operator.
Just
+ if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) {
+ enabled = (secureboot > 1) ? true : false;
or even
+ if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) {
+ enabled = (secureboot > 1);
would work.
> + if (!of_property_read_u32(of_root, "ibm,trusted-boot", &trustedboot)) {
> + if (trustedboot)
> + enabled = (trustedboot > 0) ? true : false;
Likewise for trusted boot.
Regards,
Daniel
P.S. please could you add me to the cc: list for future revisions?
> + }
> +
> +out:
> pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled");
>
> return enabled;
> --
> 2.26.2
On Tue, 2020-07-14 at 16:38 +1000, Daniel Axtens wrote:
> Hi Nayna,
>
> Thanks! Would you be able to fold in some of the information from my
> reply to v1 into the changelog? Until we have public PAPR release with
> it, that information is the extent of the public documentation. It would
> be good to get it into the git log rather than just floating around in
> the mail archives!
>
> A couple of small nits:
>
> > + if (enabled)
> > + goto out;
> > +
> > + if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) {
> > + if (secureboot)
> > + enabled = (secureboot > 1) ? true : false;
>
> Your tests double up here - you don't need both the 'if' statement and
> the 'secureboot > 1' ternary operator.
>
> Just
>
> + if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) {
> + enabled = (secureboot > 1) ? true : false;
>
> or even
>
> + if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) {
> + enabled = (secureboot > 1);
>
> would work.
I haven't been following this thread, which might be the reason I'm
missing something here. The patch description should explain why the
test is for "(secureboot > 1)", rather than a fixed number.
thanks,
Mimi