diff mbox

[1/6] s390x/virtio-ccw: enable has_dynamic_sysbus

Message ID 1429257161-29597-2-git-send-email-cornelia.huck@de.ibm.com
State New
Headers show

Commit Message

Cornelia Huck April 17, 2015, 7:52 a.m. UTC
From: Xu Wang <gesaint@linux.vnet.ibm.com>

We have to enable this flag to support dynamically adding devices to the
sysbus. This change is needed for the the upcoming diag288 watchdog.

Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Alexander Graf April 21, 2015, 7:06 p.m. UTC | #1
On 04/17/2015 09:52 AM, Cornelia Huck wrote:
> From: Xu Wang <gesaint@linux.vnet.ibm.com>
>
> 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.


Alex

>
> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com>
> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>   hw/s390x/s390-virtio-ccw.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index afb539a..df89440 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -216,6 +216,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
>       mc->no_sdcard = 1;
>       mc->use_sclp = 1;
>       mc->max_cpus = 255;
> +    mc->has_dynamic_sysbus = true;
>       nc->nmi_monitor_handler = s390_nmi;
>   }
>
Cornelia Huck April 22, 2015, 8:25 a.m. UTC | #2
On Tue, 21 Apr 2015 21:06:42 +0200
Alexander Graf <agraf@suse.de> wrote:

> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
> > From: Xu Wang <gesaint@linux.vnet.ibm.com>
> >
> > 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.
Alexander Graf April 22, 2015, 9:14 a.m. UTC | #3
On 04/22/2015 10:25 AM, Cornelia Huck wrote:
> On Tue, 21 Apr 2015 21:06:42 +0200
> Alexander Graf <agraf@suse.de> wrote:
>
>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
>>>
>>> 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.


Alex
Cornelia Huck April 22, 2015, 11:40 a.m. UTC | #4
On Wed, 22 Apr 2015 11:14:40 +0200
Alexander Graf <agraf@suse.de> wrote:

> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
> > On Tue, 21 Apr 2015 21:06:42 +0200
> > Alexander Graf <agraf@suse.de> wrote:
> >
> >> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
> >>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
> >>>
> >>> 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. 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.
Alexander Graf April 22, 2015, 12:21 p.m. UTC | #5
> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
> 
> On Wed, 22 Apr 2015 11:14:40 +0200
> Alexander Graf <agraf@suse.de> wrote:
> 
>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
>>> On Tue, 21 Apr 2015 21:06:42 +0200
>>> Alexander Graf <agraf@suse.de> wrote:
>>> 
>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
>>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
>>>>> 
>>>>> 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?

Alex
Cornelia Huck April 24, 2015, 9:07 a.m. UTC | #6
On Wed, 22 Apr 2015 14:21:36 +0200
Alexander Graf <agraf@suse.de> wrote:

> 
> 
> > Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
> > 
> > On Wed, 22 Apr 2015 11:14:40 +0200
> > Alexander Graf <agraf@suse.de> wrote:
> > 
> >>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
> >>> On Tue, 21 Apr 2015 21:06:42 +0200
> >>> Alexander Graf <agraf@suse.de> wrote:
> >>> 
> >>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
> >>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
> >>>>> 
> >>>>> 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
- in the list of devices printed by "-device help", diag288 is now the
  only device without any bus

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 ;)

Does sysbus instead need to get more generic? The platform stuff seems
to be a better fit for MMIO, PIO et al.?
Alexander Graf April 27, 2015, 1:57 p.m. UTC | #7
On 04/24/2015 11:07 AM, Cornelia Huck wrote:
> On Wed, 22 Apr 2015 14:21:36 +0200
> Alexander Graf <agraf@suse.de> wrote:
>
>>
>>> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
>>>
>>> On Wed, 22 Apr 2015 11:14:40 +0200
>>> Alexander Graf <agraf@suse.de> wrote:
>>>
>>>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
>>>>> On Tue, 21 Apr 2015 21:06:42 +0200
>>>>> Alexander Graf <agraf@suse.de> wrote:
>>>>>
>>>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
>>>>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
>>>>>>>
>>>>>>> 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.

