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 <j...@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 <j...@jms.id.au>
> > > Signed-off-by: Cédric Le Goater <c...@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 <p...@fb.com>
> > Tested-by: Peter Delevoryas <p...@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
> > >
> > >
>
