Re: [Qemu-devel] [PATCH] sun4u: implement power device

[Philippe Mathieu-Daudé]

CC'ing PCI maintainers.

Hi Mark,

On 01/15/2018 05:58 PM, Mark Cave-Ayland wrote:
> This inbuilt device contains a single 4-byte register, of which bit 24 is used
> to power down the machine on a real Ultra 5.
> 
> The power device exists at offset 0x724000 on a real machine, but due to the
> current configuration of the BARs in QEMU it must be located lower in PCI IO
> space.
Is is some issue in pci_bar_address()?

> 
> For the moment we place the power device at offset 0x7240 as a reminder of its
> original location and raise the base PCI IO address from 0x4000 to 0x8000.

If we can't fix it, I rather prefer a #define with a comment in the
code, IMHO this is safer and easier to remember than looking in git history.

> 
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> ---
>  hw/sparc64/sun4u.c | 64 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index ec45ec2801..26ab6e9a9c 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -205,6 +205,59 @@ typedef struct ResetData {
>      uint64_t prom_addr;
>  } ResetData;
>  
> +#define TYPE_SUN4U_POWER "power"
> +#define SUN4U_POWER(obj) OBJECT_CHECK(PowerDevice, (obj), TYPE_SUN4U_POWER)
> +
> +typedef struct PowerDevice {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion power_mmio;
> +} PowerDevice;
> +
> +/* Power */
> +static void power_mem_write(void *opaque, hwaddr addr,
> +                            uint64_t val, unsigned size)
> +{
> +    /* According to a real Ultra 5, bit 24 controls the power */
> +    if (val & 0x1000000) {
> +        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +    }
> +}
> +
> +static const MemoryRegionOps power_mem_ops = {
> +    .write = power_mem_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static void power_realize(DeviceState *dev, Error **errp)
> +{
> +    PowerDevice *d = SUN4U_POWER(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    memory_region_init_io(&d->power_mmio, OBJECT(dev), &power_mem_ops, d,
> +                          "power", sizeof(uint32_t));
> +
> +    sysbus_init_mmio(sbd, &d->power_mmio);
> +}
> +
> +static void power_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = power_realize;
> +}
> +
> +static const TypeInfo power_info = {
> +    .name          = TYPE_SUN4U_POWER,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(PowerDevice),
> +    .class_init    = power_class_init,
> +};
> +
>  static void ebus_isa_irq_handler(void *opaque, int n, int level)
>  {
>      EbusState *s = EBUS(opaque);
> @@ -221,6 +274,7 @@ static void ebus_isa_irq_handler(void *opaque, int n, int 
> level)
>  static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      EbusState *s = EBUS(pci_dev);
> +    SysBusDevice *sbd;
>      DeviceState *dev;
>      qemu_irq *isa_irq;
>      DriveInfo *fd[MAX_FD];
> @@ -270,6 +324,13 @@ static void ebus_realize(PCIDevice *pci_dev, Error 
> **errp)
>      qdev_prop_set_uint32(dev, "dma", -1);
>      qdev_init_nofail(dev);
>  
> +    /* Power */
> +    dev = qdev_create(NULL, TYPE_SUN4U_POWER);
> +    qdev_init_nofail(dev);
> +    sbd = SYS_BUS_DEVICE(dev);
> +    memory_region_add_subregion(pci_address_space_io(pci_dev), 0x7240,
> +                                sysbus_mmio_get_region(sbd, 0));
> +
>      /* PCI */
>      pci_dev->config[0x04] = 0x06; // command = bus master, pci mem
>      pci_dev->config[0x05] = 0x00;
> @@ -282,7 +343,7 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>                               0, 0x1000000);
>      pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar0);
>      memory_region_init_alias(&s->bar1, OBJECT(s), "bar1", get_system_io(),
> -                             0, 0x4000);
> +                             0, 0x8000);

What about using 2 MR, bar1_lo/bar1_hi, registering bar1_lo at offset @0
and bar1_hi at @0x724000?

>      pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar1);
>  }
>  
> @@ -693,6 +754,7 @@ static const TypeInfo sun4v_type = {
>  
>  static void sun4u_register_types(void)
>  {
> +    type_register_static(&power_info);
>      type_register_static(&ebus_info);
>      type_register_static(&prom_info);
>      type_register_static(&ram_info);
>

signature.asc
Description: OpenPGP digital signature