* [PATCH 0/3] aspeed: small cleanups
@ 2022-06-28 15:47 Cédric Le Goater
2022-06-28 15:47 ` [PATCH 1/3] aspeed/scu: Add trace events for read ops Cédric Le Goater
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Cédric Le Goater @ 2022-06-28 15:47 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Joel Stanley, Cédric Le Goater
Hello,
I was preparing a PR and I realized that these small patches had never
seen the light. Here they are.
Thanks,
C.
Cédric Le Goater (2):
aspeed/scu: Add trace events for read ops
aspeed/i2c: Change trace event for NORMAL_STOP states
Joel Stanley (1):
aspeed: sbc: Allow per-machine settings
include/hw/misc/aspeed_sbc.h | 13 ++++++++++++
hw/i2c/aspeed_i2c.c | 2 +-
hw/misc/aspeed_sbc.c | 41 ++++++++++++++++++++++++++++++++++--
hw/misc/aspeed_scu.c | 2 ++
hw/misc/trace-events | 1 +
5 files changed, 56 insertions(+), 3 deletions(-)
--
2.35.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] aspeed/scu: Add trace events for read ops
2022-06-28 15:47 [PATCH 0/3] aspeed: small cleanups Cédric Le Goater
@ 2022-06-28 15:47 ` Cédric Le Goater
2022-06-28 16:14 ` Peter Delevoryas
2022-06-28 15:47 ` [PATCH 2/3] aspeed/i2c: Change trace event for NORMAL_STOP states Cédric Le Goater
2022-06-28 15:47 ` [PATCH 3/3] aspeed: sbc: Allow per-machine settings Cédric Le Goater
2 siblings, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2022-06-28 15:47 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Joel Stanley, Cédric Le Goater
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/misc/aspeed_scu.c | 2 ++
hw/misc/trace-events | 1 +
2 files changed, 3 insertions(+)
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 19b03471fc4e..83353649064a 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -270,6 +270,7 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
break;
}
+ trace_aspeed_scu_read(offset, size, s->regs[reg]);
return s->regs[reg];
}
@@ -637,6 +638,7 @@ static uint64_t aspeed_ast2600_scu_read(void *opaque, hwaddr offset,
break;
}
+ trace_aspeed_scu_read(offset, size, s->regs[reg]);
return s->regs[reg];
}
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index c5e37b01547f..f776f24fb5d1 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -69,6 +69,7 @@ slavio_led_mem_readw(uint32_t ret) "Read diagnostic LED 0x%04x"
# aspeed_scu.c
aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32
+aspeed_scu_read(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32
# mps2-scc.c
mps2_scc_read(uint64_t offset, uint64_t data, unsigned size) "MPS2 SCC read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
--
2.35.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] aspeed/i2c: Change trace event for NORMAL_STOP states
2022-06-28 15:47 [PATCH 0/3] aspeed: small cleanups Cédric Le Goater
2022-06-28 15:47 ` [PATCH 1/3] aspeed/scu: Add trace events for read ops Cédric Le Goater
@ 2022-06-28 15:47 ` Cédric Le Goater
2022-06-28 16:13 ` Peter Delevoryas
2022-06-28 15:47 ` [PATCH 3/3] aspeed: sbc: Allow per-machine settings Cédric Le Goater
2 siblings, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2022-06-28 15:47 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Joel Stanley, Cédric Le Goater
Using a 'stop' string seems more appropriate than 'normal'.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/i2c/aspeed_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 37ae1f2e04bd..9b41bc38964f 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -58,7 +58,7 @@ static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
ARRAY_FIELD_EX32(bus->regs, I2CD_INTR_STS, SLAVE_ADDR_RX_MATCH) ?
"slave-match|" : "",
SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, NORMAL_STOP) ?
- "normal|" : "",
+ "stop|" : "",
SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, ABNORMAL) ?
"abnormal" : "");
--
2.35.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] aspeed: sbc: Allow per-machine settings
2022-06-28 15:47 [PATCH 0/3] aspeed: small cleanups Cédric Le Goater
2022-06-28 15:47 ` [PATCH 1/3] aspeed/scu: Add trace events for read ops Cédric Le Goater
2022-06-28 15:47 ` [PATCH 2/3] aspeed/i2c: Change trace event for NORMAL_STOP states Cédric Le Goater
@ 2022-06-28 15:47 ` Cédric Le Goater
2022-07-01 1:36 ` Peter Delevoryas
2 siblings, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2022-06-28 15:47 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Joel Stanley, Cédric Le Goater
From: Joel Stanley <joel@jms.id.au>
In order to correctly report secure boot running firmware the values
of certain registers must be set.
We don't yet have documentation from ASPEED on what they mean. The
meaning is inferred from u-boot's use of them.
Introduce properties so the settings can be configured per-machine.
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/misc/aspeed_sbc.h | 13 ++++++++++++
hw/misc/aspeed_sbc.c | 41 ++++++++++++++++++++++++++++++++++--
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/include/hw/misc/aspeed_sbc.h b/include/hw/misc/aspeed_sbc.h
index 67e43b53ecc3..405e6782b97a 100644
--- a/include/hw/misc/aspeed_sbc.h
+++ b/include/hw/misc/aspeed_sbc.h
@@ -17,9 +17,22 @@ OBJECT_DECLARE_TYPE(AspeedSBCState, AspeedSBCClass, ASPEED_SBC)
#define ASPEED_SBC_NR_REGS (0x93c >> 2)
+#define QSR_AES BIT(27)
+#define QSR_RSA1024 (0x0 << 12)
+#define QSR_RSA2048 (0x1 << 12)
+#define QSR_RSA3072 (0x2 << 12)
+#define QSR_RSA4096 (0x3 << 12)
+#define QSR_SHA224 (0x0 << 10)
+#define QSR_SHA256 (0x1 << 10)
+#define QSR_SHA384 (0x2 << 10)
+#define QSR_SHA512 (0x3 << 10)
+
struct AspeedSBCState {
SysBusDevice parent;
+ bool emmc_abr;
+ uint32_t signing_settings;
+
MemoryRegion iomem;
uint32_t regs[ASPEED_SBC_NR_REGS];
diff --git a/hw/misc/aspeed_sbc.c b/hw/misc/aspeed_sbc.c
index bfa8b81d01c7..3946e6179bdd 100644
--- a/hw/misc/aspeed_sbc.c
+++ b/hw/misc/aspeed_sbc.c
@@ -11,6 +11,7 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
#include "qemu/error-report.h"
+#include "hw/qdev-properties.h"
#include "hw/misc/aspeed_sbc.h"
#include "qapi/error.h"
#include "migration/vmstate.h"
@@ -19,6 +20,27 @@
#define R_STATUS (0x014 / 4)
#define R_QSR (0x040 / 4)
+/* R_STATUS */
+#define ABR_EN BIT(14) /* Mirrors SCU510[11] */
+#define ABR_IMAGE_SOURCE BIT(13)
+#define SPI_ABR_IMAGE_SOURCE BIT(12)
+#define SB_CRYPTO_KEY_EXP_DONE BIT(11)
+#define SB_CRYPTO_BUSY BIT(10)
+#define OTP_WP_EN BIT(9)
+#define OTP_ADDR_WP_EN BIT(8)
+#define LOW_SEC_KEY_EN BIT(7)
+#define SECURE_BOOT_EN BIT(6)
+#define UART_BOOT_EN BIT(5)
+/* bit 4 reserved*/
+#define OTP_CHARGE_PUMP_READY BIT(3)
+#define OTP_IDLE BIT(2)
+#define OTP_MEM_IDLE BIT(1)
+#define OTP_COMPARE_STATUS BIT(0)
+
+/* QSR */
+#define QSR_RSA_MASK (0x3 << 12)
+#define QSR_HASH_MASK (0x3 << 10)
+
static uint64_t aspeed_sbc_read(void *opaque, hwaddr addr, unsigned int size)
{
AspeedSBCState *s = ASPEED_SBC(opaque);
@@ -80,8 +102,17 @@ static void aspeed_sbc_reset(DeviceState *dev)
memset(s->regs, 0, sizeof(s->regs));
/* Set secure boot enabled with RSA4096_SHA256 and enable eMMC ABR */
- s->regs[R_STATUS] = 0x000044C6;
- s->regs[R_QSR] = 0x07C07C89;
+ s->regs[R_STATUS] = OTP_IDLE | OTP_MEM_IDLE;
+
+ if (s->emmc_abr) {
+ s->regs[R_STATUS] &= ABR_EN;
+ }
+
+ if (s->signing_settings) {
+ s->regs[R_STATUS] &= SECURE_BOOT_EN;
+ }
+
+ s->regs[R_QSR] = s->signing_settings;
}
static void aspeed_sbc_realize(DeviceState *dev, Error **errp)
@@ -105,6 +136,11 @@ static const VMStateDescription vmstate_aspeed_sbc = {
}
};
+static Property aspeed_sbc_properties[] = {
+ DEFINE_PROP_BOOL("emmc-abr", AspeedSBCState, emmc_abr, 0),
+ DEFINE_PROP_UINT32("signing-settings", AspeedSBCState, signing_settings, 0),
+};
+
static void aspeed_sbc_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -112,6 +148,7 @@ static void aspeed_sbc_class_init(ObjectClass *klass, void *data)
dc->realize = aspeed_sbc_realize;
dc->reset = aspeed_sbc_reset;
dc->vmsd = &vmstate_aspeed_sbc;
+ device_class_set_props(dc, aspeed_sbc_properties);
}
static const TypeInfo aspeed_sbc_info = {
--
2.35.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] aspeed/i2c: Change trace event for NORMAL_STOP states
2022-06-28 15:47 ` [PATCH 2/3] aspeed/i2c: Change trace event for NORMAL_STOP states Cédric Le Goater
@ 2022-06-28 16:13 ` Peter Delevoryas
0 siblings, 0 replies; 9+ messages in thread
From: Peter Delevoryas @ 2022-06-28 16:13 UTC (permalink / raw)
Cc: Peter Delevoryas, qemu-arm, Cameron Esfahani via, Peter Maydell,
Andrew Jeffery, Joel Stanley, Cédric Le Goater
> On Jun 28, 2022, at 8:47 AM, Cédric Le Goater <clg@kaod.org> wrote:
>
> Using a 'stop' string seems more appropriate than 'normal'.
Ha yes! This is an understatement, the change is much appreciated.
I've seen that trace and never realized it was referring to ‘stop’.
Reviewed-by: Peter Delevoryas <pdel@fb.com>
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/i2c/aspeed_i2c.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 37ae1f2e04bd..9b41bc38964f 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -58,7 +58,7 @@ static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
> ARRAY_FIELD_EX32(bus->regs, I2CD_INTR_STS, SLAVE_ADDR_RX_MATCH) ?
> "slave-match|" : "",
> SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, NORMAL_STOP) ?
> - "normal|" : "",
> + "stop|" : "",
> SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, ABNORMAL) ?
> "abnormal" : "");
>
> --
> 2.35.3
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] aspeed/scu: Add trace events for read ops
2022-06-28 15:47 ` [PATCH 1/3] aspeed/scu: Add trace events for read ops Cédric Le Goater
@ 2022-06-28 16:14 ` Peter Delevoryas
0 siblings, 0 replies; 9+ messages in thread
From: Peter Delevoryas @ 2022-06-28 16:14 UTC (permalink / raw)
Cc: Peter Delevoryas, qemu-arm, Cameron Esfahani via, Peter Maydell,
Andrew Jeffery, Joel Stanley, Cédric Le Goater
> On Jun 28, 2022, at 8:47 AM, Cédric Le Goater <clg@kaod.org> wrote:
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Also a nice change.
Reviewed-by: Peter Delevoryas <pdel@fb.com>
> ---
> hw/misc/aspeed_scu.c | 2 ++
> hw/misc/trace-events | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 19b03471fc4e..83353649064a 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -270,6 +270,7 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
> break;
> }
>
> + trace_aspeed_scu_read(offset, size, s->regs[reg]);
> return s->regs[reg];
> }
>
> @@ -637,6 +638,7 @@ static uint64_t aspeed_ast2600_scu_read(void *opaque, hwaddr offset,
> break;
> }
>
> + trace_aspeed_scu_read(offset, size, s->regs[reg]);
> return s->regs[reg];
> }
>
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index c5e37b01547f..f776f24fb5d1 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -69,6 +69,7 @@ slavio_led_mem_readw(uint32_t ret) "Read diagnostic LED 0x%04x"
>
> # aspeed_scu.c
> aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32
> +aspeed_scu_read(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32
>
> # mps2-scc.c
> mps2_scc_read(uint64_t offset, uint64_t data, unsigned size) "MPS2 SCC read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
> --
> 2.35.3
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] aspeed: sbc: Allow per-machine settings
2022-06-28 15:47 ` [PATCH 3/3] aspeed: sbc: Allow per-machine settings Cédric Le Goater
@ 2022-07-01 1:36 ` Peter Delevoryas
2022-07-01 5:23 ` Cédric Le Goater
0 siblings, 1 reply; 9+ messages in thread
From: Peter Delevoryas @ 2022-07-01 1:36 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-arm, qemu-devel, Peter Maydell, Andrew Jeffery, Joel Stanley
On Tue, Jun 28, 2022 at 05:47:40PM +0200, Cédric Le Goater wrote:
> From: Joel Stanley <joel@jms.id.au>
>
> In order to correctly report secure boot running firmware the values
> of certain registers must be set.
>
> We don't yet have documentation from ASPEED on what they mean. The
> meaning is inferred from u-boot's use of them.
>
> Introduce properties so the settings can be configured per-machine.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> include/hw/misc/aspeed_sbc.h | 13 ++++++++++++
> hw/misc/aspeed_sbc.c | 41 ++++++++++++++++++++++++++++++++++--
> 2 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/misc/aspeed_sbc.h b/include/hw/misc/aspeed_sbc.h
> index 67e43b53ecc3..405e6782b97a 100644
> --- a/include/hw/misc/aspeed_sbc.h
> +++ b/include/hw/misc/aspeed_sbc.h
> @@ -17,9 +17,22 @@ OBJECT_DECLARE_TYPE(AspeedSBCState, AspeedSBCClass, ASPEED_SBC)
>
> #define ASPEED_SBC_NR_REGS (0x93c >> 2)
>
> +#define QSR_AES BIT(27)
> +#define QSR_RSA1024 (0x0 << 12)
> +#define QSR_RSA2048 (0x1 << 12)
> +#define QSR_RSA3072 (0x2 << 12)
> +#define QSR_RSA4096 (0x3 << 12)
> +#define QSR_SHA224 (0x0 << 10)
> +#define QSR_SHA256 (0x1 << 10)
> +#define QSR_SHA384 (0x2 << 10)
> +#define QSR_SHA512 (0x3 << 10)
> +
> struct AspeedSBCState {
> SysBusDevice parent;
>
> + bool emmc_abr;
> + uint32_t signing_settings;
> +
> MemoryRegion iomem;
>
> uint32_t regs[ASPEED_SBC_NR_REGS];
> diff --git a/hw/misc/aspeed_sbc.c b/hw/misc/aspeed_sbc.c
> index bfa8b81d01c7..3946e6179bdd 100644
> --- a/hw/misc/aspeed_sbc.c
> +++ b/hw/misc/aspeed_sbc.c
> @@ -11,6 +11,7 @@
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> #include "qemu/error-report.h"
> +#include "hw/qdev-properties.h"
> #include "hw/misc/aspeed_sbc.h"
> #include "qapi/error.h"
> #include "migration/vmstate.h"
> @@ -19,6 +20,27 @@
> #define R_STATUS (0x014 / 4)
> #define R_QSR (0x040 / 4)
>
> +/* R_STATUS */
> +#define ABR_EN BIT(14) /* Mirrors SCU510[11] */
> +#define ABR_IMAGE_SOURCE BIT(13)
> +#define SPI_ABR_IMAGE_SOURCE BIT(12)
> +#define SB_CRYPTO_KEY_EXP_DONE BIT(11)
> +#define SB_CRYPTO_BUSY BIT(10)
> +#define OTP_WP_EN BIT(9)
> +#define OTP_ADDR_WP_EN BIT(8)
> +#define LOW_SEC_KEY_EN BIT(7)
> +#define SECURE_BOOT_EN BIT(6)
> +#define UART_BOOT_EN BIT(5)
> +/* bit 4 reserved*/
> +#define OTP_CHARGE_PUMP_READY BIT(3)
> +#define OTP_IDLE BIT(2)
> +#define OTP_MEM_IDLE BIT(1)
> +#define OTP_COMPARE_STATUS BIT(0)
> +
> +/* QSR */
> +#define QSR_RSA_MASK (0x3 << 12)
> +#define QSR_HASH_MASK (0x3 << 10)
> +
> static uint64_t aspeed_sbc_read(void *opaque, hwaddr addr, unsigned int size)
> {
> AspeedSBCState *s = ASPEED_SBC(opaque);
> @@ -80,8 +102,17 @@ static void aspeed_sbc_reset(DeviceState *dev)
> memset(s->regs, 0, sizeof(s->regs));
>
> /* Set secure boot enabled with RSA4096_SHA256 and enable eMMC ABR */
> - s->regs[R_STATUS] = 0x000044C6;
> - s->regs[R_QSR] = 0x07C07C89;
> + s->regs[R_STATUS] = OTP_IDLE | OTP_MEM_IDLE;
> +
> + if (s->emmc_abr) {
> + s->regs[R_STATUS] &= ABR_EN;
> + }
> +
> + if (s->signing_settings) {
> + s->regs[R_STATUS] &= SECURE_BOOT_EN;
> + }
> +
> + s->regs[R_QSR] = s->signing_settings;
> }
>
> static void aspeed_sbc_realize(DeviceState *dev, Error **errp)
> @@ -105,6 +136,11 @@ static const VMStateDescription vmstate_aspeed_sbc = {
> }
> };
>
> +static Property aspeed_sbc_properties[] = {
> + DEFINE_PROP_BOOL("emmc-abr", AspeedSBCState, emmc_abr, 0),
> + DEFINE_PROP_UINT32("signing-settings", AspeedSBCState, signing_settings, 0),
> +};
This needs a DEFINE_PROP_END_OF_LIST(), I bisected to this commit in Cedric's
aspeed-7.1 branch.
Reviewed-by: Peter Delevoryas <pdel@fb.com>
Tested-by: Peter Delevoryas <pdel@fb.com>
> +
> static void aspeed_sbc_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -112,6 +148,7 @@ static void aspeed_sbc_class_init(ObjectClass *klass, void *data)
> dc->realize = aspeed_sbc_realize;
> dc->reset = aspeed_sbc_reset;
> dc->vmsd = &vmstate_aspeed_sbc;
> + device_class_set_props(dc, aspeed_sbc_properties);
> }
>
> static const TypeInfo aspeed_sbc_info = {
> --
> 2.35.3
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] aspeed: sbc: Allow per-machine settings
2022-07-01 1:36 ` Peter Delevoryas
@ 2022-07-01 5:23 ` Cédric Le Goater
2022-07-01 5:30 ` Peter Delevoryas
0 siblings, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2022-07-01 5:23 UTC (permalink / raw)
To: Peter Delevoryas
Cc: qemu-arm, qemu-devel, Peter Maydell, Andrew Jeffery, Joel Stanley
On 7/1/22 03:36, Peter Delevoryas wrote:
> On Tue, Jun 28, 2022 at 05:47:40PM +0200, Cédric Le Goater wrote:
>> From: Joel Stanley <joel@jms.id.au>
>>
>> In order to correctly report secure boot running firmware the values
>> of certain registers must be set.
>>
>> We don't yet have documentation from ASPEED on what they mean. The
>> meaning is inferred from u-boot's use of them.
>>
>> Introduce properties so the settings can be configured per-machine.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> include/hw/misc/aspeed_sbc.h | 13 ++++++++++++
>> hw/misc/aspeed_sbc.c | 41 ++++++++++++++++++++++++++++++++++--
>> 2 files changed, 52 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/misc/aspeed_sbc.h b/include/hw/misc/aspeed_sbc.h
>> index 67e43b53ecc3..405e6782b97a 100644
>> --- a/include/hw/misc/aspeed_sbc.h
>> +++ b/include/hw/misc/aspeed_sbc.h
>> @@ -17,9 +17,22 @@ OBJECT_DECLARE_TYPE(AspeedSBCState, AspeedSBCClass, ASPEED_SBC)
>>
>> #define ASPEED_SBC_NR_REGS (0x93c >> 2)
>>
>> +#define QSR_AES BIT(27)
>> +#define QSR_RSA1024 (0x0 << 12)
>> +#define QSR_RSA2048 (0x1 << 12)
>> +#define QSR_RSA3072 (0x2 << 12)
>> +#define QSR_RSA4096 (0x3 << 12)
>> +#define QSR_SHA224 (0x0 << 10)
>> +#define QSR_SHA256 (0x1 << 10)
>> +#define QSR_SHA384 (0x2 << 10)
>> +#define QSR_SHA512 (0x3 << 10)
>> +
>> struct AspeedSBCState {
>> SysBusDevice parent;
>>
>> + bool emmc_abr;
>> + uint32_t signing_settings;
>> +
>> MemoryRegion iomem;
>>
>> uint32_t regs[ASPEED_SBC_NR_REGS];
>> diff --git a/hw/misc/aspeed_sbc.c b/hw/misc/aspeed_sbc.c
>> index bfa8b81d01c7..3946e6179bdd 100644
>> --- a/hw/misc/aspeed_sbc.c
>> +++ b/hw/misc/aspeed_sbc.c
>> @@ -11,6 +11,7 @@
>> #include "qemu/osdep.h"
>> #include "qemu/log.h"
>> #include "qemu/error-report.h"
>> +#include "hw/qdev-properties.h"
>> #include "hw/misc/aspeed_sbc.h"
>> #include "qapi/error.h"
>> #include "migration/vmstate.h"
>> @@ -19,6 +20,27 @@
>> #define R_STATUS (0x014 / 4)
>> #define R_QSR (0x040 / 4)
>>
>> +/* R_STATUS */
>> +#define ABR_EN BIT(14) /* Mirrors SCU510[11] */
>> +#define ABR_IMAGE_SOURCE BIT(13)
>> +#define SPI_ABR_IMAGE_SOURCE BIT(12)
>> +#define SB_CRYPTO_KEY_EXP_DONE BIT(11)
>> +#define SB_CRYPTO_BUSY BIT(10)
>> +#define OTP_WP_EN BIT(9)
>> +#define OTP_ADDR_WP_EN BIT(8)
>> +#define LOW_SEC_KEY_EN BIT(7)
>> +#define SECURE_BOOT_EN BIT(6)
>> +#define UART_BOOT_EN BIT(5)
>> +/* bit 4 reserved*/
>> +#define OTP_CHARGE_PUMP_READY BIT(3)
>> +#define OTP_IDLE BIT(2)
>> +#define OTP_MEM_IDLE BIT(1)
>> +#define OTP_COMPARE_STATUS BIT(0)
>> +
>> +/* QSR */
>> +#define QSR_RSA_MASK (0x3 << 12)
>> +#define QSR_HASH_MASK (0x3 << 10)
>> +
>> static uint64_t aspeed_sbc_read(void *opaque, hwaddr addr, unsigned int size)
>> {
>> AspeedSBCState *s = ASPEED_SBC(opaque);
>> @@ -80,8 +102,17 @@ static void aspeed_sbc_reset(DeviceState *dev)
>> memset(s->regs, 0, sizeof(s->regs));
>>
>> /* Set secure boot enabled with RSA4096_SHA256 and enable eMMC ABR */
>> - s->regs[R_STATUS] = 0x000044C6;
>> - s->regs[R_QSR] = 0x07C07C89;
>> + s->regs[R_STATUS] = OTP_IDLE | OTP_MEM_IDLE;
>> +
>> + if (s->emmc_abr) {
>> + s->regs[R_STATUS] &= ABR_EN;
>> + }
>> +
>> + if (s->signing_settings) {
>> + s->regs[R_STATUS] &= SECURE_BOOT_EN;
>> + }
>> +
>> + s->regs[R_QSR] = s->signing_settings;
>> }
>>
>> static void aspeed_sbc_realize(DeviceState *dev, Error **errp)
>> @@ -105,6 +136,11 @@ static const VMStateDescription vmstate_aspeed_sbc = {
>> }
>> };
>>
>> +static Property aspeed_sbc_properties[] = {
>> + DEFINE_PROP_BOOL("emmc-abr", AspeedSBCState, emmc_abr, 0),
>> + DEFINE_PROP_UINT32("signing-settings", AspeedSBCState, signing_settings, 0),
>> +};
>
> This needs a DEFINE_PROP_END_OF_LIST(), I bisected to this commit in Cedric's
> aspeed-7.1 branch.
Ah you did also ! Sorry I should have told. The problem only showed
on f35 using clang, and I didn't notice until I pushed the branch
on gitlab yersterday.
> Reviewed-by: Peter Delevoryas <pdel@fb.com>
> Tested-by: Peter Delevoryas <pdel@fb.com>
I will include the patch in the next PR.
Thanks,
C.
>> +
>> static void aspeed_sbc_class_init(ObjectClass *klass, void *data)
>> {
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -112,6 +148,7 @@ static void aspeed_sbc_class_init(ObjectClass *klass, void *data)
>> dc->realize = aspeed_sbc_realize;
>> dc->reset = aspeed_sbc_reset;
>> dc->vmsd = &vmstate_aspeed_sbc;
>> + device_class_set_props(dc, aspeed_sbc_properties);
>> }
>>
>> static const TypeInfo aspeed_sbc_info = {
>> --
>> 2.35.3
>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] aspeed: sbc: Allow per-machine settings
2022-07-01 5:23 ` Cédric Le Goater
@ 2022-07-01 5:30 ` Peter Delevoryas
0 siblings, 0 replies; 9+ messages in thread
From: Peter Delevoryas @ 2022-07-01 5:30 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-arm, qemu-devel, Peter Maydell, Andrew Jeffery, Joel Stanley
On Fri, Jul 01, 2022 at 07:23:58AM +0200, Cédric Le Goater wrote:
> On 7/1/22 03:36, Peter Delevoryas wrote:
> > On Tue, Jun 28, 2022 at 05:47:40PM +0200, Cédric Le Goater wrote:
> > > From: Joel Stanley <joel@jms.id.au>
> > >
> > > In order to correctly report secure boot running firmware the values
> > > of certain registers must be set.
> > >
> > > We don't yet have documentation from ASPEED on what they mean. The
> > > meaning is inferred from u-boot's use of them.
> > >
> > > Introduce properties so the settings can be configured per-machine.
> > >
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > ---
> > > include/hw/misc/aspeed_sbc.h | 13 ++++++++++++
> > > hw/misc/aspeed_sbc.c | 41 ++++++++++++++++++++++++++++++++++--
> > > 2 files changed, 52 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/hw/misc/aspeed_sbc.h b/include/hw/misc/aspeed_sbc.h
> > > index 67e43b53ecc3..405e6782b97a 100644
> > > --- a/include/hw/misc/aspeed_sbc.h
> > > +++ b/include/hw/misc/aspeed_sbc.h
> > > @@ -17,9 +17,22 @@ OBJECT_DECLARE_TYPE(AspeedSBCState, AspeedSBCClass, ASPEED_SBC)
> > > #define ASPEED_SBC_NR_REGS (0x93c >> 2)
> > > +#define QSR_AES BIT(27)
> > > +#define QSR_RSA1024 (0x0 << 12)
> > > +#define QSR_RSA2048 (0x1 << 12)
> > > +#define QSR_RSA3072 (0x2 << 12)
> > > +#define QSR_RSA4096 (0x3 << 12)
> > > +#define QSR_SHA224 (0x0 << 10)
> > > +#define QSR_SHA256 (0x1 << 10)
> > > +#define QSR_SHA384 (0x2 << 10)
> > > +#define QSR_SHA512 (0x3 << 10)
> > > +
> > > struct AspeedSBCState {
> > > SysBusDevice parent;
> > > + bool emmc_abr;
> > > + uint32_t signing_settings;
> > > +
> > > MemoryRegion iomem;
> > > uint32_t regs[ASPEED_SBC_NR_REGS];
> > > diff --git a/hw/misc/aspeed_sbc.c b/hw/misc/aspeed_sbc.c
> > > index bfa8b81d01c7..3946e6179bdd 100644
> > > --- a/hw/misc/aspeed_sbc.c
> > > +++ b/hw/misc/aspeed_sbc.c
> > > @@ -11,6 +11,7 @@
> > > #include "qemu/osdep.h"
> > > #include "qemu/log.h"
> > > #include "qemu/error-report.h"
> > > +#include "hw/qdev-properties.h"
> > > #include "hw/misc/aspeed_sbc.h"
> > > #include "qapi/error.h"
> > > #include "migration/vmstate.h"
> > > @@ -19,6 +20,27 @@
> > > #define R_STATUS (0x014 / 4)
> > > #define R_QSR (0x040 / 4)
> > > +/* R_STATUS */
> > > +#define ABR_EN BIT(14) /* Mirrors SCU510[11] */
> > > +#define ABR_IMAGE_SOURCE BIT(13)
> > > +#define SPI_ABR_IMAGE_SOURCE BIT(12)
> > > +#define SB_CRYPTO_KEY_EXP_DONE BIT(11)
> > > +#define SB_CRYPTO_BUSY BIT(10)
> > > +#define OTP_WP_EN BIT(9)
> > > +#define OTP_ADDR_WP_EN BIT(8)
> > > +#define LOW_SEC_KEY_EN BIT(7)
> > > +#define SECURE_BOOT_EN BIT(6)
> > > +#define UART_BOOT_EN BIT(5)
> > > +/* bit 4 reserved*/
> > > +#define OTP_CHARGE_PUMP_READY BIT(3)
> > > +#define OTP_IDLE BIT(2)
> > > +#define OTP_MEM_IDLE BIT(1)
> > > +#define OTP_COMPARE_STATUS BIT(0)
> > > +
> > > +/* QSR */
> > > +#define QSR_RSA_MASK (0x3 << 12)
> > > +#define QSR_HASH_MASK (0x3 << 10)
> > > +
> > > static uint64_t aspeed_sbc_read(void *opaque, hwaddr addr, unsigned int size)
> > > {
> > > AspeedSBCState *s = ASPEED_SBC(opaque);
> > > @@ -80,8 +102,17 @@ static void aspeed_sbc_reset(DeviceState *dev)
> > > memset(s->regs, 0, sizeof(s->regs));
> > > /* Set secure boot enabled with RSA4096_SHA256 and enable eMMC ABR */
> > > - s->regs[R_STATUS] = 0x000044C6;
> > > - s->regs[R_QSR] = 0x07C07C89;
> > > + s->regs[R_STATUS] = OTP_IDLE | OTP_MEM_IDLE;
> > > +
> > > + if (s->emmc_abr) {
> > > + s->regs[R_STATUS] &= ABR_EN;
> > > + }
> > > +
> > > + if (s->signing_settings) {
> > > + s->regs[R_STATUS] &= SECURE_BOOT_EN;
> > > + }
> > > +
> > > + s->regs[R_QSR] = s->signing_settings;
> > > }
> > > static void aspeed_sbc_realize(DeviceState *dev, Error **errp)
> > > @@ -105,6 +136,11 @@ static const VMStateDescription vmstate_aspeed_sbc = {
> > > }
> > > };
> > > +static Property aspeed_sbc_properties[] = {
> > > + DEFINE_PROP_BOOL("emmc-abr", AspeedSBCState, emmc_abr, 0),
> > > + DEFINE_PROP_UINT32("signing-settings", AspeedSBCState, signing_settings, 0),
> > > +};
> >
> > This needs a DEFINE_PROP_END_OF_LIST(), I bisected to this commit in Cedric's
> > aspeed-7.1 branch.
>
> Ah you did also ! Sorry I should have told. The problem only showed
> on f35 using clang, and I didn't notice until I pushed the branch
> on gitlab yersterday.
Oh glad you noticed too, it's no problem.
>
> > Reviewed-by: Peter Delevoryas <pdel@fb.com>
> > Tested-by: Peter Delevoryas <pdel@fb.com>
>
> I will include the patch in the next PR.
That's great, thanks!
>
> Thanks,
>
> C.
>
>
> > > +
> > > static void aspeed_sbc_class_init(ObjectClass *klass, void *data)
> > > {
> > > DeviceClass *dc = DEVICE_CLASS(klass);
> > > @@ -112,6 +148,7 @@ static void aspeed_sbc_class_init(ObjectClass *klass, void *data)
> > > dc->realize = aspeed_sbc_realize;
> > > dc->reset = aspeed_sbc_reset;
> > > dc->vmsd = &vmstate_aspeed_sbc;
> > > + device_class_set_props(dc, aspeed_sbc_properties);
> > > }
> > > static const TypeInfo aspeed_sbc_info = {
> > > --
> > > 2.35.3
> > >
> > >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-07-01 5:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-28 15:47 [PATCH 0/3] aspeed: small cleanups Cédric Le Goater
2022-06-28 15:47 ` [PATCH 1/3] aspeed/scu: Add trace events for read ops Cédric Le Goater
2022-06-28 16:14 ` Peter Delevoryas
2022-06-28 15:47 ` [PATCH 2/3] aspeed/i2c: Change trace event for NORMAL_STOP states Cédric Le Goater
2022-06-28 16:13 ` Peter Delevoryas
2022-06-28 15:47 ` [PATCH 3/3] aspeed: sbc: Allow per-machine settings Cédric Le Goater
2022-07-01 1:36 ` Peter Delevoryas
2022-07-01 5:23 ` Cédric Le Goater
2022-07-01 5:30 ` Peter Delevoryas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.