> - 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?

> 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.

The target for devices that are not the above we now have non-bus'ed 
devices.


Alex


> Does sysbus instead need to get more generic? The platform stuff seems
> to be a better fit for MMIO, PIO et al.?
Cornelia Huck April 27, 2015, 2:19 p.m. UTC | #8
On Mon, 27 Apr 2015 15:57:04 +0200
Alexander Graf <agraf@suse.de> wrote:

> On 04/24/2015 11:07 AM, Cornelia Huck wrote:
> > On Wed, 22 Apr 2015 14:21:36 +0200
> > Alexander Graf <agraf@suse.de> wrote:
> >
> >>
> >>> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
> >>>
> >>> On Wed, 22 Apr 2015 11:14:40 +0200
> >>> Alexander Graf <agraf@suse.de> wrote:
> >>>
> >>>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
> >>>>> On Tue, 21 Apr 2015 21:06:42 +0200
> >>>>> Alexander Graf <agraf@suse.de> wrote:
> >>>>>
> >>>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
> >>>>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
> >>>>>>>
> >>>>>>> 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.

> 
> > - 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?

> 
> > 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.

> 
> The target for devices that are not the above we now have non-bus'ed 
> devices.

I'm afraid I can't parse that sentence :)
Andreas Färber April 27, 2015, 3:15 p.m. UTC | #9
Am 27.04.2015 um 16:19 schrieb Cornelia Huck:
> On Mon, 27 Apr 2015 15:57:04 +0200
> Alexander Graf <agraf@suse.de> wrote:
>> On 04/24/2015 11:07 AM, Cornelia Huck wrote:
>>> On Wed, 22 Apr 2015 14:21:36 +0200
>>> Alexander Graf <agraf@suse.de> wrote:
>>>>> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
>>>>> On Wed, 22 Apr 2015 11:14:40 +0200
>>>>> Alexander Graf <agraf@suse.de> wrote:
>>>>>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
>>>>>>> On Tue, 21 Apr 2015 21:06:42 +0200
>>>>>>> Alexander Graf <agraf@suse.de> wrote:
>>>>>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
>>>>>>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
>>>>>>>>>
>>>>>>>>> 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
Alexander Graf April 27, 2015, 3:28 p.m. UTC | #10
On 04/27/2015 04:19 PM, Cornelia Huck wrote:
> On Mon, 27 Apr 2015 15:57:04 +0200
> Alexander Graf <agraf@suse.de> wrote:
>
>> On 04/24/2015 11:07 AM, Cornelia Huck wrote:
>>> On Wed, 22 Apr 2015 14:21:36 +0200
>>> Alexander Graf <agraf@suse.de> wrote:
>>>
>>>>> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
>>>>>
>>>>> On Wed, 22 Apr 2015 11:14:40 +0200
>>>>> Alexander Graf <agraf@suse.de> wrote:
>>>>>
>>>>>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
>>>>>>> On Tue, 21 Apr 2015 21:06:42 +0200
>>>>>>> Alexander Graf <agraf@suse.de> wrote:
>>>>>>>
>>>>>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
>>>>>>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
>>>>>>>>>
>>>>>>>>> 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.

It's not really about hiding it, but just about putting it at a 
different location. S390 won't be the only target that slowly moves to 
non-qdev'ed devices, so this is a problem that we'll need to solve 
regardless.

>
>>> - 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?

IIUC Andreas wants to move CPUs to bus-less devices. I'm sure there are 
more to come.

That said, if you feel more comfortable with a bus, just create an 
artificial s390 bus for "s390 platform devices".

>
>>> 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.

The problem is that before QOM sysbus was the lowest common denominator 
that we had. Now with QOM, we're only slowly starting to get the ability 
to have devices that are not attached to a bus.

>
>> The target for devices that are not the above we now have non-bus'ed
>> devices.
> I'm afraid I can't parse that sentence :)

FWIW you're supposed to use direct, non-bus'ed QOM devices for devices 
that don't sit on a bus now. Before this was not possible, now it is :).

