Re: [Qemu-devel] KVM call minutes for Feb 8

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/13/2011 03:00 PM, Blue Swirl wrote:
> On Sun, Feb 13, 2011 at 9:57 PM, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>    
>> On 02/13/2011 01:37 PM, Blue Swirl wrote:
>>      
>>> On Sun, Feb 13, 2011 at 5:31 PM, Anthony Liguori<anthony@codemonkey.ws>
>>>   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?
>    

The point is that it's not.

Instead of doing error checking in one call, you've gotta do error 
checking in a half dozen calls because each of the set calls can fail.

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

Vararg is designed for direct invocation.  You can still build a list of 
construction properties if you're so inclined.

Regards,

Anthony Liguori