Re: [Qemu-devel] [PATCH] sysbus: add no_user for devices using mmio or IRQ or GPIO

[Markus Armbruster]

Peter Maydell <address@hidden> writes:

> On 7 March 2013 16:48, Markus Armbruster <address@hidden> wrote:
>> Can we make some progress towards something that makes more sense?
>>
>> First step: reasons for marking a device no_user.
>>
>> From a user point of view, I think there's just one: -device/device_add
>> cannot possibly result in a working device.  Coherent enough semantics
>> for no_user, in my opinion, but I readily concede it's not particularly
>> useful for maintaining it as infrastructure and device models evolve.
>
> Ideally it would be nice to move to a model where the error message
> was "the device you've created has some required connections which
> haven't been wired up", and then for some devices you'd always get
> that error [until we implemented syntax for doing the wiring] and
> for some you'd only get it if you messed up the command line switches.

Sounds good to me.

>> For that, we need to drill down into reasons for "cannot possibly work".
>> Here are two, please add more:
>>
>> * Can't make required connections
>
>> * Resource collision with board
>>
>>   The device must connect to some fixed resource, but the board already
>>   connects something to it.  Without no_user, this should result in an
>>   error message of sorts, which is much better than the silent breakage
>>   above.  Whether the message makes any sense to the user is a different
>>   question.
>
> Do we have any concrete examples of this? I don't think devices should
> be making connections to resources themselves. If the only things wiring
> up devices are (a) the board models and (b) anything the user says on
> the command line, then the conflict only happens if the user says
> -device foo,bar=0x42 and bar 0x42 is in use; in that case the message
> will make sense to them.

Not exactly what you asked for, but here goes anyway:

    $ qemu-system-x86_64 -nodefaults -S -vnc :0 -monitor stdio -device 
isa-fdc,id=foo
    QEMU 1.4.50 monitor - type 'help' for more information
    (qemu) info qtree
    bus: main-system-bus
[...]
            bus: isa.0
              type ISA
              dev: isa-fdc, id "foo"
                iobase = 0x3f0
                irq = 6
                dma = 2
                driveA = <null>
                driveB = <null>
                bootindexA = -1
                bootindexB = -1
                check_media_rate = on
                isa irq 6
              dev: isa-fdc, id ""
                iobase = 0x3f0
                irq = 6
                dma = 2
                driveA = <null>
                driveB = <null>
                bootindexA = -1
                bootindexB = -1
                check_media_rate = on
                isa irq 6
[...]
    $ qemu-system-x86_64 -nodefaults -S -vnc :0 -monitor stdio -device 
q35-pcihost
    Segmentation fault (core dumped)

> I think a third case for no-user is "this device is actually an
> abstract base class" (eg arm_gic_common), which we could deal with
> by checking the TypeInfo instead.

Yes.

> Case four: "we really don't expect anybody to be trying to wire this
> up dynamically", which would apply to things like the on-cpu peripherals
> for some ARM cores. There it is really just an attempt at being friendly
> by cutting down the length of the devices list.

Yes.  Case-by-case decision, I guess.

> Speaking of friendlyness, it might be helpful if the '-devices help'
> list was somehow more structured than a single long list and if
> more devices had description text?

Oh yes.  I wanted to do that, but when Anthony started to dig up qdev, I
hastened to get out of the way.

>>>                          [and I don't think "this device
>>> can be added via the monitor but not the command line"
>>> counts as consistent or coherent...]
>>
>> no_user applies equally to -device and device_add.
>
> In the codebase as it stands, it applies only to -device.
> I agree that we should be consistent here, which we could do
> by applying Christian's patch or some variation to make device_add
> honour no_user. (Or by removing no_user altogether :-))

Actually, it appears to apply only to help now!

git-bisect blames this one:

commit 18b6dade8c0799c48f5c5e124b8c407cd5e22e96
Author: Anthony Liguori <address@hidden>
Date:   Fri Dec 9 12:08:01 2011 -0600

    qdev: refactor device creation to allow bus_info to be set only in class
    
    As we use class_init to set class members, DeviceInfo no longer holds this
    information.
    
    Signed-off-by: Anthony Liguori <address@hidden>

Review fail, testing fail :(