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

[Andreas Färber]

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.

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.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)