diff mbox series

[v2] powerpc/pseries: detect secure and trusted boot state of the system.

Message ID 1594434329-31219-1-git-send-email-nayna@linux.ibm.com (mailing list archive)
State Changes Requested
Headers show
Series [v2] powerpc/pseries: detect secure and trusted boot state of the system. | expand

Commit Message

Nayna Jain July 11, 2020, 2:25 a.m. UTC 
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 <nayna@linux.ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
---
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(-)

Comments

Daniel Axtens July 14, 2020, 6:38 a.m. UTC | #1 
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
Mimi Zohar July 14, 2020, 3:07 p.m. UTC | #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
diff mbox series

Patch

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;