| Message ID | 20171114122018.12204-1-joel@jms.id.au |
|---|---|
| State | Accepted |
| Commit | b6e70d1d7fea681306a3c8e74934b66dc9524969 |
| Headers | show |
| Series | [v3] hw/arm/aspeed: Unlock SCU when running kernel | expand |
On 11/14/2017 01:20 PM, Joel Stanley wrote: > The ASPEED hardware contains a lock register for the SCU that disables > any writes to the SCU when it is locked. The machine comes up with the > lock enabled, but on all known hardware u-boot will unlock it and leave > it unlocked when loading the kernel. > > This means the kernel expects the SCU to be unlocked. When booting from > an emulated ROM the normal u-boot unlock path is executed. Things don't > go well when booting using the -kernel command line, as u-boot does not > run first. > > Change behaviour so that when a kernel is passed to the machine, set the > reset value of the SCU to be unlocked. > > Signed-off-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > v3: > - remove unused member from AspeedBoardConfig > v2: > - use the correct value for the key > - rename it ASPEED_SCU_PROT_KEY > --- > hw/arm/aspeed.c | 9 +++++++++ > hw/arm/aspeed_soc.c | 2 ++ > hw/misc/aspeed_scu.c | 5 +++-- > include/hw/misc/aspeed_scu.h | 3 +++ > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index ab895ad490af..7088c907bd23 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -186,6 +186,15 @@ static void aspeed_board_init(MachineState *machine, > &error_abort); > object_property_set_int(OBJECT(&bmc->soc), cfg->num_cs, "num-cs", > &error_abort); > + if (machine->kernel_filename) { > + /* > + * When booting with a -kernel command line there is no u-boot > + * that runs to unlock the SCU. In this case set the default to > + * be unlocked as the kernel expects > + */ > + object_property_set_int(OBJECT(&bmc->soc), ASPEED_SCU_PROT_KEY, > + "hw-prot-key", &error_abort); > + } > object_property_set_bool(OBJECT(&bmc->soc), true, "realized", > &error_abort); > > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c > index 5aa3d2ddd9cd..c83b7e207b86 100644 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -154,6 +154,8 @@ static void aspeed_soc_init(Object *obj) > "hw-strap1", &error_abort); > object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu), > "hw-strap2", &error_abort); > + object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu), > + "hw-prot-key", &error_abort); > > object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename); > object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL); > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index 95022d3607ad..74537ce9755d 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -85,7 +85,6 @@ > #define BMC_REV TO_REG(0x19C) > #define BMC_DEV_ID TO_REG(0x1A4) > > -#define PROT_KEY_UNLOCK 0x1688A8A8 > #define SCU_IO_REGION_SIZE 0x1000 > > static const uint32_t ast2400_a0_resets[ASPEED_SCU_NR_REGS] = { > @@ -192,7 +191,7 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data, > } > > if (reg > PROT_KEY && reg < CPU2_BASE_SEG1 && > - s->regs[PROT_KEY] != PROT_KEY_UNLOCK) { > + s->regs[PROT_KEY] != ASPEED_SCU_PROT_KEY) { > qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__); > return; > } > @@ -246,6 +245,7 @@ static void aspeed_scu_reset(DeviceState *dev) > s->regs[SILICON_REV] = s->silicon_rev; > s->regs[HW_STRAP1] = s->hw_strap1; > s->regs[HW_STRAP2] = s->hw_strap2; > + s->regs[PROT_KEY] = s->hw_prot_key; > } > > static uint32_t aspeed_silicon_revs[] = { > @@ -299,6 +299,7 @@ static Property aspeed_scu_properties[] = { > DEFINE_PROP_UINT32("silicon-rev", AspeedSCUState, silicon_rev, 0), > DEFINE_PROP_UINT32("hw-strap1", AspeedSCUState, hw_strap1, 0), > DEFINE_PROP_UINT32("hw-strap2", AspeedSCUState, hw_strap2, 0), > + DEFINE_PROP_UINT32("hw-prot-key", AspeedSCUState, hw_prot_key, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h > index bd4ac013f997..d70cc0aeca61 100644 > --- a/include/hw/misc/aspeed_scu.h > +++ b/include/hw/misc/aspeed_scu.h > @@ -29,6 +29,7 @@ typedef struct AspeedSCUState { > uint32_t silicon_rev; > uint32_t hw_strap1; > uint32_t hw_strap2; > + uint32_t hw_prot_key; > } AspeedSCUState; > > #define AST2400_A0_SILICON_REV 0x02000303U > @@ -38,6 +39,8 @@ typedef struct AspeedSCUState { > > extern bool is_supported_silicon_rev(uint32_t silicon_rev); > > +#define ASPEED_SCU_PROT_KEY 0x1688A8A8 > + > /* > * Extracted from Aspeed SDK v00.03.21. Fixes and extra definitions > * were added. >
On 15 November 2017 at 08:18, Cédric Le Goater <clg@kaod.org> wrote: > On 11/14/2017 01:20 PM, Joel Stanley wrote: >> The ASPEED hardware contains a lock register for the SCU that disables >> any writes to the SCU when it is locked. The machine comes up with the >> lock enabled, but on all known hardware u-boot will unlock it and leave >> it unlocked when loading the kernel. >> >> This means the kernel expects the SCU to be unlocked. When booting from >> an emulated ROM the normal u-boot unlock path is executed. Things don't >> go well when booting using the -kernel command line, as u-boot does not >> run first. >> >> Change behaviour so that when a kernel is passed to the machine, set the >> reset value of the SCU to be unlocked. >> >> Signed-off-by: Joel Stanley <joel@jms.id.au> > > Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks. Should this go into 2.11 ? It sounds like the kind of bugfix that ought to... -- PMM
On 11/15/2017 11:56 AM, Peter Maydell wrote: > On 15 November 2017 at 08:18, Cédric Le Goater <clg@kaod.org> wrote: >> On 11/14/2017 01:20 PM, Joel Stanley wrote: >>> The ASPEED hardware contains a lock register for the SCU that disables >>> any writes to the SCU when it is locked. The machine comes up with the >>> lock enabled, but on all known hardware u-boot will unlock it and leave >>> it unlocked when loading the kernel. >>> >>> This means the kernel expects the SCU to be unlocked. When booting from >>> an emulated ROM the normal u-boot unlock path is executed. Things don't >>> go well when booting using the -kernel command line, as u-boot does not >>> run first. >>> >>> Change behaviour so that when a kernel is passed to the machine, set the >>> reset value of the SCU to be unlocked. >>> >>> Signed-off-by: Joel Stanley <joel@jms.id.au> >> >> Reviewed-by: Cédric Le Goater <clg@kaod.org> > > Thanks. Should this go into 2.11 ? It sounds like the kind of > bugfix that ought to... Yes. Please. Thanks, C.
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index ab895ad490af..7088c907bd23 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -186,6 +186,15 @@ static void aspeed_board_init(MachineState *machine, &error_abort); object_property_set_int(OBJECT(&bmc->soc), cfg->num_cs, "num-cs", &error_abort); + if (machine->kernel_filename) { + /* + * When booting with a -kernel command line there is no u-boot + * that runs to unlock the SCU. In this case set the default to + * be unlocked as the kernel expects + */ + object_property_set_int(OBJECT(&bmc->soc), ASPEED_SCU_PROT_KEY, + "hw-prot-key", &error_abort); + } object_property_set_bool(OBJECT(&bmc->soc), true, "realized", &error_abort); diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index 5aa3d2ddd9cd..c83b7e207b86 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -154,6 +154,8 @@ static void aspeed_soc_init(Object *obj) "hw-strap1", &error_abort); object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu), "hw-strap2", &error_abort); + object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu), + "hw-prot-key", &error_abort); object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename); object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL); diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index 95022d3607ad..74537ce9755d 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -85,7 +85,6 @@ #define BMC_REV TO_REG(0x19C) #define BMC_DEV_ID TO_REG(0x1A4) -#define PROT_KEY_UNLOCK 0x1688A8A8 #define SCU_IO_REGION_SIZE 0x1000 static const uint32_t ast2400_a0_resets[ASPEED_SCU_NR_REGS] = { @@ -192,7 +191,7 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data, } if (reg > PROT_KEY && reg < CPU2_BASE_SEG1 && - s->regs[PROT_KEY] != PROT_KEY_UNLOCK) { + s->regs[PROT_KEY] != ASPEED_SCU_PROT_KEY) { qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__); return; } @@ -246,6 +245,7 @@ static void aspeed_scu_reset(DeviceState *dev) s->regs[SILICON_REV] = s->silicon_rev; s->regs[HW_STRAP1] = s->hw_strap1; s->regs[HW_STRAP2] = s->hw_strap2; + s->regs[PROT_KEY] = s->hw_prot_key; } static uint32_t aspeed_silicon_revs[] = { @@ -299,6 +299,7 @@ static Property aspeed_scu_properties[] = { DEFINE_PROP_UINT32("silicon-rev", AspeedSCUState, silicon_rev, 0), DEFINE_PROP_UINT32("hw-strap1", AspeedSCUState, hw_strap1, 0), DEFINE_PROP_UINT32("hw-strap2", AspeedSCUState, hw_strap2, 0), + DEFINE_PROP_UINT32("hw-prot-key", AspeedSCUState, hw_prot_key, 0), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h index bd4ac013f997..d70cc0aeca61 100644 --- a/include/hw/misc/aspeed_scu.h +++ b/include/hw/misc/aspeed_scu.h @@ -29,6 +29,7 @@ typedef struct AspeedSCUState { uint32_t silicon_rev; uint32_t hw_strap1; uint32_t hw_strap2; + uint32_t hw_prot_key; } AspeedSCUState; #define AST2400_A0_SILICON_REV 0x02000303U @@ -38,6 +39,8 @@ typedef struct AspeedSCUState { extern bool is_supported_silicon_rev(uint32_t silicon_rev); +#define ASPEED_SCU_PROT_KEY 0x1688A8A8 + /* * Extracted from Aspeed SDK v00.03.21. Fixes and extra definitions * were added.
The ASPEED hardware contains a lock register for the SCU that disables any writes to the SCU when it is locked. The machine comes up with the lock enabled, but on all known hardware u-boot will unlock it and leave it unlocked when loading the kernel. This means the kernel expects the SCU to be unlocked. When booting from an emulated ROM the normal u-boot unlock path is executed. Things don't go well when booting using the -kernel command line, as u-boot does not run first. Change behaviour so that when a kernel is passed to the machine, set the reset value of the SCU to be unlocked. Signed-off-by: Joel Stanley <joel@jms.id.au> --- v3: - remove unused member from AspeedBoardConfig v2: - use the correct value for the key - rename it ASPEED_SCU_PROT_KEY --- hw/arm/aspeed.c | 9 +++++++++ hw/arm/aspeed_soc.c | 2 ++ hw/misc/aspeed_scu.c | 5 +++-- include/hw/misc/aspeed_scu.h | 3 +++ 4 files changed, 17 insertions(+), 2 deletions(-) -- 2.14.1