I don't feel incredibly strongly about this, but I just consider it 
awkward to plug s390 specific devices into a bus that implements 
everything s390 doesn't do :).


Alex
Cornelia Huck April 27, 2015, 4:56 p.m. UTC | #11
On Mon, 27 Apr 2015 17:28:29 +0200
Alexander Graf <agraf@suse.de> wrote:

> On 04/27/2015 04:19 PM, Cornelia Huck wrote:
> > On Mon, 27 Apr 2015 15:57:04 +0200
> > Alexander Graf <agraf@suse.de> wrote:
> >
> >> On 04/24/2015 11:07 AM, Cornelia Huck wrote:
> >>> On Wed, 22 Apr 2015 14:21:36 +0200
> >>> Alexander Graf <agraf@suse.de> wrote:
> >>>
> >>>>> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
> >>>>>
> >>>>> On Wed, 22 Apr 2015 11:14:40 +0200
> >>>>> Alexander Graf <agraf@suse.de> wrote:
> >>>>>
> >>>>>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
> >>>>>>> On Tue, 21 Apr 2015 21:06:42 +0200
> >>>>>>> Alexander Graf <agraf@suse.de> wrote:
> >>>>>>>
> >>>>>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
> >>>>>>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
> >>>>>>>>>
> >>>>>>>>> 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.
> 
> It's not really about hiding it, but just about putting it at a 
> different location. S390 won't be the only target that slowly moves to 
> non-qdev'ed devices, so this is a problem that we'll need to solve 
> regardless.

I'm just a bit uncomfortable with devices not showing up in info qtree.
info qom-tree is still too new :)

> 
> >
> >>> - 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?
> 
> IIUC Andreas wants to move CPUs to bus-less devices. I'm sure there are 
> more to come.
> 
> That said, if you feel more comfortable with a bus, just create an 
> artificial s390 bus for "s390 platform devices".

Might make sense for now. I'm not really sure where I'd want to plug in
the devices alternatively.

> 
> >
> >>> 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.
> 
> The problem is that before QOM sysbus was the lowest common denominator 
> that we had. Now with QOM, we're only slowly starting to get the ability 
> to have devices that are not attached to a bus.
> 
> >
> >> The target for devices that are not the above we now have non-bus'ed
> >> devices.
> > I'm afraid I can't parse that sentence :)
> 
> FWIW you're supposed to use direct, non-bus'ed QOM devices for devices 
> that don't sit on a bus now. Before this was not possible, now it is :).
> 
> I don't feel incredibly strongly about this, but I just consider it 
> awkward to plug s390 specific devices into a bus that implements 
> everything s390 doesn't do :).

Let me see if an s390 platform bus does it for us. Machine options or
so don't look particulary attractive to me right now.
Cornelia Huck April 27, 2015, 5:02 p.m. UTC | #12
On Mon, 27 Apr 2015 17:15:03 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 27.04.2015 um 16:19 schrieb Cornelia Huck:
> > On Mon, 27 Apr 2015 15:57:04 +0200
> > Alexander Graf <agraf@suse.de> wrote:
> >> On 04/24/2015 11:07 AM, Cornelia Huck wrote:
> >>> On Wed, 22 Apr 2015 14:21:36 +0200
> >>> Alexander Graf <agraf@suse.de> wrote:
> >>>>> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
> >>>>> On Wed, 22 Apr 2015 11:14:40 +0200
> >>>>> Alexander Graf <agraf@suse.de> wrote:
> >>>>>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
> >>>>>>> On Tue, 21 Apr 2015 21:06:42 +0200
> >>>>>>> Alexander Graf <agraf@suse.de> wrote:
> >>>>>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
> >>>>>>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
> >>>>>>>>>
> >>>>>>>>> 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?
Markus Armbruster April 28, 2015, 6:50 a.m. UTC | #13
Cornelia Huck <cornelia.huck@de.ibm.com> writes:

