Re: [Qemu-devel] [PATCH 1/6] s390x/virtio-ccw: enable has_dynamic_sysbus

[Cornelia Huck]

On Mon, 27 Apr 2015 17:15:03 +0200
Andreas Färber <address@hidden> wrote:

> Am 27.04.2015 um 16:19 schrieb Cornelia Huck:
> > On Mon, 27 Apr 2015 15:57:04 +0200
> > Alexander Graf <address@hidden> wrote:
> >> On 04/24/2015 11:07 AM, Cornelia Huck wrote:
> >>> On Wed, 22 Apr 2015 14:21:36 +0200
> >>> Alexander Graf <address@hidden> wrote:
> >>>>> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <address@hidden>:
> >>>>> On Wed, 22 Apr 2015 11:14:40 +0200
> >>>>> Alexander Graf <address@hidden> wrote:
> >>>>>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
> >>>>>>> On Tue, 21 Apr 2015 21:06:42 +0200
> >>>>>>> Alexander Graf <address@hidden> wrote:
> >>>>>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
> >>>>>>>>> From: Xu Wang <address@hidden>
> >>>>>>>>>
> >>>>>>>>> We have to enable this flag to support dynamically adding devices 
> >>>>>>>>> to the
> >>>>>>>>> sysbus. This change is needed for the the upcoming diag288 watchdog.
> >>>>>>>> s390 doesn't have a "sysbus" per se. Please create a new bus type.
> >>>>>>> So what's wrong with the sysbus? I don't see why we should be 
> >>>>>>> different
> >>>>>>> than everyone else.
> >>>>>> The idea behind sysbus is that you have MMIO, PIO and IRQ pins
> >>>>>> connecting to a PIC. It provides a lot of infrastructure for those
> >>>>>> interfaces. S390 doesn't use any of them and instead wants registration
> >>>>>> on "diag" interfaces for example which I'd put on the same layer as PIO
> >>>>>> or MMIO registration.
> >>>>> I don't think a "diag" bus makes sense.
> >>>> You don't need a bus necessarily, just a parent class.
> >>>>
> >>>>> The individual diagnoses are
> >>>>> way too heterogenous beyond the fact that they use the same base
> >>>>> instruction.
> >>>>>
> >>>>> So where's the proper place for "misc" devices? My impression was that
> >>>>> they can go on the sysbus.
> >>>>>
> >>>> If you really don't want to create your own class, how about you inherit 
> >>>> from the DeviceState class?
> >>> I tried that for the watchdog, and it certainly works, but some things
> >>> end up odd:
> >>>
> >>> - in 'info qtree', the watchdog device does not show up at all
> >>
> >> Please try "info qom-tree". Andreas also has a patch outstanding that 
> >> shows properties along the way with a verbose switch.
> > 
> > While it does show up in info qom-tree, is hiding it from info qtree a
> > good idea? I'd think that it is still widely used.
> 
> That's why I proposed to drop info qtree, so that people no longer
> mistakenly use it and do weird designs because of it. But there was
> opposition to it and its incomplete replacements at the time - in 2.3 we
> at least have the qom-tree display and an external qom-tree script to
> display all the properties.
> 
> In the end, the bus view and the composition view complement each other
> as long as we can't or don't want to get rid of qbuses completely.
> 
> >>> - in the list of devices printed by "-device help", diag288 is now the
> >>>    only device without any bus
> >>
> >> But it's not attached to a bus, so that's reasonable, no?
> > 
> > Hm. Are there bus-less devices on other platforms?
> 
> Take a look at the recent APIC patches, it's being converted from an ICC
> bus (for making hot-add work at the time) to bus-less device.
> 
> PCMCIA is a bus-less device already IIRC.

I'll take a look.

> 
> Just search for .parent = TYPE_DEVICE. :)
> 
> >>> I would have thought that any device not attached to a specialized bus
> >>> should end up on the main system bus, which brings me back to adding it
> >>> as a sysbus device ;)
> >>
> >> Not really, sysbus is QEMU's wording for what Linux calls "platform 
> >> bus". It's where devices go to that are attached to MMIO/PIO/IRQ lines 
> >> via some fabric that we don't model.
> > 
> > But in practice sysbus seems to be more like a catch-all: On s390x,
> > there are already things like the flic, various sclp-related devices,
> > the virtio bridges or the ipl device sitting on the sysbus. Should they
> > really be thrown out from the sysbus and dangle as bus-less devices? I
> > think there is a case to be made for a catch-all bus, even if it is not
> > the sysbus.
> 
> There's a difference between "dangling" and SysBus. There cannot be
> dangling QOM objects - that's part of the ongoing CPU discussion we're
> having (and that people seem to keep forgetting). They need to have a
> parent, i.e. a child<> property leading to them, recursively forming a
> canonical path. Internal devices are usually dangling and that is
> currently being caught be placing them in /machine/unattached at
> realization time - much too late. Devices added via -device or
> device_add are never dangling as they are placed in /machine/peripheral
> or /machine/peripheral-anon. A better question is whether that is
> actually desired for your PV devices or whether it should just be a
> -machine option that enabled a device sitting on /machine directly or
> wherever sensible.

Having the devices we currently have on the sysbus controlled via
machine options does not quite feel right to me at a first glance.
Placing them as "peripheral" somehow does not feel quite right either,
but that seems to be what we get from making it a device.

I think I'll try the s390 platform bus first. But should we perhaps
have something like /machine/infrastructure or so?