Re: [Qemu-devel] [PATCH 4/5] PPC: e500: Support platform devices

[Alexander Graf <agraf@suse.de>]

On 13.06.14 10:58, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: qemu-devel-bounces+bharat.bhushan=freescale.com@nongnu.org [mailto:qemu-
>> devel-bounces+bharat.bhushan=freescale.com@nongnu.org] On Behalf Of Alexander
>> Graf
>> Sent: Wednesday, June 04, 2014 5:59 PM
>> To: qemu-ppc@nongnu.org
>> Cc: qemu-devel@nongnu.org; eric.auger@linaro.org
>> Subject: [Qemu-devel] [PATCH 4/5] PPC: e500: Support platform devices
>>
>> For e500 our approach to supporting platform devices is to create a simple
>> bus from the guest's point of view within which we map platform devices
>> dynamically.
>>
>> We allocate memory regions always within the "platform" hole in address
>> space and map IRQs to predetermined IRQ lines that are reserved for platform
>> device usage.
>>
>> This maps really nicely into device tree logic, so we can just tell the
>> guest about our virtual simple bus in device tree as well.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>   default-configs/ppc-softmmu.mak   |   1 +
>>   default-configs/ppc64-softmmu.mak |   1 +
>>   hw/ppc/e500.c                     | 221 ++++++++++++++++++++++++++++++++++++++
>>   hw/ppc/e500.h                     |   1 +
>>   hw/ppc/e500plat.c                 |   1 +
>>   5 files changed, 225 insertions(+)
>>
>> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
>> index 33f8d84..d6ec8b9 100644
>> --- a/default-configs/ppc-softmmu.mak
>> +++ b/default-configs/ppc-softmmu.mak
>> @@ -45,6 +45,7 @@ CONFIG_PREP=y
>>   CONFIG_MAC=y
>>   CONFIG_E500=y
>>   CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM))
>> +CONFIG_PLATFORM=y
>>   # For PReP
>>   CONFIG_MC146818RTC=y
>>   CONFIG_ETSEC=y
>> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-
>> softmmu.mak
>> index 37a15b7..06677bf 100644
>> --- a/default-configs/ppc64-softmmu.mak
>> +++ b/default-configs/ppc64-softmmu.mak
>> @@ -45,6 +45,7 @@ CONFIG_PSERIES=y
>>   CONFIG_PREP=y
>>   CONFIG_MAC=y
>>   CONFIG_E500=y
>> +CONFIG_PLATFORM=y
>>   CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM))
>>   # For pSeries
>>   CONFIG_XICS=$(CONFIG_PSERIES)
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 33d54b3..bc26215 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -36,6 +36,7 @@
>>   #include "exec/address-spaces.h"
>>   #include "qemu/host-utils.h"
>>   #include "hw/pci-host/ppce500.h"
>> +#include "hw/platform/device.h"
>>
>>   #define EPAPR_MAGIC                (0x45504150)
>>   #define BINARY_DEVICE_TREE_FILE    "mpc8544ds.dtb"
>> @@ -47,6 +48,14 @@
>>
>>   #define RAM_SIZES_ALIGN            (64UL << 20)
>>
>> +#define E500_PLATFORM_BASE         0xF0000000ULL
>> +#define E500_PLATFORM_HOLE         (128ULL * 1024 * 1024) /* 128 MB */
>> +#define E500_PLATFORM_PAGE_SHIFT   12
>> +#define E500_PLATFORM_HOLE_PAGES   (E500_PLATFORM_HOLE >> \
>> +                                    E500_PLATFORM_PAGE_SHIFT)
>> +#define E500_PLATFORM_FIRST_IRQ    5
> How we come to value "5" ?
>
> How it is ensured that this does not overlap with IRQ numbers taken by PCI devices (with no-msi case)?

UART = 42
PCI = 1, 2, 3, 4

So I figured 5 is right behind the PCI IRQs, but before the UART ;)

