* [RFC PATCH] ARM: aspeed: Add secure boot controller support
@ 2021-10-19 8:06 Joel Stanley
2021-10-23 2:25 ` Ryan Chen
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Joel Stanley @ 2021-10-19 8:06 UTC (permalink / raw)
To: Andrew Jeffery, openbmc; +Cc: Ryan Chen
This reads out the status of the secure boot controller and exposes it
in sysfs.
An example on a AST2600A3 QEmu model:
# grep . /sys/bus/soc/devices/soc0/*
/sys/bus/soc/devices/soc0/abr_image:0
/sys/bus/soc/devices/soc0/family:AST2600
/sys/bus/soc/devices/soc0/low_security_key:0
/sys/bus/soc/devices/soc0/machine:Rainier 2U
/sys/bus/soc/devices/soc0/otp_protected:0
/sys/bus/soc/devices/soc0/revision:A3
/sys/bus/soc/devices/soc0/secure_boot:1
/sys/bus/soc/devices/soc0/serial_number:888844441234abcd
/sys/bus/soc/devices/soc0/soc_id:05030303
/sys/bus/soc/devices/soc0/uart_boot:1
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
arch/arm/boot/dts/aspeed-g6.dtsi | 5 ++
drivers/soc/aspeed/aspeed-socinfo.c | 71 +++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index ee171b3364fa..8f947ed47fc5 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -384,6 +384,11 @@ adc1: adc@1e6e9100 {
status = "disabled";
};
+ sbc: secure-boot-controller@1e6f2000 {
+ compatible = "aspeed,ast2600-sbc";
+ reg = <0x1e6f2000 0x1000>;
+ };
+
gpio0: gpio@1e780000 {
#gpio-cells = <2>;
gpio-controller;
diff --git a/drivers/soc/aspeed/aspeed-socinfo.c b/drivers/soc/aspeed/aspeed-socinfo.c
index 1ca140356a08..6fa0c891f3cb 100644
--- a/drivers/soc/aspeed/aspeed-socinfo.c
+++ b/drivers/soc/aspeed/aspeed-socinfo.c
@@ -9,6 +9,8 @@
#include <linux/slab.h>
#include <linux/sys_soc.h>
+static u32 security_status;
+
static struct {
const char *name;
const u32 id;
@@ -74,6 +76,54 @@ static const char *siliconid_to_rev(u32 siliconid)
return "??";
}
+#define SEC_STATUS 0x14
+#define ABR_IMAGE_SOURCE BIT(13)
+#define OTP_PROTECTED BIT(8)
+#define LOW_SEC_KEY BIT(7)
+#define SECURE_BOOT BIT(6)
+#define UART_BOOT BIT(5)
+
+static ssize_t abr_image_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", !!(security_status & ABR_IMAGE_SOURCE));
+}
+static DEVICE_ATTR_RO(abr_image);
+
+static ssize_t low_security_key_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", !!(security_status & LOW_SEC_KEY));
+}
+static DEVICE_ATTR_RO(low_security_key);
+
+static ssize_t otp_protected_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", !!(security_status & OTP_PROTECTED));
+}
+static DEVICE_ATTR_RO(otp_protected);
+
+static ssize_t secure_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", !!(security_status & SECURE_BOOT));
+}
+static DEVICE_ATTR_RO(secure_boot);
+
+static ssize_t uart_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ /* Invert the bit, as 1 is boot from SPI/eMMC */
+ return sprintf(buf, "%d\n", !(security_status & UART_BOOT));
+}
+static DEVICE_ATTR_RO(uart_boot);
+
+static struct attribute *aspeed_attrs[] = {
+ &dev_attr_abr_image.attr,
+ &dev_attr_low_security_key.attr,
+ &dev_attr_otp_protected.attr,
+ &dev_attr_secure_boot.attr,
+ &dev_attr_uart_boot.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(aspeed);
+
static int __init aspeed_socinfo_init(void)
{
struct soc_device_attribute *attrs;
@@ -81,6 +131,7 @@ static int __init aspeed_socinfo_init(void)
struct device_node *np;
void __iomem *reg;
bool has_chipid = false;
+ bool has_sbe = false;
u32 siliconid;
u32 chipid[2];
const char *machine = NULL;
@@ -109,6 +160,20 @@ static int __init aspeed_socinfo_init(void)
}
of_node_put(np);
+ /* AST2600 only */
+ np = of_find_compatible_node(NULL, NULL, "aspeed,ast2600-sbc");
+ if (of_device_is_available(np)) {
+ void *base = of_iomap(np, 0);
+ if (!base) {
+ of_node_put(np);
+ return -ENODEV;
+ }
+ security_status = readl(base + SEC_STATUS);
+ has_sbe = true;
+ iounmap(base);
+ of_node_put(np);
+ }
+
attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
if (!attrs)
return -ENODEV;
@@ -135,6 +200,9 @@ static int __init aspeed_socinfo_init(void)
attrs->serial_number = kasprintf(GFP_KERNEL, "%08x%08x",
chipid[1], chipid[0]);
+ if (has_sbe)
+ attrs->custom_attr_group = aspeed_groups[0];
+
soc_dev = soc_device_register(attrs);
if (IS_ERR(soc_dev)) {
kfree(attrs->soc_id);
@@ -148,6 +216,9 @@ static int __init aspeed_socinfo_init(void)
attrs->revision,
attrs->soc_id);
+ if (has_sbe && (security_status & SECURE_BOOT))
+ pr_info("AST2600 secure boot enabled\n");
+
return 0;
}
early_initcall(aspeed_socinfo_init);
--
2.33.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [RFC PATCH] ARM: aspeed: Add secure boot controller support
2021-10-19 8:06 [RFC PATCH] ARM: aspeed: Add secure boot controller support Joel Stanley
@ 2021-10-23 2:25 ` Ryan Chen
2021-10-23 5:14 ` Paul Menzel
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2021-10-23 2:25 UTC (permalink / raw)
To: Joel Stanley, Andrew Jeffery, openbmc
> -----Original Message-----
> From: joel.stan@gmail.com <joel.stan@gmail.com> On Behalf Of Joel Stanley
> Sent: Tuesday, October 19, 2021 4:06 PM
> To: Andrew Jeffery <andrew@aj.id.u>; openbmc@lists.ozlabs.org
> Cc: Ryan Chen <ryan_chen@aspeedtech.com>
> Subject: [RFC PATCH] ARM: aspeed: Add secure boot controller support
>
> This reads out the status of the secure boot controller and exposes it in sysfs.
>
> An example on a AST2600A3 QEmu model:
>
> # grep . /sys/bus/soc/devices/soc0/*
> /sys/bus/soc/devices/soc0/abr_image:0
> /sys/bus/soc/devices/soc0/family:AST2600
> /sys/bus/soc/devices/soc0/low_security_key:0
> /sys/bus/soc/devices/soc0/machine:Rainier 2U
> /sys/bus/soc/devices/soc0/otp_protected:0
> /sys/bus/soc/devices/soc0/revision:A3
> /sys/bus/soc/devices/soc0/secure_boot:1
> /sys/bus/soc/devices/soc0/serial_number:888844441234abcd
> /sys/bus/soc/devices/soc0/soc_id:05030303
> /sys/bus/soc/devices/soc0/uart_boot:1
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
> arch/arm/boot/dts/aspeed-g6.dtsi | 5 ++
> drivers/soc/aspeed/aspeed-socinfo.c | 71 +++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi
> b/arch/arm/boot/dts/aspeed-g6.dtsi
> index ee171b3364fa..8f947ed47fc5 100644
> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -384,6 +384,11 @@ adc1: adc@1e6e9100 {
> status = "disabled";
> };
>
> + sbc: secure-boot-controller@1e6f2000 {
> + compatible = "aspeed,ast2600-sbc";
> + reg = <0x1e6f2000 0x1000>;
> + };
> +
> gpio0: gpio@1e780000 {
> #gpio-cells = <2>;
> gpio-controller;
> diff --git a/drivers/soc/aspeed/aspeed-socinfo.c
> b/drivers/soc/aspeed/aspeed-socinfo.c
> index 1ca140356a08..6fa0c891f3cb 100644
> --- a/drivers/soc/aspeed/aspeed-socinfo.c
> +++ b/drivers/soc/aspeed/aspeed-socinfo.c
> @@ -9,6 +9,8 @@
> #include <linux/slab.h>
> #include <linux/sys_soc.h>
>
> +static u32 security_status;
> +
> static struct {
> const char *name;
> const u32 id;
> @@ -74,6 +76,54 @@ static const char *siliconid_to_rev(u32 siliconid)
> return "??";
> }
>
> +#define SEC_STATUS 0x14
> +#define ABR_IMAGE_SOURCE BIT(13)
> +#define OTP_PROTECTED BIT(8)
> +#define LOW_SEC_KEY BIT(7)
> +#define SECURE_BOOT BIT(6)
> +#define UART_BOOT BIT(5)
> +
> +static ssize_t abr_image_show(struct device *dev, struct
> +device_attribute *attr, char *buf) {
> + return sprintf(buf, "%d\n", !!(security_status & ABR_IMAGE_SOURCE)); }
> +static DEVICE_ATTR_RO(abr_image);
> +
> +static ssize_t low_security_key_show(struct device *dev, struct
> +device_attribute *attr, char *buf) {
> + return sprintf(buf, "%d\n", !!(security_status & LOW_SEC_KEY)); }
> +static DEVICE_ATTR_RO(low_security_key);
> +
> +static ssize_t otp_protected_show(struct device *dev, struct
> +device_attribute *attr, char *buf) {
> + return sprintf(buf, "%d\n", !!(security_status & OTP_PROTECTED)); }
> +static DEVICE_ATTR_RO(otp_protected);
> +
> +static ssize_t secure_boot_show(struct device *dev, struct
> +device_attribute *attr, char *buf) {
> + return sprintf(buf, "%d\n", !!(security_status & SECURE_BOOT)); }
> +static DEVICE_ATTR_RO(secure_boot);
> +
> +static ssize_t uart_boot_show(struct device *dev, struct
> +device_attribute *attr, char *buf) {
> + /* Invert the bit, as 1 is boot from SPI/eMMC */
> + return sprintf(buf, "%d\n", !(security_status & UART_BOOT)); } static
> +DEVICE_ATTR_RO(uart_boot);
> +
> +static struct attribute *aspeed_attrs[] = {
> + &dev_attr_abr_image.attr,
> + &dev_attr_low_security_key.attr,
> + &dev_attr_otp_protected.attr,
> + &dev_attr_secure_boot.attr,
> + &dev_attr_uart_boot.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(aspeed);
> +
> static int __init aspeed_socinfo_init(void) {
> struct soc_device_attribute *attrs;
> @@ -81,6 +131,7 @@ static int __init aspeed_socinfo_init(void)
> struct device_node *np;
> void __iomem *reg;
> bool has_chipid = false;
> + bool has_sbe = false;
> u32 siliconid;
> u32 chipid[2];
> const char *machine = NULL;
> @@ -109,6 +160,20 @@ static int __init aspeed_socinfo_init(void)
> }
> of_node_put(np);
>
> + /* AST2600 only */
> + np = of_find_compatible_node(NULL, NULL, "aspeed,ast2600-sbc");
> + if (of_device_is_available(np)) {
> + void *base = of_iomap(np, 0);
> + if (!base) {
> + of_node_put(np);
> + return -ENODEV;
> + }
> + security_status = readl(base + SEC_STATUS);
> + has_sbe = true;
> + iounmap(base);
> + of_node_put(np);
> + }
> +
> attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> if (!attrs)
> return -ENODEV;
> @@ -135,6 +200,9 @@ static int __init aspeed_socinfo_init(void)
> attrs->serial_number = kasprintf(GFP_KERNEL, "%08x%08x",
> chipid[1], chipid[0]);
>
> + if (has_sbe)
> + attrs->custom_attr_group = aspeed_groups[0];
> +
> soc_dev = soc_device_register(attrs);
> if (IS_ERR(soc_dev)) {
> kfree(attrs->soc_id);
> @@ -148,6 +216,9 @@ static int __init aspeed_socinfo_init(void)
> attrs->revision,
> attrs->soc_id);
>
> + if (has_sbe && (security_status & SECURE_BOOT))
> + pr_info("AST2600 secure boot enabled\n");
> +
> return 0;
> }
> early_initcall(aspeed_socinfo_init);
> --
> 2.33.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] ARM: aspeed: Add secure boot controller support
2021-10-19 8:06 [RFC PATCH] ARM: aspeed: Add secure boot controller support Joel Stanley
2021-10-23 2:25 ` Ryan Chen
@ 2021-10-23 5:14 ` Paul Menzel
2021-11-16 5:39 ` Joel Stanley
2021-10-23 5:16 ` Paul Menzel
2021-10-23 5:17 ` Paul Menzel
3 siblings, 1 reply; 8+ messages in thread
From: Paul Menzel @ 2021-10-23 5:14 UTC (permalink / raw)
To: Joel Stanley; +Cc: Andrew Jeffery, openbmc, Ryan Chen
Dear Joel,
Am 19.10.21 um 10:06 schrieb Joel Stanley:
> This reads out the status of the secure boot controller and exposes it
> in sysfs.
Please elaborate, what that secure boot controller is, what ASpeed
devices support it (code seems to check for AST2600), and where it is
documented.
> An example on a AST2600A3 QEmu model:
Nit: QEMU
>
> # grep . /sys/bus/soc/devices/soc0/*
> /sys/bus/soc/devices/soc0/abr_image:0
> /sys/bus/soc/devices/soc0/family:AST2600
> /sys/bus/soc/devices/soc0/low_security_key:0
> /sys/bus/soc/devices/soc0/machine:Rainier 2U
> /sys/bus/soc/devices/soc0/otp_protected:0
> /sys/bus/soc/devices/soc0/revision:A3
> /sys/bus/soc/devices/soc0/secure_boot:1
> /sys/bus/soc/devices/soc0/serial_number:888844441234abcd
> /sys/bus/soc/devices/soc0/soc_id:05030303
> /sys/bus/soc/devices/soc0/uart_boot:1
Can you please paste the full command to start the QEMU virtual machine?
Maybe also mention the new log message:
AST2600 secure boot enabled
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/boot/dts/aspeed-g6.dtsi | 5 ++
> drivers/soc/aspeed/aspeed-socinfo.c | 71 +++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
> index ee171b3364fa..8f947ed47fc5 100644
> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -384,6 +384,11 @@ adc1: adc@1e6e9100 {
> status = "disabled";
> };
>
> + sbc: secure-boot-controller@1e6f2000 {
> + compatible = "aspeed,ast2600-sbc";
> + reg = <0x1e6f2000 0x1000>;
> + };
> +
> gpio0: gpio@1e780000 {
> #gpio-cells = <2>;
> gpio-controller;
> diff --git a/drivers/soc/aspeed/aspeed-socinfo.c b/drivers/soc/aspeed/aspeed-socinfo.c
> index 1ca140356a08..6fa0c891f3cb 100644
> --- a/drivers/soc/aspeed/aspeed-socinfo.c
> +++ b/drivers/soc/aspeed/aspeed-socinfo.c
> @@ -9,6 +9,8 @@
> #include <linux/slab.h>
> #include <linux/sys_soc.h>
>
> +static u32 security_status;
> +
> static struct {
> const char *name;
> const u32 id;
> @@ -74,6 +76,54 @@ static const char *siliconid_to_rev(u32 siliconid)
> return "??";
> }
>
> +#define SEC_STATUS 0x14
> +#define ABR_IMAGE_SOURCE BIT(13)
> +#define OTP_PROTECTED BIT(8)
> +#define LOW_SEC_KEY BIT(7)
> +#define SECURE_BOOT BIT(6)
> +#define UART_BOOT BIT(5)
> +
> +static ssize_t abr_image_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & ABR_IMAGE_SOURCE));
> +}
> +static DEVICE_ATTR_RO(abr_image);
> +
> +static ssize_t low_security_key_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & LOW_SEC_KEY));
> +}
> +static DEVICE_ATTR_RO(low_security_key);
> +
> +static ssize_t otp_protected_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & OTP_PROTECTED));
> +}
> +static DEVICE_ATTR_RO(otp_protected);
> +
> +static ssize_t secure_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & SECURE_BOOT));
> +}
> +static DEVICE_ATTR_RO(secure_boot);
> +
> +static ssize_t uart_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + /* Invert the bit, as 1 is boot from SPI/eMMC */
> + return sprintf(buf, "%d\n", !(security_status & UART_BOOT));
> +}
> +static DEVICE_ATTR_RO(uart_boot);
> +
> +static struct attribute *aspeed_attrs[] = {
> + &dev_attr_abr_image.attr,
> + &dev_attr_low_security_key.attr,
> + &dev_attr_otp_protected.attr,
> + &dev_attr_secure_boot.attr,
> + &dev_attr_uart_boot.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(aspeed);
> +
> static int __init aspeed_socinfo_init(void)
> {
> struct soc_device_attribute *attrs;
> @@ -81,6 +131,7 @@ static int __init aspeed_socinfo_init(void)
> struct device_node *np;
> void __iomem *reg;
> bool has_chipid = false;
> + bool has_sbe = false;
> u32 siliconid;
> u32 chipid[2];
> const char *machine = NULL;
> @@ -109,6 +160,20 @@ static int __init aspeed_socinfo_init(void)
> }
> of_node_put(np);
>
> + /* AST2600 only */
> + np = of_find_compatible_node(NULL, NULL, "aspeed,ast2600-sbc");
> + if (of_device_is_available(np)) {
> + void *base = of_iomap(np, 0);
> + if (!base) {
> + of_node_put(np);
> + return -ENODEV;
> + }
> + security_status = readl(base + SEC_STATUS);
> + has_sbe = true;
> + iounmap(base);
> + of_node_put(np);
> + }
> +
> attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> if (!attrs)
> return -ENODEV;
> @@ -135,6 +200,9 @@ static int __init aspeed_socinfo_init(void)
> attrs->serial_number = kasprintf(GFP_KERNEL, "%08x%08x",
> chipid[1], chipid[0]);
>
> + if (has_sbe)
> + attrs->custom_attr_group = aspeed_groups[0];
> +
> soc_dev = soc_device_register(attrs);
> if (IS_ERR(soc_dev)) {
> kfree(attrs->soc_id);
> @@ -148,6 +216,9 @@ static int __init aspeed_socinfo_init(void)
> attrs->revision,
> attrs->soc_id);
>
> + if (has_sbe && (security_status & SECURE_BOOT))
> + pr_info("AST2600 secure boot enabled\n");
> +
Maybe it is more interesting to log, when it is not enabled?
> return 0;
> }
> early_initcall(aspeed_socinfo_init);
>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] ARM: aspeed: Add secure boot controller support
2021-10-19 8:06 [RFC PATCH] ARM: aspeed: Add secure boot controller support Joel Stanley
2021-10-23 2:25 ` Ryan Chen
2021-10-23 5:14 ` Paul Menzel
@ 2021-10-23 5:16 ` Paul Menzel
2021-10-23 5:17 ` Paul Menzel
3 siblings, 0 replies; 8+ messages in thread
From: Paul Menzel @ 2021-10-23 5:16 UTC (permalink / raw)
To: Joel Stanley; +Cc: Andrew Jeffery, Ryan Chen, openbmc
[Resent with typo fixed Andrew’s address]
Dear Joel,
Am 19.10.21 um 10:06 schrieb Joel Stanley:
> This reads out the status of the secure boot controller and exposes it
> in sysfs.
Please elaborate, what that secure boot controller is, what ASpeed
devices support it (code seems to check for AST2600), and where it is
documented.
> An example on a AST2600A3 QEmu model:
Nit: QEMU
>
> # grep . /sys/bus/soc/devices/soc0/*
> /sys/bus/soc/devices/soc0/abr_image:0
> /sys/bus/soc/devices/soc0/family:AST2600
> /sys/bus/soc/devices/soc0/low_security_key:0
> /sys/bus/soc/devices/soc0/machine:Rainier 2U
> /sys/bus/soc/devices/soc0/otp_protected:0
> /sys/bus/soc/devices/soc0/revision:A3
> /sys/bus/soc/devices/soc0/secure_boot:1
> /sys/bus/soc/devices/soc0/serial_number:888844441234abcd
> /sys/bus/soc/devices/soc0/soc_id:05030303
> /sys/bus/soc/devices/soc0/uart_boot:1
Can you please paste the full command to start the QEMU virtual machine?
Maybe also mention the new log message:
AST2600 secure boot enabled
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/boot/dts/aspeed-g6.dtsi | 5 ++
> drivers/soc/aspeed/aspeed-socinfo.c | 71 +++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
> index ee171b3364fa..8f947ed47fc5 100644
> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -384,6 +384,11 @@ adc1: adc@1e6e9100 {
> status = "disabled";
> };
>
> + sbc: secure-boot-controller@1e6f2000 {
> + compatible = "aspeed,ast2600-sbc";
> + reg = <0x1e6f2000 0x1000>;
> + };
> +
> gpio0: gpio@1e780000 {
> #gpio-cells = <2>;
> gpio-controller;
> diff --git a/drivers/soc/aspeed/aspeed-socinfo.c b/drivers/soc/aspeed/aspeed-socinfo.c
> index 1ca140356a08..6fa0c891f3cb 100644
> --- a/drivers/soc/aspeed/aspeed-socinfo.c
> +++ b/drivers/soc/aspeed/aspeed-socinfo.c
> @@ -9,6 +9,8 @@
> #include <linux/slab.h>
> #include <linux/sys_soc.h>
>
> +static u32 security_status;
> +
> static struct {
> const char *name;
> const u32 id;
> @@ -74,6 +76,54 @@ static const char *siliconid_to_rev(u32 siliconid)
> return "??";
> }
>
> +#define SEC_STATUS 0x14
> +#define ABR_IMAGE_SOURCE BIT(13)
> +#define OTP_PROTECTED BIT(8)
> +#define LOW_SEC_KEY BIT(7)
> +#define SECURE_BOOT BIT(6)
> +#define UART_BOOT BIT(5)
> +
> +static ssize_t abr_image_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & ABR_IMAGE_SOURCE));
> +}
> +static DEVICE_ATTR_RO(abr_image);
> +
> +static ssize_t low_security_key_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & LOW_SEC_KEY));
> +}
> +static DEVICE_ATTR_RO(low_security_key);
> +
> +static ssize_t otp_protected_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & OTP_PROTECTED));
> +}
> +static DEVICE_ATTR_RO(otp_protected);
> +
> +static ssize_t secure_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & SECURE_BOOT));
> +}
> +static DEVICE_ATTR_RO(secure_boot);
> +
> +static ssize_t uart_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + /* Invert the bit, as 1 is boot from SPI/eMMC */
> + return sprintf(buf, "%d\n", !(security_status & UART_BOOT));
> +}
> +static DEVICE_ATTR_RO(uart_boot);
> +
> +static struct attribute *aspeed_attrs[] = {
> + &dev_attr_abr_image.attr,
> + &dev_attr_low_security_key.attr,
> + &dev_attr_otp_protected.attr,
> + &dev_attr_secure_boot.attr,
> + &dev_attr_uart_boot.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(aspeed);
> +
> static int __init aspeed_socinfo_init(void)
> {
> struct soc_device_attribute *attrs;
> @@ -81,6 +131,7 @@ static int __init aspeed_socinfo_init(void)
> struct device_node *np;
> void __iomem *reg;
> bool has_chipid = false;
> + bool has_sbe = false;
> u32 siliconid;
> u32 chipid[2];
> const char *machine = NULL;
> @@ -109,6 +160,20 @@ static int __init aspeed_socinfo_init(void)
> }
> of_node_put(np);
>
> + /* AST2600 only */
> + np = of_find_compatible_node(NULL, NULL, "aspeed,ast2600-sbc");
> + if (of_device_is_available(np)) {
> + void *base = of_iomap(np, 0);
> + if (!base) {
> + of_node_put(np);
> + return -ENODEV;
> + }
> + security_status = readl(base + SEC_STATUS);
> + has_sbe = true;
> + iounmap(base);
> + of_node_put(np);
> + }
> +
> attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> if (!attrs)
> return -ENODEV;
> @@ -135,6 +200,9 @@ static int __init aspeed_socinfo_init(void)
> attrs->serial_number = kasprintf(GFP_KERNEL, "%08x%08x",
> chipid[1], chipid[0]);
>
> + if (has_sbe)
> + attrs->custom_attr_group = aspeed_groups[0];
> +
> soc_dev = soc_device_register(attrs);
> if (IS_ERR(soc_dev)) {
> kfree(attrs->soc_id);
> @@ -148,6 +216,9 @@ static int __init aspeed_socinfo_init(void)
> attrs->revision,
> attrs->soc_id);
>
> + if (has_sbe && (security_status & SECURE_BOOT))
> + pr_info("AST2600 secure boot enabled\n");
> +
Maybe it is more interesting to log, when it is not enabled?
> return 0;
> }
> early_initcall(aspeed_socinfo_init);
>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] ARM: aspeed: Add secure boot controller support
2021-10-19 8:06 [RFC PATCH] ARM: aspeed: Add secure boot controller support Joel Stanley
` (2 preceding siblings ...)
2021-10-23 5:16 ` Paul Menzel
@ 2021-10-23 5:17 ` Paul Menzel
3 siblings, 0 replies; 8+ messages in thread
From: Paul Menzel @ 2021-10-23 5:17 UTC (permalink / raw)
To: Joel Stanley; +Cc: Andrew Jeffery, openbmc, Ryan Chen
[2nd try: Resent with typo fixed Andrew’s address]
Dear Joel,
Am 19.10.21 um 10:06 schrieb Joel Stanley:
> This reads out the status of the secure boot controller and exposes it
> in sysfs.
Please elaborate, what that secure boot controller is, what ASpeed
devices support it (code seems to check for AST2600), and where it is
documented.
> An example on a AST2600A3 QEmu model:
Nit: QEMU
>
> # grep . /sys/bus/soc/devices/soc0/*
> /sys/bus/soc/devices/soc0/abr_image:0
> /sys/bus/soc/devices/soc0/family:AST2600
> /sys/bus/soc/devices/soc0/low_security_key:0
> /sys/bus/soc/devices/soc0/machine:Rainier 2U
> /sys/bus/soc/devices/soc0/otp_protected:0
> /sys/bus/soc/devices/soc0/revision:A3
> /sys/bus/soc/devices/soc0/secure_boot:1
> /sys/bus/soc/devices/soc0/serial_number:888844441234abcd
> /sys/bus/soc/devices/soc0/soc_id:05030303
> /sys/bus/soc/devices/soc0/uart_boot:1
Can you please paste the full command to start the QEMU virtual machine?
Maybe also mention the new log message:
AST2600 secure boot enabled
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/boot/dts/aspeed-g6.dtsi | 5 ++
> drivers/soc/aspeed/aspeed-socinfo.c | 71 +++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
> index ee171b3364fa..8f947ed47fc5 100644
> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -384,6 +384,11 @@ adc1: adc@1e6e9100 {
> status = "disabled";
> };
>
> + sbc: secure-boot-controller@1e6f2000 {
> + compatible = "aspeed,ast2600-sbc";
> + reg = <0x1e6f2000 0x1000>;
> + };
> +
> gpio0: gpio@1e780000 {
> #gpio-cells = <2>;
> gpio-controller;
> diff --git a/drivers/soc/aspeed/aspeed-socinfo.c b/drivers/soc/aspeed/aspeed-socinfo.c
> index 1ca140356a08..6fa0c891f3cb 100644
> --- a/drivers/soc/aspeed/aspeed-socinfo.c
> +++ b/drivers/soc/aspeed/aspeed-socinfo.c
> @@ -9,6 +9,8 @@
> #include <linux/slab.h>
> #include <linux/sys_soc.h>
>
> +static u32 security_status;
> +
> static struct {
> const char *name;
> const u32 id;
> @@ -74,6 +76,54 @@ static const char *siliconid_to_rev(u32 siliconid)
> return "??";
> }
>
> +#define SEC_STATUS 0x14
> +#define ABR_IMAGE_SOURCE BIT(13)
> +#define OTP_PROTECTED BIT(8)
> +#define LOW_SEC_KEY BIT(7)
> +#define SECURE_BOOT BIT(6)
> +#define UART_BOOT BIT(5)
> +
> +static ssize_t abr_image_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & ABR_IMAGE_SOURCE));
> +}
> +static DEVICE_ATTR_RO(abr_image);
> +
> +static ssize_t low_security_key_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & LOW_SEC_KEY));
> +}
> +static DEVICE_ATTR_RO(low_security_key);
> +
> +static ssize_t otp_protected_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & OTP_PROTECTED));
> +}
> +static DEVICE_ATTR_RO(otp_protected);
> +
> +static ssize_t secure_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(security_status & SECURE_BOOT));
> +}
> +static DEVICE_ATTR_RO(secure_boot);
> +
> +static ssize_t uart_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + /* Invert the bit, as 1 is boot from SPI/eMMC */
> + return sprintf(buf, "%d\n", !(security_status & UART_BOOT));
> +}
> +static DEVICE_ATTR_RO(uart_boot);
> +
> +static struct attribute *aspeed_attrs[] = {
> + &dev_attr_abr_image.attr,
> + &dev_attr_low_security_key.attr,
> + &dev_attr_otp_protected.attr,
> + &dev_attr_secure_boot.attr,
> + &dev_attr_uart_boot.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(aspeed);
> +
> static int __init aspeed_socinfo_init(void)
> {
> struct soc_device_attribute *attrs;
> @@ -81,6 +131,7 @@ static int __init aspeed_socinfo_init(void)
> struct device_node *np;
> void __iomem *reg;
> bool has_chipid = false;
> + bool has_sbe = false;
> u32 siliconid;
> u32 chipid[2];
> const char *machine = NULL;
> @@ -109,6 +160,20 @@ static int __init aspeed_socinfo_init(void)
> }
> of_node_put(np);
>
> + /* AST2600 only */
> + np = of_find_compatible_node(NULL, NULL, "aspeed,ast2600-sbc");
> + if (of_device_is_available(np)) {
> + void *base = of_iomap(np, 0);
> + if (!base) {
> + of_node_put(np);
> + return -ENODEV;
> + }
> + security_status = readl(base + SEC_STATUS);
> + has_sbe = true;
> + iounmap(base);
> + of_node_put(np);
> + }
> +
> attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> if (!attrs)
> return -ENODEV;
> @@ -135,6 +200,9 @@ static int __init aspeed_socinfo_init(void)
> attrs->serial_number = kasprintf(GFP_KERNEL, "%08x%08x",
> chipid[1], chipid[0]);
>
> + if (has_sbe)
> + attrs->custom_attr_group = aspeed_groups[0];
> +
> soc_dev = soc_device_register(attrs);
> if (IS_ERR(soc_dev)) {
> kfree(attrs->soc_id);
> @@ -148,6 +216,9 @@ static int __init aspeed_socinfo_init(void)
> attrs->revision,
> attrs->soc_id);
>
> + if (has_sbe && (security_status & SECURE_BOOT))
> + pr_info("AST2600 secure boot enabled\n");
> +
Maybe it is more interesting to log, when it is not enabled?
> return 0;
> }
> early_initcall(aspeed_socinfo_init);
>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] ARM: aspeed: Add secure boot controller support
2021-10-23 5:14 ` Paul Menzel
@ 2021-11-16 5:39 ` Joel Stanley
2021-11-16 10:15 ` Paul Menzel
0 siblings, 1 reply; 8+ messages in thread
From: Joel Stanley @ 2021-11-16 5:39 UTC (permalink / raw)
To: Paul Menzel; +Cc: Andrew Jeffery, OpenBMC Maillist, Ryan Chen
On Sat, 23 Oct 2021 at 05:15, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Joel,
>
>
> Am 19.10.21 um 10:06 schrieb Joel Stanley:
> > This reads out the status of the secure boot controller and exposes it
> > in sysfs.
>
> Please elaborate, what that secure boot controller is, what ASpeed
ASPEED
> devices support it (code seems to check for AST2600), and where it is
> documented.
>
> > An example on a AST2600A3 QEmu model:
>
> Nit: QEMU
>
> >
> > # grep . /sys/bus/soc/devices/soc0/*
> > /sys/bus/soc/devices/soc0/abr_image:0
> > /sys/bus/soc/devices/soc0/family:AST2600
> > /sys/bus/soc/devices/soc0/low_security_key:0
> > /sys/bus/soc/devices/soc0/machine:Rainier 2U
> > /sys/bus/soc/devices/soc0/otp_protected:0
> > /sys/bus/soc/devices/soc0/revision:A3
> > /sys/bus/soc/devices/soc0/secure_boot:1
> > /sys/bus/soc/devices/soc0/serial_number:888844441234abcd
> > /sys/bus/soc/devices/soc0/soc_id:05030303
> > /sys/bus/soc/devices/soc0/uart_boot:1
>
> Can you please paste the full command to start the QEMU virtual machine?
The first hit on my search engine is for the QEMU documentation:
https://qemu.readthedocs.io/en/latest/system/arm/aspeed.html
I think this is sufficant.
>
> Maybe also mention the new log message:
>
> AST2600 secure boot enabled
Sure.
>
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> > arch/arm/boot/dts/aspeed-g6.dtsi | 5 ++
> > drivers/soc/aspeed/aspeed-socinfo.c | 71 +++++++++++++++++++++++++++++
> > 2 files changed, 76 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
> > index ee171b3364fa..8f947ed47fc5 100644
> > --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> > +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> > @@ -384,6 +384,11 @@ adc1: adc@1e6e9100 {
> > status = "disabled";
> > };
> >
> > + sbc: secure-boot-controller@1e6f2000 {
> > + compatible = "aspeed,ast2600-sbc";
> > + reg = <0x1e6f2000 0x1000>;
> > + };
> > +
> > gpio0: gpio@1e780000 {
> > #gpio-cells = <2>;
> > gpio-controller;
> > diff --git a/drivers/soc/aspeed/aspeed-socinfo.c b/drivers/soc/aspeed/aspeed-socinfo.c
> > index 1ca140356a08..6fa0c891f3cb 100644
> > --- a/drivers/soc/aspeed/aspeed-socinfo.c
> > +++ b/drivers/soc/aspeed/aspeed-socinfo.c
> > @@ -9,6 +9,8 @@
> > #include <linux/slab.h>
> > #include <linux/sys_soc.h>
> >
> > +static u32 security_status;
> > +
> > static struct {
> > const char *name;
> > const u32 id;
> > @@ -74,6 +76,54 @@ static const char *siliconid_to_rev(u32 siliconid)
> > return "??";
> > }
> >
> > +#define SEC_STATUS 0x14
> > +#define ABR_IMAGE_SOURCE BIT(13)
> > +#define OTP_PROTECTED BIT(8)
> > +#define LOW_SEC_KEY BIT(7)
> > +#define SECURE_BOOT BIT(6)
> > +#define UART_BOOT BIT(5)
> > +
> > +static ssize_t abr_image_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + return sprintf(buf, "%d\n", !!(security_status & ABR_IMAGE_SOURCE));
> > +}
> > +static DEVICE_ATTR_RO(abr_image);
> > +
> > +static ssize_t low_security_key_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + return sprintf(buf, "%d\n", !!(security_status & LOW_SEC_KEY));
> > +}
> > +static DEVICE_ATTR_RO(low_security_key);
> > +
> > +static ssize_t otp_protected_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + return sprintf(buf, "%d\n", !!(security_status & OTP_PROTECTED));
> > +}
> > +static DEVICE_ATTR_RO(otp_protected);
> > +
> > +static ssize_t secure_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + return sprintf(buf, "%d\n", !!(security_status & SECURE_BOOT));
> > +}
> > +static DEVICE_ATTR_RO(secure_boot);
> > +
> > +static ssize_t uart_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + /* Invert the bit, as 1 is boot from SPI/eMMC */
> > + return sprintf(buf, "%d\n", !(security_status & UART_BOOT));
> > +}
> > +static DEVICE_ATTR_RO(uart_boot);
> > +
> > +static struct attribute *aspeed_attrs[] = {
> > + &dev_attr_abr_image.attr,
> > + &dev_attr_low_security_key.attr,
> > + &dev_attr_otp_protected.attr,
> > + &dev_attr_secure_boot.attr,
> > + &dev_attr_uart_boot.attr,
> > + NULL,
> > +};
> > +ATTRIBUTE_GROUPS(aspeed);
> > +
> > static int __init aspeed_socinfo_init(void)
> > {
> > struct soc_device_attribute *attrs;
> > @@ -81,6 +131,7 @@ static int __init aspeed_socinfo_init(void)
> > struct device_node *np;
> > void __iomem *reg;
> > bool has_chipid = false;
> > + bool has_sbe = false;
> > u32 siliconid;
> > u32 chipid[2];
> > const char *machine = NULL;
> > @@ -109,6 +160,20 @@ static int __init aspeed_socinfo_init(void)
> > }
> > of_node_put(np);
> >
> > + /* AST2600 only */
> > + np = of_find_compatible_node(NULL, NULL, "aspeed,ast2600-sbc");
> > + if (of_device_is_available(np)) {
> > + void *base = of_iomap(np, 0);
> > + if (!base) {
> > + of_node_put(np);
> > + return -ENODEV;
> > + }
> > + security_status = readl(base + SEC_STATUS);
> > + has_sbe = true;
> > + iounmap(base);
> > + of_node_put(np);
> > + }
> > +
> > attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> > if (!attrs)
> > return -ENODEV;
> > @@ -135,6 +200,9 @@ static int __init aspeed_socinfo_init(void)
> > attrs->serial_number = kasprintf(GFP_KERNEL, "%08x%08x",
> > chipid[1], chipid[0]);
> >
> > + if (has_sbe)
> > + attrs->custom_attr_group = aspeed_groups[0];
> > +
> > soc_dev = soc_device_register(attrs);
> > if (IS_ERR(soc_dev)) {
> > kfree(attrs->soc_id);
> > @@ -148,6 +216,9 @@ static int __init aspeed_socinfo_init(void)
> > attrs->revision,
> > attrs->soc_id);
> >
> > + if (has_sbe && (security_status & SECURE_BOOT))
> > + pr_info("AST2600 secure boot enabled\n");
> > +
>
> Maybe it is more interesting to log, when it is not enabled?
By default the soc will boot without secure boot, and this is not a
problem. It is informative to know that the system has been configured
with secure boot as this indicates the system has it enabled, and has
correctly booted (otherwise you would not see any message).
>
> > return 0;
> > }
> > early_initcall(aspeed_socinfo_init);
> >
>
>
> Kind regards,
>
> Paul
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] ARM: aspeed: Add secure boot controller support
2021-11-16 5:39 ` Joel Stanley
@ 2021-11-16 10:15 ` Paul Menzel
2021-11-17 3:03 ` Joel Stanley
0 siblings, 1 reply; 8+ messages in thread
From: Paul Menzel @ 2021-11-16 10:15 UTC (permalink / raw)
To: Joel Stanley; +Cc: Andrew Jeffery, OpenBMC Maillist, Ryan Chen
Dear Joel,
Am 16.11.21 um 06:39 schrieb Joel Stanley:
> On Sat, 23 Oct 2021 at 05:15, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>> Am 19.10.21 um 10:06 schrieb Joel Stanley:
>>> This reads out the status of the secure boot controller and exposes it
>>> in sysfs.
>>
>> Please elaborate, what that secure boot controller is, what ASpeed
>
> ASPEED
Point taken. ;-)
>> devices support it (code seems to check for AST2600), and where it is
>> documented.
>>
>>> An example on a AST2600A3 QEmu model:
>>
>> Nit: QEMU
>>
>>>
>>> # grep . /sys/bus/soc/devices/soc0/*
>>> /sys/bus/soc/devices/soc0/abr_image:0
>>> /sys/bus/soc/devices/soc0/family:AST2600
>>> /sys/bus/soc/devices/soc0/low_security_key:0
>>> /sys/bus/soc/devices/soc0/machine:Rainier 2U
>>> /sys/bus/soc/devices/soc0/otp_protected:0
>>> /sys/bus/soc/devices/soc0/revision:A3
>>> /sys/bus/soc/devices/soc0/secure_boot:1
>>> /sys/bus/soc/devices/soc0/serial_number:888844441234abcd
>>> /sys/bus/soc/devices/soc0/soc_id:05030303
>>> /sys/bus/soc/devices/soc0/uart_boot:1
>>
>> Can you please paste the full command to start the QEMU virtual machine?
>
> The first hit on my search engine is for the QEMU documentation:
>
> https://qemu.readthedocs.io/en/latest/system/arm/aspeed.html
>
> I think this is sufficant.
Understood. Let’s hope the page is staying up for a long time, and
nobody edits the information.
>> Maybe also mention the new log message:
>>
>> AST2600 secure boot enabled
>
> Sure.
>
>>
>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>> ---
>>> arch/arm/boot/dts/aspeed-g6.dtsi | 5 ++
>>> drivers/soc/aspeed/aspeed-socinfo.c | 71 +++++++++++++++++++++++++++++
>>> 2 files changed, 76 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
>>> index ee171b3364fa..8f947ed47fc5 100644
>>> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
>>> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
>>> @@ -384,6 +384,11 @@ adc1: adc@1e6e9100 {
>>> status = "disabled";
>>> };
>>>
>>> + sbc: secure-boot-controller@1e6f2000 {
>>> + compatible = "aspeed,ast2600-sbc";
>>> + reg = <0x1e6f2000 0x1000>;
>>> + };
>>> +
>>> gpio0: gpio@1e780000 {
>>> #gpio-cells = <2>;
>>> gpio-controller;
>>> diff --git a/drivers/soc/aspeed/aspeed-socinfo.c b/drivers/soc/aspeed/aspeed-socinfo.c
>>> index 1ca140356a08..6fa0c891f3cb 100644
>>> --- a/drivers/soc/aspeed/aspeed-socinfo.c
>>> +++ b/drivers/soc/aspeed/aspeed-socinfo.c
>>> @@ -9,6 +9,8 @@
>>> #include <linux/slab.h>
>>> #include <linux/sys_soc.h>
>>>
>>> +static u32 security_status;
>>> +
>>> static struct {
>>> const char *name;
>>> const u32 id;
>>> @@ -74,6 +76,54 @@ static const char *siliconid_to_rev(u32 siliconid)
>>> return "??";
>>> }
>>>
>>> +#define SEC_STATUS 0x14
>>> +#define ABR_IMAGE_SOURCE BIT(13)
>>> +#define OTP_PROTECTED BIT(8)
>>> +#define LOW_SEC_KEY BIT(7)
>>> +#define SECURE_BOOT BIT(6)
>>> +#define UART_BOOT BIT(5)
>>> +
>>> +static ssize_t abr_image_show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> + return sprintf(buf, "%d\n", !!(security_status & ABR_IMAGE_SOURCE));
>>> +}
>>> +static DEVICE_ATTR_RO(abr_image);
>>> +
>>> +static ssize_t low_security_key_show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> + return sprintf(buf, "%d\n", !!(security_status & LOW_SEC_KEY));
>>> +}
>>> +static DEVICE_ATTR_RO(low_security_key);
>>> +
>>> +static ssize_t otp_protected_show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> + return sprintf(buf, "%d\n", !!(security_status & OTP_PROTECTED));
>>> +}
>>> +static DEVICE_ATTR_RO(otp_protected);
>>> +
>>> +static ssize_t secure_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> + return sprintf(buf, "%d\n", !!(security_status & SECURE_BOOT));
>>> +}
>>> +static DEVICE_ATTR_RO(secure_boot);
>>> +
>>> +static ssize_t uart_boot_show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> + /* Invert the bit, as 1 is boot from SPI/eMMC */
>>> + return sprintf(buf, "%d\n", !(security_status & UART_BOOT));
>>> +}
>>> +static DEVICE_ATTR_RO(uart_boot);
>>> +
>>> +static struct attribute *aspeed_attrs[] = {
>>> + &dev_attr_abr_image.attr,
>>> + &dev_attr_low_security_key.attr,
>>> + &dev_attr_otp_protected.attr,
>>> + &dev_attr_secure_boot.attr,
>>> + &dev_attr_uart_boot.attr,
>>> + NULL,
>>> +};
>>> +ATTRIBUTE_GROUPS(aspeed);
>>> +
>>> static int __init aspeed_socinfo_init(void)
>>> {
>>> struct soc_device_attribute *attrs;
>>> @@ -81,6 +131,7 @@ static int __init aspeed_socinfo_init(void)
>>> struct device_node *np;
>>> void __iomem *reg;
>>> bool has_chipid = false;
>>> + bool has_sbe = false;
>>> u32 siliconid;
>>> u32 chipid[2];
>>> const char *machine = NULL;
>>> @@ -109,6 +160,20 @@ static int __init aspeed_socinfo_init(void)
>>> }
>>> of_node_put(np);
>>>
>>> + /* AST2600 only */
>>> + np = of_find_compatible_node(NULL, NULL, "aspeed,ast2600-sbc");
>>> + if (of_device_is_available(np)) {
>>> + void *base = of_iomap(np, 0);
>>> + if (!base) {
>>> + of_node_put(np);
>>> + return -ENODEV;
>>> + }
>>> + security_status = readl(base + SEC_STATUS);
>>> + has_sbe = true;
>>> + iounmap(base);
>>> + of_node_put(np);
>>> + }
>>> +
>>> attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
>>> if (!attrs)
>>> return -ENODEV;
>>> @@ -135,6 +200,9 @@ static int __init aspeed_socinfo_init(void)
>>> attrs->serial_number = kasprintf(GFP_KERNEL, "%08x%08x",
>>> chipid[1], chipid[0]);
>>>
>>> + if (has_sbe)
>>> + attrs->custom_attr_group = aspeed_groups[0];
>>> +
>>> soc_dev = soc_device_register(attrs);
>>> if (IS_ERR(soc_dev)) {
>>> kfree(attrs->soc_id);
>>> @@ -148,6 +216,9 @@ static int __init aspeed_socinfo_init(void)
>>> attrs->revision,
>>> attrs->soc_id);
>>>
>>> + if (has_sbe && (security_status & SECURE_BOOT))
>>> + pr_info("AST2600 secure boot enabled\n");
>>> +
>>
>> Maybe it is more interesting to log, when it is not enabled?
>
> By default the soc will boot without secure boot, and this is not a
> problem. It is informative to know that the system has been configured
> with secure boot as this indicates the system has it enabled, and has
> correctly booted (otherwise you would not see any message).
Maybe log a message in both cases? Linux also logs the message below on
my old Dell Intel laptop:
[ 0.000000] secureboot: Secure boot could not be determined (mode 0)
>>> return 0;
>>> }
>>> early_initcall(aspeed_socinfo_init);
>>>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] ARM: aspeed: Add secure boot controller support
2021-11-16 10:15 ` Paul Menzel
@ 2021-11-17 3:03 ` Joel Stanley
0 siblings, 0 replies; 8+ messages in thread
From: Joel Stanley @ 2021-11-17 3:03 UTC (permalink / raw)
To: Paul Menzel; +Cc: Andrew Jeffery, OpenBMC Maillist, Ryan Chen
On Tue, 16 Nov 2021 at 10:15, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> >>> + if (has_sbe && (security_status & SECURE_BOOT))
> >>> + pr_info("AST2600 secure boot enabled\n");
> >>> +
> >>
> >> Maybe it is more interesting to log, when it is not enabled?
> >
> > By default the soc will boot without secure boot, and this is not a
> > problem. It is informative to know that the system has been configured
> > with secure boot as this indicates the system has it enabled, and has
> > correctly booted (otherwise you would not see any message).
>
> Maybe log a message in both cases? Linux also logs the message below on
> my old Dell Intel laptop:
>
> [ 0.000000] secureboot: Secure boot could not be determined (mode 0)
I'll print a message either way, as long as the secure boot controller
compatible was found.
Thanks for the review.
Cheers,
Joel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-11-17 3:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-19 8:06 [RFC PATCH] ARM: aspeed: Add secure boot controller support Joel Stanley
2021-10-23 2:25 ` Ryan Chen
2021-10-23 5:14 ` Paul Menzel
2021-11-16 5:39 ` Joel Stanley
2021-11-16 10:15 ` Paul Menzel
2021-11-17 3:03 ` Joel Stanley
2021-10-23 5:16 ` Paul Menzel
2021-10-23 5:17 ` Paul Menzel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).