Re: [Qemu-devel] [PATCH 1/5] Platform: Add platform device class

[Eric Auger]

On 06/04/2014 02:28 PM, Alexander Graf wrote:
> This patch adds a new device class called "platform device". This is an
> abstract class for consumption of actual classes that implement devices.
> 
> The new thing about platform devices is that they have awareness of all
> memory regions and IRQs that the device exposes. That gives us the ability
> to manually specify thing using properties from the command line.

Hi Alex,

thanks for this serie. I am currently reworking the vfio-platform device
to inherit from this device instead of SysBusDevice.

Best Regards

Eric
> 
> Signed-off-by: Alexander Graf <address@hidden>
> ---
>  hw/Makefile.objs             |   1 +
>  hw/platform/Makefile.objs    |   1 +
>  hw/platform/device.c         | 108 
> +++++++++++++++++++++++++++++++++++++++++++
>  include/hw/platform/device.h |  45 ++++++++++++++++++
>  4 files changed, 155 insertions(+)
>  create mode 100644 hw/platform/Makefile.objs
>  create mode 100644 hw/platform/device.c
>  create mode 100644 include/hw/platform/device.h
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index d178b65..f300f68 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -20,6 +20,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += nvram/
>  devices-dirs-$(CONFIG_SOFTMMU) += pci/
>  devices-dirs-$(CONFIG_PCI) += pci-bridge/ pci-host/
>  devices-dirs-$(CONFIG_SOFTMMU) += pcmcia/
> +devices-dirs-$(CONFIG_PLATFORM) += platform/
>  devices-dirs-$(CONFIG_SOFTMMU) += scsi/
>  devices-dirs-$(CONFIG_SOFTMMU) += sd/
>  devices-dirs-$(CONFIG_SOFTMMU) += ssi/
> diff --git a/hw/platform/Makefile.objs b/hw/platform/Makefile.objs
> new file mode 100644
> index 0000000..824356b
> --- /dev/null
> +++ b/hw/platform/Makefile.objs
> @@ -0,0 +1 @@
> +common-obj-$(CONFIG_PLATFORM) += device.o
> diff --git a/hw/platform/device.c b/hw/platform/device.c
> new file mode 100644
> index 0000000..9e23370
> --- /dev/null
> +++ b/hw/platform/device.c
> @@ -0,0 +1,108 @@
> +/*
> + * Platform Device that can expose its memory regions and IRQ lines
> + *
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + *
> + * Authors: Alexander Graf,   <address@hidden>
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of  the GNU General  Public License as published by
> + * the Free Software Foundation;  either version 2 of the  License, or
> + * (at your option) any later version.
> + *
> + *
> + * This is an abstract platform device, so you really only want to use it
> + * as parent class for platform devices.
> + *
> + * It ensures that all boilerplate is properly abstracted away from children
> + * and consistent across devices.
> + *
> + * When instantiating a platform device you can optionally always specify 2
> + * properties which otherwise get populated automatically:
> + *
> + *  regions: Offsets in the platform hole the device's memory regions get 
> mapped
> + *           to.
> + *  irqs: IRQ pins in the linear platform IRQ range the device's IRQs get 
> mapped
> + *           to.
> + */
> +
> +#include "qemu-common.h"
> +#include "hw/platform/device.h"
> +
> +static void fixup_regions(PlatformDeviceState *s)
> +{
> +    uint64_t *addrs = g_new(uint64_t, s->num_regions);
> +    int i;
> +
> +    /* Treat memory offsets that the user did not specify as dynamic */
> +    for (i = 0; i < s->num_regions; i++) {
> +        if (s->num_plat_region_addrs > i) {
> +            addrs[i] = s->plat_region_addrs[i];
> +        } else {
> +            addrs[i] = PLATFORM_DYNAMIC;
> +        }
> +    }
> +
> +    s->plat_region_addrs = addrs;
> +    s->num_plat_region_addrs = s->num_regions;
> +}
> +
> +static void fixup_irqs(PlatformDeviceState *s)
> +{
> +    uint32_t *irqs = g_new(uint32_t, s->num_irqs);
> +    int i;
> +
> +    /* Treat IRQs that the user did not specify as dynamic */
> +    for (i = 0; i < s->num_irqs; i++) {
> +        if (s->num_plat_irqs > i) {
> +            irqs[i] = s->plat_irqs[i];
> +        } else {
> +            irqs[i] = PLATFORM_DYNAMIC;
> +        }
> +    }
> +
> +    s->plat_irqs = irqs;
> +    s->num_plat_irqs = s->num_irqs;
> +}
> +
> +static void platform_device_realize(DeviceState *dev, Error **errp)
> +{
> +    PlatformDeviceState *s = PLATFORM_DEVICE(dev);
> +
> +    fixup_regions(s);
> +    fixup_irqs(s);
> +}
> +
> +static Property platform_device_properties[] = {
> +    /* memory regions for a device */
> +    DEFINE_PROP_ARRAY("regions", PlatformDeviceState, num_plat_region_addrs,
> +                      plat_region_addrs, qdev_prop_uint64, uint64_t),
> +    /* interrupts for a device */
> +    DEFINE_PROP_ARRAY("irqs", PlatformDeviceState, num_plat_irqs,
> +                      plat_irqs, qdev_prop_uint32, uint32_t),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void platform_device_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = platform_device_realize;
> +    dc->props = platform_device_properties;
> +}
> +
> +static const TypeInfo platform_device_type_info = {
> +    .name = TYPE_PLATFORM_DEVICE,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(PlatformDeviceState),
> +    .abstract = true,
> +    .class_init = platform_device_class_init,
> +};
> +
> +
> +static void device_register_types(void)
> +{
> +    type_register_static(&platform_device_type_info);
> +}
> +
> +type_init(device_register_types)
> diff --git a/include/hw/platform/device.h b/include/hw/platform/device.h
> new file mode 100644
> index 0000000..a17f79e
> --- /dev/null
> +++ b/include/hw/platform/device.h
> @@ -0,0 +1,45 @@
> +/*
> + * Generic platform device with IRQ and IO placement hints
> + *
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + *
> + * Authors: Alexander Graf,   <address@hidden>
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of  the GNU General  Public License as published by
> + * the Free Software Foundation;  either version 2 of the  License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef QEMU_HW_PLATFORM_DEVICE_H
> +#define QEMU_HW_PLATFORM_DEVICE_H
> +
> +#include "qemu-common.h"
> +#include "hw/qdev.h"
> +
> +#define TYPE_PLATFORM_DEVICE "platform-device"
> +#define PLATFORM_DEVICE(obj) OBJECT_CHECK(PlatformDeviceState, (obj), 
> TYPE_PLATFORM_DEVICE)
> +#define PLATFORM_DYNAMIC -1LL
> +
> +typedef struct PlatformDeviceState {
> +    /*< private >*/
> +
> +    DeviceState parent_obj;
> +    QTAILQ_ENTRY(PlatformDeviceState) next;
> +
> +    /* these get set by children to indicate their resources */
> +    uint32_t num_regions;
> +    MemoryRegion **regions;
> +    uint32_t num_irqs;
> +    qemu_irq **irqs;
> +
> +    /*< public >*/
> +
> +    /* these get set by the user though qdev parameters */
> +    uint32_t num_plat_region_addrs;
> +    uint64_t *plat_region_addrs;
> +    uint32_t num_plat_irqs;
> +    uint32_t *plat_irqs;
> +} PlatformDeviceState;
> +
> +#endif /* !QEMU_HW_PLATFORM_DEVICE_H */
>