[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/5] PPC: e500: Support platform devices
From: |
Eric Auger |
Subject: |
Re: [Qemu-devel] [PATCH 4/5] PPC: e500: Support platform devices |
Date: |
Fri, 27 Jun 2014 18:50:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 |
On 06/27/2014 01:30 PM, Alexander Graf wrote:
>
> On 27.06.14 11:29, Eric Auger wrote:
>> On 06/04/2014 02:28 PM, Alexander Graf wrote:
>>> 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 <address@hidden>
>>> ---
>>> 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
>>> +#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);
>>> +}
>>> +
>>> 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;
>> Hi Alex,
>>
>> Wouldn' it be more natural to add IRQ_START here, given the semantic of
>> plat_irqs? Besides, I saw you added it dt_serial_create.
>
> It depends on what you call "natural". I personally find it natural to
> only expose the limited number space that is actually available to us -
> that's the way device tree also does it.
>
> It's incredibly hard to define a flat interrupt numbering scheme for the
> whole machine. What do you do when you have 2 interrupt controllers?
> What do you do with different types of interrupts, such as critical
> interrupt lines?
Hi Alex
I agree with you for both irqs and region_addrs. With your explanations
it makes sense but for me this semantic was not self-explanatory from
the device.h.
BR
Eric
>
>>> +
>>> + 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;
>> same for E500_PLATFORM_BASE? Actually you use plat_region_addrs[0] as an
>> offset wrt platorm parent region in dt_serial_create. The exact semantic
>> may be clarified.
>
> Not sure I understand :). A bus in device tree logic maps its address
> space to a subset of its parent address space. So we configure the
> device tree address space to be in line with the virtual "platform bus"
> we model.
>
>>
>> Otherwise, the full patch works fine for vfio & virt too. I just moved
>> that code in a separate file and moved E500_* in a PlatformSettings
>> struct included in both PlatformNotifier and PlatformInitData.
>
> Cool, I'm glad to hear that it works :). I'm currently reworking the
> patches to add hints to SysBus instead - let's see how that works out.
> But at the end of the day, the idea stays the same and converting from
> one approach to the other should be minimal mechanical work.
>
>
> Alex
>
- [Qemu-devel] [PATCH 0/5] Platform device support, Alexander Graf, 2014/06/04
- [Qemu-devel] [PATCH 5/5] PPC: e500: Add support for platform serial devices, Alexander Graf, 2014/06/04
- [Qemu-devel] [PATCH 1/5] Platform: Add platform device class, Alexander Graf, 2014/06/04
- [Qemu-devel] [PATCH 2/5] Platform: Add serial device, Alexander Graf, 2014/06/04
- [Qemu-devel] [PATCH 3/5] PPC: e500: Only create dt entries for existing serial ports, Alexander Graf, 2014/06/04
- Re: [Qemu-devel] [PATCH 0/5] Platform device support, Paolo Bonzini, 2014/06/19
- Re: [Qemu-devel] [PATCH 0/5] Platform device support, Peter Crosthwaite, 2014/06/20