>
>> +#define E500_PLATFORM_NUM_IRQS     10
>> +
>>   /* TODO: parameterize */
>>   #define MPC8544_CCSRBAR_BASE       0xE0000000ULL
>>   #define MPC8544_CCSRBAR_SIZE       0x00100000ULL
>> @@ -122,6 +131,62 @@ static void dt_serial_create(void *fdt, unsigned long long
>> offset,
>>       }
>>   }
>>
>> +typedef struct PlatformDevtreeData {
>> +    void *fdt;
>> +    const char *mpic;
>> +    int irq_start;
>> +    const char *node;
>> +} PlatformDevtreeData;
>> +
>> +static int platform_device_create_devtree(Object *obj, void *opaque)
>> +{
>> +    PlatformDevtreeData *data = opaque;
>> +    Object *dev;
>> +    PlatformDeviceState *pdev;
>> +
>> +    dev = object_dynamic_cast(obj, TYPE_PLATFORM_DEVICE);
>> +    pdev = (PlatformDeviceState *)dev;
>> +
>> +    if (!pdev) {
>> +        /* Container, traverse it for children */
>> +        return object_child_foreach(obj, platform_device_create_devtree, data);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void platform_create_devtree(void *fdt, const char *node, uint64_t addr,
>> +                                    const char *mpic, int irq_start,
>> +                                    int nr_irqs)
>> +{
>> +    const char platcomp[] = "qemu,platform\0simple-bus";
>> +    PlatformDevtreeData data;
>> +
>> +    /* Create a /platform node that we can put all devices into */
>> +
>> +    qemu_fdt_add_subnode(fdt, node);
>> +    qemu_fdt_setprop(fdt, node, "compatible", platcomp, sizeof(platcomp));
>> +    qemu_fdt_setprop_string(fdt, node, "device_type", "platform");
>> +
>> +    /* Our platform hole is less than 32bit big, so 1 cell is enough for
>> address
>> +       and size */
>> +    qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1);
>> +    qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1);
>> +    qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr,
>> +                           E500_PLATFORM_HOLE);
>> +
>> +    qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic);
>> +
>> +    /* Loop through all devices and create nodes for known ones */
>> +
>> +    data.fdt = fdt;
>> +    data.mpic = mpic;
>> +    data.irq_start = irq_start;
>> +    data.node = node;
>> +
>> +    platform_device_create_devtree(qdev_get_machine(), &data);
> So I see dma device with two dm-channel then h/w device tree looks like :
> 	dma@100300 {
>                          #address-cells = <0x1>;
>                          #size-cells = <0x1>;
>                          compatible = "fsl,elo3-dma";
>                          reg = <0x100300 0x4 0x100600 0x4>;
>                          ranges = <0x0 0x100100 0x500>;
>
>                          dma-channel@0 {
>                                  compatible = "fsl,eloplus-dma-channel";
>                                  reg = <0x0 0x80>;
>                                  interrupts = <0x1c 0x2 0x0 0x0>;
>                          };
>
>                          dma-channel@80 {
>                                  compatible = "fsl,eloplus-dma-channel";
>                                  reg = <0x80 0x80>;
>                                  interrupts = <0x1d 0x2 0x0 0x0>;
>                          };
>
> 		}
>
>
> Now for assigning same device to guest, We will unbind dma@ device from kernel and then bind with vfio-platform.
>
> 1) What will be my qemu command line for this ? I think this will be like:
> 	-device vfio-platform,vfio_device="dma@100300",compat="fsl,elo3-dma"
>
> Then how similar device tree will be created? It can happen then some dma have 1 channel while other have more "n" channel.

I don't see any point in only forwarding a DMA channel on its own, but 
if you really have to, it would work like

   -device vfio-platform-elo3-dma,vfio=device="dma@100300"

This creates an object of the VFIO_PLATFORM_ELO3_DMA type in QEMU which 
the board file can look up in its device tree generation and create 
device tree entries accordingly.

