Re: [PATCH 3/3] aspeed: sbc: Allow per-machine settings

[Peter Delevoryas]

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
> 
>