Message ID | 20220628154740.1117349-4-clg@kaod.org |
---|---|
State | New |
Headers | show |
Series | aspeed: small cleanups | expand |
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 > >
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 >> >>
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 > > > > > > >
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 = {