On Sun, Feb 13, 2011 at 5:31 PM, Anthony Liguori <anth...@codemonkey.ws> wrote:
> On 02/11/2011 12:14 PM, Blue Swirl wrote:
>>
>> On Thu, Feb 10, 2011 at 6:05 PM, Anthony Liguori<anth...@codemonkey.ws>
>> wrote:
>>
>>>
>>> On 02/10/2011 03:20 PM, Gleb Natapov wrote:
>>>
>>>>
>>>> Jugging by how well all previous conversion went we will end up with one
>>>> more way of creating devices. One legacy, another qdev and your new one.
>>>> And what is the problem with qdev again (not that I am a big qdev fan)?
>>>>
>>>>
>>>
>>> We've really been arguing about probably the most minor aspect of the
>>> problem with qdev.
>>>
>>> All I'm really saying is that we shouldn't tie device construction to a
>>> factory interface as we do with qdev.
>>>
>>> That simply means that we should be able to do:
>>>
>>> RTC *rtc_create(arg1, arg2, arg2);
>>>
>>
>> I don't see how that would help at all. Throwing qdev away and just
>> calling various functions directly, with all states exposed would be
>> like QEMU 0.9.0.
>>
>
> 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.
>>> And that a separate piece of code decides which devices are exposed
>>> through
>>> -device or device_add. Which devices are exposed is really a minor
>>> detail.
>>>
>>> That said, qdev has a number of significant limitations in my mind. The
>>> first is that the only relationship between devices is through the
>>> BusState
>>> interface.
>>>
>>
>> There's also qemu_irq for arbitrary signals.
>>
>
> 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. ;-)
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.
>>> I don't think we should even try to have a generic bus model.
>>> When you look at how badly broken PCI hotplug is current in qdev, I
>>> think
>>> this is symptomatic of this.
>>>
>>
>> And how should this be fixed? The API change would not help.
>>
>
> Just as we have bus level creation functions, we should have bus level
> hotplug interfaces.
>
>>> There's also no way in qdev to really have polymorphism. Interfaces
>>> really
>>> aren't meaningful in qdev so you have things like PCIDevice where some
>>> methods are stored in the object instead of the class dispatch table and
>>> you
>>> have overuse of static class members.
>>>
>>
>> QEMU is developed in C, not C++.
>>
>
> But we're trying to do object oriented programming in C so as long as we're
> doing that, we ought to do it right.
>
>>> And it's all unrelated to VMState.
>>>
>>
>> Right, but this has also the good side that not all device state is
>> automatically exported. If other devices would be allowed to muck with
>> a devices internal state freely, bad things could happen.
>>
>> Device reset could also use standard register definitions, shared with
>> VMState.
>>
>
> There's a way to have formally verifiable serialization/deserialization if
> we can satisfy two conditions 1) the devices rely on no global state (i.e.
> static variables) and 2) every field asssociated with a device is marshalled
> during serialization/deserialization.
>
> When we define a device, right now we say that certain state is writable
> during construction. It's not a stretch to want to have some properties
> writable during runtime. If we also had a mechanism to mark certain
> properties as read-only, but still were able to introspect them, we could
> implement serialization without having to have a second VMState definition.
>
> Compatibility will always require manipulating state, but once you have the
> state stored in a data structure, you can describe those transformations in
> a pretty high level fashion.
>
>>> And this is just the basic mechanisms of qdev. The actual implementation
>>> is
>>> worse. The use of qemu_irq as gpio in the base class and overuse of
>>> SystemBus is really quite insane.
>>>
>>
>> Maybe qemu_irq should be renamed to QEMUSignal (and I don't like
>> typedeffing pointers), otherwise it looks quite sane to me.
>>
>
> 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.
> 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.
>>> I don't think there is any device that has been improved by qdev.
>>> -device
>>> is a nice feature, but it could have been implemented without qdev.
>>>
>>
>> We have 'info qtree' which can't be implemented easily without a
>> generic device class. Avi (or who was it) sent patches to expose even
>> more device state.
>>
>> With the patches I'm going to apply, if Redhat wants to disable
>> building various devices, it can be done without #ifdeffery. This is
>> not possible without a generic factory interface.
>>
>
> 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?
