Re: [Qemu-ppc] [Qemu-devel] [PATCH 0/9] Add platform bus

[Anthony Liguori]

Alexander Graf <address@hidden> writes:

> Platforms without ISA and/or PCI have had a seriously hard time in the dynamic
> device creation world of QEMU. Devices on these were modeled as SysBus devices
> which can only be instantiated in machine files, not through -device.
>
> Why is that so?
>
> Well, SysBus is trying to be incredibly generic. It allows you to plug any
> interrupt sender into any other interrupt receiver. It allows you to map
> a device's memory regions into any other random memory region. All of that
> only works from C code.
>
> But do we need that level of complexity for normal devices usually? In a
> normal platform world (SoCs, PV machines) we have a flat memory layout we
> can plug our device memory into. We also have a flat IRQ model where we
> can plug our device IRQs into.
>
> This platform bus creates a simple bus that models the easy world. It allows
> for dynamic device creation through -device. A device may or may not explictly
> request to be mapped at a specific IRQ and/or memory address. If no explicit
> mapping is requested, platform devices just get mapped at convenient places.
>
> This goes hand in hand with automatic device tree generation. When QEMU
> places devices somewhere and also generates a device tree to tell the guest
> where exactly those devices are, we close the cycle and everyone knows
> everything and lives happily ever after.
>
> The actual pressing issue this solves is that today it's impossible to spawn
> serial ports from the command line. With this patch set, it's possible to
> do so. But it lays the groundwork for much more...

tl;dr, this is a PV bus for the e500 described as something more
generic.  I don't buy it.  I don't think there are many platforms or
devices out there where you can arbitrarily hook up devices at random
offsets/IRQs and expect things to work in any meaningful way.

So I'll suggest one of three things:

1) Just use PCI and call it a day

2) Rename this to E500PlatformBus and call it a day

3) If you really want to solve the general problem of platform devices,
   I think the approach needs to involve:

   a) Forget about any bus at all, just inherit from a common base class

   b) Use the common base class to expose interfaces to enumerate IRQs
      and MemoryRegions

   c) Have machine-specific logic (in the post init hook) that looks for
      any device of the type defined in (a) and connects their
      IRQs/MemoryRegions as appropriate.

   I can believe that this approach could be reasonably generic.  It's
   not abnormal for a hardware vendor to produce a single chip with all
   of the possible silicon on it and disable bits and pieces in order to
   offer a wider range of products.

   In this case, I could believe that a single SoC family could have up
   to four network adapters but I would also expect these adapters to
   live in four well known places.  With a machine specific hook, you
   can look at the specific device type and make these types of
   decisions appropriately.

Regards,

Anthony Liguori

>
> Alex
>
> Alexander Graf (9):
>   PlatBus: Add Platform Bus
>   PlatBus: Add abstract Platform Device
>   PlatBus: Add Sysbus/Platform bridge device
>   PlatBus: Hook up into Makefile system
>   PPC: Add platform bus to the default compile set
>   PlatBus: Add serial-platbus device
>   PPC: Add PlatBus Serial to default configs
>   PPC: E500: Spawn PlatBus bridge for ppce500 machine
>   PPC: E500: Add PlatBus device tree walker
>
>  default-configs/ppc-softmmu.mak    |   2 +
>  default-configs/ppc64-softmmu.mak  |   2 +
>  default-configs/ppcemb-softmmu.mak |   2 +
>  hw/Makefile.objs                   |   1 +
>  hw/char/Makefile.objs              |   1 +
>  hw/char/serial-platbus.c           | 100 +++++++++++++++++++++++
>  hw/platbus/Makefile.objs           |   1 +
>  hw/platbus/bridge.c                |  64 +++++++++++++++
>  hw/platbus/device.c                | 162 
> +++++++++++++++++++++++++++++++++++++
>  hw/platbus/platbus.c               | 107 ++++++++++++++++++++++++
>  hw/ppc/e500.c                      |  75 ++++++++++++++++-
>  hw/ppc/e500.h                      |   1 +
>  hw/ppc/e500plat.c                  |   1 +
>  include/hw/char/serial-platbus.h   |  56 +++++++++++++
>  include/hw/platbus/bridge.h        |  32 ++++++++
>  include/hw/platbus/device.h        | 102 +++++++++++++++++++++++
>  include/hw/platbus/platbus.h       |  86 ++++++++++++++++++++
>  17 files changed, 791 insertions(+), 4 deletions(-)
>  create mode 100644 hw/char/serial-platbus.c
>  create mode 100644 hw/platbus/Makefile.objs
>  create mode 100644 hw/platbus/bridge.c
>  create mode 100644 hw/platbus/device.c
>  create mode 100644 hw/platbus/platbus.c
>  create mode 100644 include/hw/char/serial-platbus.h
>  create mode 100644 include/hw/platbus/bridge.h
>  create mode 100644 include/hw/platbus/device.h
>  create mode 100644 include/hw/platbus/platbus.h
>
> -- 
> 1.8.1.4