On Sun, Feb 13, 2011 at 9:57 PM, Anthony Liguori <anthony@xxxxxxxxxxxxx> wrote:
> On 02/13/2011 01:37 PM, Blue Swirl wrote:
>>
>> On Sun, Feb 13, 2011 at 5:31 PM, Anthony Liguori<anthony@xxxxxxxxxxxxx>
>> Âwrote:
>>
>>>
>>> qdev doesn't expose any state today. Âqdev properties are
>>> construction-only
>>> properties that happen to be stored in each device state.
>>>
>>> What we really need is a full property framework that includes properties
>>> with hookable getters and setters along with the ability to mark
>>> properties
>>> as construct-only, read-only, or read-write.
>>>
>>> But I think it's reasonable to expose construct-only properties as just
>>> an
>>> initfn argument.
>>>
>>
>> Sounds OK. About read-write properties, what happens if we one day
>> have extensive threading, and locks are pushed to device level? I can
>> imagine a deadlock involving one thread running in IO thread for a
>> device and another trying to access that device's properties. Maybe
>> that is not different from function call version.
>>
>
> You need hookable setters/getters that can acquire a lock and do the right
> thing. ÂIt shouldn't be able to dead lock if the locking is designed right.
>
>
>>> Yes, but qemu_irq is very restricted as it only models a signal bit of
>>> information and doesn't really have a mechanism to attach/detach in any
>>> generic way.
>>>
>>
>> Basic signals are already very useful for many purposes, since they
>> match digital logic signals in real HW. In theory, whole machines
>> could be constructed with just qemu_irq and NAND gate emulator. ;-)
>>
>
> It's not just in theory. ÂIn the C++ port of QEMU that I wrote, I
> implemented an AND, OR, and XOR gate and implemented a full 32-bit adder by
> just using a device config file.
>
> If done correctly, using referencing can be extremely powerful. ÂA full
> adder is a good example. ÂThe gates really don't have any concept of bus and
> the relationship between gates is definitely not a tree.
>
>> In the message passing IRQ discussion earlier, it was IIRC decided
>> that the one bit version would not be changed but a separate message
>> passing version would be created if ever needed.
>>
>
> C already has a message passing interface that supports type safety called
> function pointers :-)
>
> An object that implements multiple interfaces where the interface becomes
> the "message passing interface" is exactly what I've been saying we need.
> ÂIt's flexible and the compiler helps us enforce typing.
>
>>>
>>> Any interfaces of a base class should make sense even for derived
>>> classes.
>>>
>>> That means if the base class is going to expose essentially a pin-out
>>> interface, that if I have a PCIDevice and cast it to Device, I should be
>>> able to interact with the GPIO interface to interact with the PCI device.
>>> ÂPresumably, that means interfacing at the PCI signalling level. ÂThat's
>>> insane to model in QEMU :-)
>>>
>>
>> This would be doable, if we built buses from a bunch of signals, like
>> in VHDL or Verilog. It would simplify aliased MMIO addresses nicely,
>> the undecoded address pins would be ignored. I don't think it would be
>> useful, but a separate interface could be added for connecting to
>> PCIBus with just qemu_irqs.
>>
>
> Yeah, it's possible, but I don't want to spend my time doing this.
>
>>> In reality, GPIO only makes sense for a small class of simple devices
>>> where
>>> modelling the pin-out interface makes sense (like a 7-segment LCD). ÂThat
>>> suggests that GPIO should not be in the DeviceState interface but instead
>>> should be in a SimpleDevice subclass or something like that.
>>>
>>>
>>>>
>>>> Could you point to examples of SystemBus overuse?
>>>>
>>>>
>>>
>>> anthony@titi:~/git/qemu/hw$ grep qdev_create *.c | wc -l
>>> 73
>>> anthony@titi:~/git/qemu/hw$ grep 'qdev_create(NULL' *.c | wc -l
>>> 56
>>>
>>> SystemBus has become a catch-all for shallow qdev conversions. ÂWe've got
>>> Northbridges, RAM, and network devices sitting on the same bus...
>>>
>>
>> On Sparc32 I have not bothered to create a SBus bus. Now it would be
>> useful to get bootindex corrected. Most devices (even on-board IO)
>> should use SBus.
>>
>> The only other bus (MBus) would exist between CPU, IOMMU and memory.
>>
>> But SysBus fitted the need until recently.
>>
>
> A good way to judge where a device is using a bus interface correct: does
> all of it's interactions with any other part of the guest state involve
> method calls to the bus?
>
> Right now, the answer is no for just about every device in QEMU. ÂThis is
> the problem that qdev really was meant to solve and we're not really any
> further along solving it unfortunately.
>
>>> I'm not arguing against a generic factory interface, I'm arguing that it
>>> should be separate.
>>>
>>> IOW:
>>>
>>> SerialState *serial_create(int iobase, int irq, ...);
>>>
>>> static DeviceState *qdev_serial_create(QemuOpts *opts);
>>>
>>> static void serial_init(void)
>>> {
>>> Â Â qdev_register("serial", qdev_serial_create);
>>> }
>>>
>>> The key point is that when we create devices internally, we should have a
>>> C-friendly, type-safe interface to interact with. ÂThis will encourage
>>> composition and a richer device model than what we have today.
>>>
>>
>> Isn't this what we have now in most cases?
>>
>
> No. ÂThe common pattern of shallow conversion is:
>
> struct SerialState
> {
> Â Â// device state
> };
>
> struct ISASerialState
> {
> Â ISADeviceState parent;
> Â SerialState serial;
> };
>
> void serial_init(SerialState *s);
>
> void isa_serial_init(ISADevice *dev)
> {
> Â ÂISASerialState *s = DO_UPCAST(dev);
> Â Âserial_init(&s->serial);
> }
>
> The problem with this is that you cannot use serial_init() if you want to
> have the device be reflected in the device tree.
But why would serial_init() be used anymore since isa_serial_init() is as good?
> GObject takes a different approach than I'm suggesting that is equally
> valid. ÂIt supports a vararg constructor form and then provides C-friendly
> interfaces that use the vararg form. ÂFor instance:
>
> SerialState *serial_init(int iobase, int irq, ...)
> {
> Â Â return gobject_new(QEMU_SERIAL, "iobase", iobase, "irq", irq, ...,
> NULL);
> }
>
> This is not a bad solution but what I was trying to avoid in my suggestion
> is a lot of the ugliness of supporting a factory initializer. ÂIt may be a
> better approach in the long run though.
Producing varargs lists may get messy if the list is filled
differently based on some conditions. It's also not easy to do things
in between:
dev = isa_create();
qdev_prop_set_uint32(dev);
if (flag1) {
qdev_prop_set_uint32(dev);
}
if (flag2) {
int f = func();
qdev_prop_set_uint32(dev, f);
}
qdev_prop_set_uint32(dev);
qdev_init_nofail(dev);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
