> -----Original Message-----
> From: Kane Chen <[email protected]>
> Sent: Monday, August 31, 2026 1:37 PM
> To: Cédric Le Goater <[email protected]>; Peter Maydell
> <[email protected]>; Steven Lee <[email protected]>; Troy
> Lee <[email protected]>; Jamin Lin <[email protected]>; Andrew
> Jeffery <[email protected]>; Joel Stanley <[email protected]>; open
> list:ASPEED BMCs <[email protected]>; open list:All patches CC here
> <[email protected]>
> Cc: Troy Lee <[email protected]>; Kane Chen
> <[email protected]>
> Subject: [PATCH v1 2/2] hw/misc/aspeed_sbc: Derive secure boot state from
> OTP config straps
> 
> The secure boot enable bit in R_STATUS and the R_QSR signing settings were
> both driven by a fixed "signing-settings" machine property, but the two ended
> up inconsistent: the logic guarding the R_STATUS enable bit was broken and
> never actually set it, while R_QSR still reported the configured signing 
> settings,
> so a machine could show secure boot configured in R_QSR while R_STATUS
> said it was disabled.
> 
> Instead of relying on the property, read the relevant OTP configuration bits
> directly and derive both the R_STATUS secure boot enable state and the QSR
> value from them, matching how the real hardware determines these settings.
> The now-unused "signing-settings"
> property is removed.
> 
> Signed-off-by: Kane-Chen-AS <[email protected]>
> ---
>  include/hw/misc/aspeed_sbc.h |  2 --
>  hw/misc/aspeed_sbc.c         | 34 ++++++++++++++++++++++++----------
>  2 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/misc/aspeed_sbc.h b/include/hw/misc/aspeed_sbc.h
> index 7152497b2a..474922cbd2 100644
> --- a/include/hw/misc/aspeed_sbc.h
> +++ b/include/hw/misc/aspeed_sbc.h
> @@ -32,8 +32,6 @@ OBJECT_DECLARE_TYPE(AspeedSBCState,
> AspeedSBCClass, ASPEED_SBC)  struct AspeedSBCState {
>      SysBusDevice parent;
> 
> -    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
> 5d4da39d30..ce03f717a9 100644
> --- a/hw/misc/aspeed_sbc.c
> +++ b/hw/misc/aspeed_sbc.c
> @@ -63,6 +63,19 @@
>  /* OTP Address */
>  #define OTP_CFG0                (0x800)
> 
> +static bool aspeed_sbc_otp_read(AspeedSBCState *s, uint32_t otp_addr);
> +
> +static uint32_t aspeed_otp_read_cfg0(AspeedSBCState *s) {
> +    uint32_t value = 0;
> +
> +    if (aspeed_sbc_otp_read(s, OTP_CFG0)) {
> +        value = s->regs[R_CAMP1];
> +    }
> +
> +    return value;
> +}
> +
>  static uint64_t aspeed_sbc_read(void *opaque, hwaddr addr, unsigned int size)
> {
>      AspeedSBCState *s = ASPEED_SBC(opaque); @@ -76,7 +89,12 @@ static
> uint64_t aspeed_sbc_read(void *opaque, hwaddr addr, unsigned int size)
>          return 0;
>      }
> 
> -    return s->regs[addr];
> +    switch (addr) {
> +    case R_QSR:
> +        return aspeed_otp_read_cfg0(s);
> +    default:
> +        return s->regs[addr];
> +    }
>  }
> 
>  static bool aspeed_sbc_otp_read(AspeedSBCState *s, @@ -291,6 +309,7 @@
> static bool aspeed_get_abr_state(AspeedSBCState *s)  static void
> aspeed_sbc_reset_hold(Object *obj, ResetType type)  {
>      AspeedSBCState *s = ASPEED_SBC(obj);
> +    uint32_t value;
>      bool abr;
> 
>      memset(s->regs, 0, sizeof(s->regs)); @@ -304,11 +323,11 @@ static void
> aspeed_sbc_reset_hold(Object *obj, ResetType type)
>          s->regs[R_STATUS] |= ABR_EN;
>      }
> 
> -    if (s->signing_settings) {
> -        s->regs[R_STATUS] &= SECURE_BOOT_EN;
> -    }
> +    value = aspeed_otp_read_cfg0(s);
> 
> -    s->regs[R_QSR] = s->signing_settings;
> +    if (value & BIT(1)) {
> +        s->regs[R_STATUS] |= SECURE_BOOT_EN;
> +    }
>  }
> 
>  static void aspeed_sbc_instance_init(Object *obj) @@ -352,10 +371,6 @@
> static const VMStateDescription vmstate_aspeed_sbc = {
>      }
>  };
> 
> -static const Property aspeed_sbc_properties[] = {
> -    DEFINE_PROP_UINT32("signing-settings", AspeedSBCState,
> signing_settings, 0),
> -};
> -
>  static void aspeed_sbc_class_init(ObjectClass *klass, const void *data)  {
>      DeviceClass *dc = DEVICE_CLASS(klass); @@ -364,7 +379,6 @@ static
> void aspeed_sbc_class_init(ObjectClass *klass, const void *data)
>      dc->realize = aspeed_sbc_realize;
>      rc->phases.hold = aspeed_sbc_reset_hold;
>      dc->vmsd = &vmstate_aspeed_sbc;
> -    device_class_set_props(dc, aspeed_sbc_properties);
>  }
> 
> 
> --
> 2.43.0

Reviewed-by: Jamin Lin <[email protected]>

Reply via email to