Message ID | 1593882535-21368-1-git-send-email-nayna@linux.ibm.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | powerpc/pseries: detect secure and trusted boot state of the system. | expand |
Thanks Nayna! I'm hoping to get better public documentation for this soon as it's not documented in a public PAPR yet. Until then: The values of ibm,secure-boot under PowerVM are: 0 - disabled 1 - audit mode only. This patch ignores this value for Linux, which I think is the appropriate thing to do. 2 - enabled and enforcing 3-9 - enabled, OS-defined behaviour. In this patch we map all these values to enabled and enforcing. Again I think this is the appropriate thing to do. ibm,trusted-boot isn't published by a current PowerVM LPAR but will be published in future. (Currently, trusted boot state is inferred by the presence or absense of a vTPM.) It's simply 1 = enabled, 0 = disabled. As for this patch specifically, with the very small nits below, Reviewed-by: Daniel Axtens <dja@axtens.net> > - node = get_ppc_fw_sb_node(); > - enabled = of_property_read_bool(node, "os-secureboot-enforcing"); > + if (machine_is(powernv)) { > + node = get_ppc_fw_sb_node(); > + enabled = > + of_property_read_bool(node, "os-secureboot-enforcing"); > + of_node_put(node); > + } > > - of_node_put(node); > + if (machine_is(pseries)) { Maybe this should be an else if? > + secureboot = of_get_property(of_root, "ibm,secure-boot", NULL); > + if (secureboot) > + enabled = (*secureboot > 1) ? true : false; > + } > > pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled"); > > @@ -38,11 +48,20 @@ bool is_ppc_trustedboot_enabled(void) > { > struct device_node *node; > bool enabled = false; > + const u32 *trustedboot; > > - node = get_ppc_fw_sb_node(); > - enabled = of_property_read_bool(node, "trusted-enabled"); > + if (machine_is(powernv)) { > + node = get_ppc_fw_sb_node(); > + enabled = of_property_read_bool(node, "trusted-enabled"); > + of_node_put(node); > + } > > - of_node_put(node); > + if (machine_is(pseries)) { Likewise. > + trustedboot = > + of_get_property(of_root, "ibm,trusted-boot", NULL); > + if (trustedboot) > + enabled = (*trustedboot > 0) ? true : false; Regards, Daniel
As a final extra note, https://github.com/daxtens/qemu/tree/pseries-secboot is a qemu repository that fakes out the relevant properties if anyone else wants to test this. Also, > 3-9 - enabled, OS-defined behaviour. In this patch we map all these > values to enabled and enforcing. Again I think this is the > appropriate thing to do. this should read "enabled and enforcing, requirements are at the discretion of the operating system". Apologies. Regards, Daniel > ibm,trusted-boot isn't published by a current PowerVM LPAR but will be > published in future. (Currently, trusted boot state is inferred by the > presence or absense of a vTPM.) It's simply 1 = enabled, 0 = disabled. > > As for this patch specifically, with the very small nits below, > > Reviewed-by: Daniel Axtens <dja@axtens.net> > >> - node = get_ppc_fw_sb_node(); >> - enabled = of_property_read_bool(node, "os-secureboot-enforcing"); >> + if (machine_is(powernv)) { >> + node = get_ppc_fw_sb_node(); >> + enabled = >> + of_property_read_bool(node, "os-secureboot-enforcing"); >> + of_node_put(node); >> + } >> >> - of_node_put(node); >> + if (machine_is(pseries)) { > Maybe this should be an else if? > >> + secureboot = of_get_property(of_root, "ibm,secure-boot", NULL); >> + if (secureboot) >> + enabled = (*secureboot > 1) ? true : false; >> + } >> >> pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled"); >> >> @@ -38,11 +48,20 @@ bool is_ppc_trustedboot_enabled(void) >> { >> struct device_node *node; >> bool enabled = false; >> + const u32 *trustedboot; >> >> - node = get_ppc_fw_sb_node(); >> - enabled = of_property_read_bool(node, "trusted-enabled"); >> + if (machine_is(powernv)) { >> + node = get_ppc_fw_sb_node(); >> + enabled = of_property_read_bool(node, "trusted-enabled"); >> + of_node_put(node); >> + } >> >> - of_node_put(node); >> + if (machine_is(pseries)) { > Likewise. >> + trustedboot = >> + of_get_property(of_root, "ibm,trusted-boot", NULL); >> + if (trustedboot) >> + enabled = (*trustedboot > 0) ? true : false; > > Regards, > Daniel
Nayna Jain <nayna@linux.ibm.com> writes: > 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> > --- > arch/powerpc/kernel/secure_boot.c | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c > index 4b982324d368..43fc6607c7a5 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,11 +24,20 @@ bool is_ppc_secureboot_enabled(void) > { > struct device_node *node; > bool enabled = false; > + const u32 *secureboot; > > - node = get_ppc_fw_sb_node(); > - enabled = of_property_read_bool(node, "os-secureboot-enforcing"); > + if (machine_is(powernv)) { > + node = get_ppc_fw_sb_node(); > + enabled = > + of_property_read_bool(node, "os-secureboot-enforcing"); > + of_node_put(node); > + } We generally try to avoid adding new machine_is() checks if we can. In a case like this I think you can just check for both properties regardless of what platform you're on. > - of_node_put(node); > + if (machine_is(pseries)) { > + secureboot = of_get_property(of_root, "ibm,secure-boot", NULL); > + if (secureboot) > + enabled = (*secureboot > 1) ? true : false; > + } Please don't use of_get_property() in new code. Use one of the properly typed accessors that handles endian conversion for you. cheers
diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c index 4b982324d368..43fc6607c7a5 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,11 +24,20 @@ bool is_ppc_secureboot_enabled(void) { struct device_node *node; bool enabled = false; + const u32 *secureboot; - node = get_ppc_fw_sb_node(); - enabled = of_property_read_bool(node, "os-secureboot-enforcing"); + if (machine_is(powernv)) { + node = get_ppc_fw_sb_node(); + enabled = + of_property_read_bool(node, "os-secureboot-enforcing"); + of_node_put(node); + } - of_node_put(node); + if (machine_is(pseries)) { + secureboot = of_get_property(of_root, "ibm,secure-boot", NULL); + if (secureboot) + enabled = (*secureboot > 1) ? true : false; + } pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled"); @@ -38,11 +48,20 @@ bool is_ppc_trustedboot_enabled(void) { struct device_node *node; bool enabled = false; + const u32 *trustedboot; - node = get_ppc_fw_sb_node(); - enabled = of_property_read_bool(node, "trusted-enabled"); + if (machine_is(powernv)) { + node = get_ppc_fw_sb_node(); + enabled = of_property_read_bool(node, "trusted-enabled"); + of_node_put(node); + } - of_node_put(node); + if (machine_is(pseries)) { + trustedboot = + of_get_property(of_root, "ibm,trusted-boot", NULL); + if (trustedboot) + enabled = (*trustedboot > 0) ? true : false; + } pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled");
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> --- arch/powerpc/kernel/secure_boot.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)