> On Mon, 27 Apr 2015 17:28:29 +0200
> Alexander Graf <agraf@suse.de> wrote:
>
>> On 04/27/2015 04:19 PM, Cornelia Huck wrote:
>> > On Mon, 27 Apr 2015 15:57:04 +0200
>> > Alexander Graf <agraf@suse.de> wrote:
>> >
>> >> On 04/24/2015 11:07 AM, Cornelia Huck wrote:
>> >>> On Wed, 22 Apr 2015 14:21:36 +0200
>> >>> Alexander Graf <agraf@suse.de> wrote:
>> >>>
>> >>>>> Am 22.04.2015 um 13:40 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
>> >>>>>
>> >>>>> On Wed, 22 Apr 2015 11:14:40 +0200
>> >>>>> Alexander Graf <agraf@suse.de> wrote:
>> >>>>>
>> >>>>>>> On 04/22/2015 10:25 AM, Cornelia Huck wrote:
>> >>>>>>> On Tue, 21 Apr 2015 21:06:42 +0200
>> >>>>>>> Alexander Graf <agraf@suse.de> wrote:
>> >>>>>>>
>> >>>>>>>>> On 04/17/2015 09:52 AM, Cornelia Huck wrote:
>> >>>>>>>>> From: Xu Wang <gesaint@linux.vnet.ibm.com>
>> >>>>>>>>>
>> >>>>>>>>> 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.
>> 
>> It's not really about hiding it, but just about putting it at a 
>> different location. S390 won't be the only target that slowly moves to 
>> non-qdev'ed devices, so this is a problem that we'll need to solve 
>> regardless.
>
> I'm just a bit uncomfortable with devices not showing up in info qtree.
> info qom-tree is still too new :)

That feeling will pass :)

>> >>> - 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?
>> 
>> IIUC Andreas wants to move CPUs to bus-less devices. I'm sure there are 
>> more to come.
>> 
>> That said, if you feel more comfortable with a bus, just create an 
>> artificial s390 bus for "s390 platform devices".
>
> Might make sense for now. I'm not really sure where I'd want to plug in
> the devices alternatively.

I'd like to encourage you to embrace TYPE_DEVICE.

A qbus should model a real-world bus.  Stretching the "bus" concept
somewhat is okay.  Useful test: what have devices plugging into this bus
type in common?  If they share something interesting, such as the way
they connect to the rest of the machine, then modelling their
commonalities as a qbus can make sense.

What do sysbus devices have in common?  Anything regarding how they
connect?  Not really: devices expose pins, and to connect them you have
to wire them up, but that's true for *any* device.

Sysbus only exists because qdev's design required devices to plug into a
qbus.  We've since relaxed that silly design assumption.  Let's not keep
inventing silly buses just because we used to have to.

Whether an s390 platform bus would be silly I can't judge.  What would
s390 platform bus devices have in common?

>> >>> 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.
>> 
>> The problem is that before QOM sysbus was the lowest common denominator 
>> that we had. Now with QOM, we're only slowly starting to get the ability 
>> to have devices that are not attached to a bus.
>> 
>> >
>> >> The target for devices that are not the above we now have non-bus'ed
>> >> devices.
>> > I'm afraid I can't parse that sentence :)
>> 
>> FWIW you're supposed to use direct, non-bus'ed QOM devices for devices 
>> that don't sit on a bus now. Before this was not possible, now it is :).
>> 
>> I don't feel incredibly strongly about this, but I just consider it 
>> awkward to plug s390 specific devices into a bus that implements 
>> everything s390 doesn't do :).
>
> Let me see if an s390 platform bus does it for us. Machine options or
> so don't look particulary attractive to me right now.

Is it just due to unfamliarity?  That's temporary.
diff mbox

Patch

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index afb539a..df89440 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -216,6 +216,7 @@  static void ccw_machine_class_init(ObjectClass *oc, void *data)
     mc->no_sdcard = 1;
     mc->use_sclp = 1;
     mc->max_cpus = 255;
+    mc->has_dynamic_sysbus = true;
     nc->nmi_monitor_handler = s390_nmi;
 }