>
> That also reminds me that should we update documents for same " docs/qdev-device-use.txt"
>
>> +}
>> +
>>   static int ppce500_load_device_tree(MachineState *machine,
>>                                       PPCE500Params *params,
>>                                       hwaddr addr,
>> @@ -379,6 +444,12 @@ static int ppce500_load_device_tree(MachineState *machine,
>>       qemu_fdt_setprop_cell(fdt, pci, "#address-cells", 3);
>>       qemu_fdt_setprop_string(fdt, "/aliases", "pci0", pci);
>>
>> +    if (params->has_platform) {
>> +        platform_create_devtree(fdt, "/platform", E500_PLATFORM_BASE,
>> +                               mpic, E500_PLATFORM_FIRST_IRQ,
>> +                               E500_PLATFORM_NUM_IRQS);
>> +    }
>> +
>>       params->fixup_devtree(params, fdt);
>>
>>       if (toplevel_compat) {
>> @@ -618,6 +689,147 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params,
>> MemoryRegion *ccsr,
>>       return mpic;
>>   }
>>
>> +typedef struct PlatformNotifier {
>> +    Notifier notifier;
>> +    MemoryRegion *address_space_mem;
>> +    qemu_irq *mpic;
>> +} PlatformNotifier;
>> +
>> +typedef struct PlatformInitData {
>> +    unsigned long *used_irqs;
>> +    unsigned long *used_mem;
>> +    MemoryRegion *mem;
>> +    qemu_irq *irqs;
>> +    int device_count;
>> +} PlatformInitData;
>> +
>> +static int platform_map_irq(unsigned long *used_irqs, qemu_irq *platform_irqs,
>> +                            uint32_t *device_irqn, qemu_irq *device_irq)
>> +{
>> +    int max_irqs = E500_PLATFORM_NUM_IRQS;
>> +    int irqn = *device_irqn;
>> +
>> +    if (irqn == (uint32_t)PLATFORM_DYNAMIC) {
>> +        /* Find the first available IRQ */
>> +        irqn = find_first_zero_bit(used_irqs, max_irqs);
>> +    }
>> +
>> +    if ((irqn >= max_irqs) || test_and_set_bit(irqn, used_irqs)) {
>> +        hw_error("e500: IRQ %d is already allocated or no free IRQ left",
>> irqn);
>> +    }
>> +
>> +    *device_irq = platform_irqs[irqn];
>> +    *device_irqn = irqn;
>> +
>> +    return 0;
>> +}
>> +
>> +static int platform_map_region(unsigned long *used_mem, MemoryRegion *pmem,
>> +                               uint64_t *device_addr, MemoryRegion *device_mem)
>> +{
>> +    uint64_t size = memory_region_size(device_mem);
>> +    uint64_t page_size = (1 << E500_PLATFORM_PAGE_SHIFT);
>> +    uint64_t page_mask = page_size - 1;
>> +    uint64_t size_pages = (size + page_mask) >> E500_PLATFORM_PAGE_SHIFT;
>> +    hwaddr addr = *device_addr;
>> +    int page;
>> +    int i;
>> +
>> +    page = addr >> E500_PLATFORM_PAGE_SHIFT;
>> +    if (addr == (uint64_t)PLATFORM_DYNAMIC) {
>> +        uint64_t size_pages_align;
>> +
>> +        /* Align the region to at least its own size granularity */
>> +        if (is_power_of_2(size_pages)) {
>> +            size_pages_align = size_pages;
>> +        } else {
>> +            size_pages_align = pow2floor(size_pages) << 1;
>> +        }
>> +
>> +        /* Find the first available region that fits */
>> +        page = bitmap_find_next_zero_area(used_mem, E500_PLATFORM_HOLE_PAGES,
>> 0,
>> +                                          size_pages, size_pages_align);
>> +
>> +        addr = (uint64_t)page << E500_PLATFORM_PAGE_SHIFT;
>> +    }
>> +
>> +    if (page >= E500_PLATFORM_HOLE_PAGES || test_bit(page, used_mem) ||
>> +        (find_next_bit(used_mem, E500_PLATFORM_HOLE_PAGES, page) < size_pages))
>> {
>> +        hw_error("e500: Memory [%"PRIx64":%"PRIx64" is already allocated or "
>> +                 "no slot left", addr, size);
>> +    }
>> +
>> +    for (i = page; i < (page + size_pages); i++) {
>> +        set_bit(i, used_mem);
>> +    }
>> +
>> +    memory_region_add_subregion(pmem, addr, device_mem);
>> +    *device_addr = addr;
>> +
>> +    return 0;
>> +}
>> +
>> +static int platform_device_check(Object *obj, void *opaque)
>> +{
>> +    PlatformInitData *init = opaque;
>> +    Object *dev;
>> +    PlatformDeviceState *pdev;
>> +    int i;
>> +
>> +    dev = object_dynamic_cast(obj, TYPE_PLATFORM_DEVICE);
>> +    pdev = (PlatformDeviceState *)dev;
>> +
>> +    if (!pdev) {
>> +        /* Container, traverse it for children */
>> +        return object_child_foreach(obj, platform_device_check, opaque);
>> +    }
>> +
>> +    /* Connect platform device to machine */
>> +    for (i = 0; i < pdev->num_irqs; i++) {
>> +        platform_map_irq(init->used_irqs, init->irqs, &pdev->plat_irqs[i],
>> +                         pdev->irqs[i]);
>> +    }
>> +
>> +    for (i = 0; i < pdev->num_regions; i++) {
>> +        platform_map_region(init->used_mem, init->mem,
>> +                            &pdev->plat_region_addrs[i], pdev->regions[i]);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void platform_devices_init(MemoryRegion *address_space_mem,
>> +                                  qemu_irq *mpic)
>> +{
>> +    DECLARE_BITMAP(used_irqs, E500_PLATFORM_NUM_IRQS);
>> +    DECLARE_BITMAP(used_mem, E500_PLATFORM_HOLE_PAGES);
>> +    MemoryRegion *platform_region = g_new(MemoryRegion, 1);
>> +    PlatformInitData init = {
>> +        .used_irqs = used_irqs,
>> +        .used_mem = used_mem,
>> +        .mem = platform_region,
>> +        .irqs = &mpic[E500_PLATFORM_FIRST_IRQ],
>> +    };
>> +
>> +    memory_region_init(platform_region, NULL, "platform devices",
>> +                       E500_PLATFORM_HOLE);
>> +
>> +    bitmap_clear(used_irqs, 0, E500_PLATFORM_NUM_IRQS);
>> +    bitmap_clear(used_mem, 0, E500_PLATFORM_HOLE_PAGES);
>> +
>> +    /* Loop through our internal device tree and attach any dangling device */
>> +    platform_device_check(qdev_get_machine(), &init);
> Can you please describe what we are trying to do here, what could be a "dangling device" ?

A "dangling device" is a device that's not attached to any bus. Since 
there is no platform bus, all devices of the DEVICE_PLATFORM type are 
"dangling" when they get created.


Alex