Hi Nolan, Thanks for your patch!
There is something odd with your email address, which apparently became [email protected] instead of [email protected]. On 5/18/21 10:24 PM, ~nolanl wrote: > From: Nolan Leake <[email protected]> > > This is just enough to make reboot and poweroff work. Please precise "for Linux kernels", since this doesn't work with firmwares (ideally we should implement the FIRMWARE_NOTIFY_REBOOT property - which Linux sends - to handle the machine reboot/halt via the videocore firmware model). > Notably, the > watchdog timer functionality is not yet implemented. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/64 > Signed-off-by: Nolan Leake <[email protected]> > --- > hw/arm/bcm2835_peripherals.c | 13 ++- > hw/misc/bcm2835_powermgt.c | 157 +++++++++++++++++++++++++++ > hw/misc/meson.build | 1 + > include/hw/arm/bcm2835_peripherals.h | 3 +- > include/hw/misc/bcm2835_powermgt.h | 29 +++++ > 5 files changed, 201 insertions(+), 2 deletions(-) > create mode 100644 hw/misc/bcm2835_powermgt.c > create mode 100644 include/hw/misc/bcm2835_powermgt.h > > diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c > index dcff13433e..48538c9360 100644 > --- a/hw/arm/bcm2835_peripherals.c > +++ b/hw/arm/bcm2835_peripherals.c > @@ -126,6 +126,10 @@ static void bcm2835_peripherals_init(Object *obj) > > object_property_add_const_link(OBJECT(&s->dwc2), "dma-mr", > OBJECT(&s->gpu_bus_mr)); > + > + /* Power Management */ > + object_initialize_child(obj, "powermgt", &s->powermgt, > + TYPE_BCM2835_POWERMGT); > } > > static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp) > @@ -364,9 +368,16 @@ static void bcm2835_peripherals_realize(DeviceState > *dev, Error **errp) > qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ, > INTERRUPT_USB)); > > + /* Power Management */ > + if (!sysbus_realize(SYS_BUS_DEVICE(&s->powermgt), errp)) { > + return; > + } > + > + memory_region_add_subregion(&s->peri_mr, PM_OFFSET, > + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->powermgt), 0)); > + > create_unimp(s, &s->txp, "bcm2835-txp", TXP_OFFSET, 0x1000); > create_unimp(s, &s->armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, > 0x40); > - create_unimp(s, &s->powermgt, "bcm2835-powermgt", PM_OFFSET, 0x114); > create_unimp(s, &s->i2s, "bcm2835-i2s", I2S_OFFSET, 0x100); > create_unimp(s, &s->smi, "bcm2835-smi", SMI_OFFSET, 0x100); > create_unimp(s, &s->spi[0], "bcm2835-spi0", SPI0_OFFSET, 0x20); > diff --git a/hw/misc/bcm2835_powermgt.c b/hw/misc/bcm2835_powermgt.c > new file mode 100644 > index 0000000000..81107ecc8f > --- /dev/null > +++ b/hw/misc/bcm2835_powermgt.c > @@ -0,0 +1,157 @@ > +/* > + * BCM2835 Power Management emulation > + * > + * Copyright (C) 2017 Marcin Chojnacki <[email protected]> > + * Copyright (C) 2021 Nolan Leake <[email protected]> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/log.h" > +#include "qemu/module.h" > +#include "hw/misc/bcm2835_powermgt.h" > +#include "migration/vmstate.h" > +#include "sysemu/runstate.h" > + > +#define PASSWORD 0x5a000000 > +#define PASSWORD_MASK 0xff000000 > + > +#define R_RSTC 0x1c > +#define V_RSTC_RESET 0x20 > +#define R_RSTS 0x20 > +#define V_RSTS_POWEROFF 0x555 > +#define R_WDOG 0x24 > + > +static uint64_t bcm2835_powermgt_read(void *opaque, hwaddr offset, > + unsigned size) > +{ > + BCM2835PowerMgtState *s = (BCM2835PowerMgtState *)opaque; > + uint32_t res = 0; > + > + assert(size == 4); Instead of this assert, add in bcm2835_powermgt_ops: .impl.min_access_size = 4, .impl.max_access_size = 4, > + > + switch (offset) { > + case R_RSTC: > + res = s->rstc; > + break; > + case R_RSTS: > + res = s->rsts; > + break; > + case R_WDOG: > + res = s->wdog; > + break; > + > + default: > + qemu_log_mask(LOG_UNIMP, > + "bcm2835_powermgt_read: Unknown offset %x\n", > + (int)offset); > + res = 0; > + break; > + } > + > + return res; > +} > + > +static void bcm2835_powermgt_write(void *opaque, hwaddr offset, > + uint64_t value, unsigned size) > +{ > + BCM2835PowerMgtState *s = (BCM2835PowerMgtState *)opaque; > + > + assert(size == 4); (remove this assert too). > + if ((value & PASSWORD_MASK) != PASSWORD) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "bcm2835_powermgt_write: Bad password %x at offset > %x\n", Please prefix hexadecimal formats with 0x, > + (uint32_t)value, (int)offset); and do not cast. > + return; > + } > + > + value = value & ~PASSWORD_MASK; > + > + switch (offset) { > + case R_RSTC: > + s->rstc = value; > + if (value & V_RSTC_RESET) { > + if ((s->rsts & 0xfff) == V_RSTS_POWEROFF) { > + /* Linux uses partition 63 to indicate halt. */ I'd move this comment with the V_RSTS_POWEROFF definition. > + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > + } else { > + qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET); > + } > + } Shouldn't we log the unsupported bits? > + break; > + case R_RSTS: > + s->rsts = value; > + break; > + case R_WDOG: > + s->wdog = value; > + break; > + > + default: > + qemu_log_mask(LOG_UNIMP, > + "bcm2835_powermgt_write: Unknown offset %x\n", > + (int)offset); Please do not cast, use the proper format. > + break; > + } > +} > + > +static const MemoryRegionOps bcm2835_powermgt_ops = { > + .read = bcm2835_powermgt_read, > + .write = bcm2835_powermgt_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static const VMStateDescription vmstate_bcm2835_powermgt = { > + .name = TYPE_BCM2835_POWERMGT, > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(rstc, BCM2835PowerMgtState), > + VMSTATE_UINT32(rsts, BCM2835PowerMgtState), > + VMSTATE_UINT32(wdog, BCM2835PowerMgtState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void bcm2835_powermgt_init(Object *obj) > +{ > + BCM2835PowerMgtState *s = BCM2835_POWERMGT(obj); > + > + memory_region_init_io(&s->iomem, obj, &bcm2835_powermgt_ops, s, > + TYPE_BCM2835_POWERMGT, 0x114); > + sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem); > +} > + > +static void bcm2835_powermgt_reset(DeviceState *dev) > +{ > + BCM2835PowerMgtState *s = BCM2835_POWERMGT(dev); > + > + s->rstc = 0x00000102; > + s->rsts = 0x00001000; > + s->wdog = 0x00000000; Where these bits come from? > +} > + > +static void bcm2835_powermgt_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->reset = bcm2835_powermgt_reset; > + dc->vmsd = &vmstate_bcm2835_powermgt; > +} > + > +static TypeInfo bcm2835_powermgt_info = { > + .name = TYPE_BCM2835_POWERMGT, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(BCM2835PowerMgtState), > + .class_init = bcm2835_powermgt_class_init, > + .instance_init = bcm2835_powermgt_init, > +}; Minor comments, otherwise: Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Tested-by: Philippe Mathieu-Daudé <[email protected]>
