On 08/23/2010 12:46 AM, Avi Kivity wrote:
On 08/23/2010 12:02 AM, Anthony Liguori wrote:
On 08/22/2010 03:28 PM, Avi Kivity wrote:
On 08/20/2010 09:38 PM, Anthony Liguori wrote:
While that might be useful, I don't quite see what makes CPUs so
special
that they need to be kept out of qdev. Could be just my
ignorance, of
course.
CPUs have special relationships with things like memory in QEMU.
You can argue that a device is anything with pins and that CPUs are
just like any other chip.
We're not modelling chips! If we declare something a device, we do
it because it's functionally a device. It could be part of a chip,
or spread along multiple chips.
This is really a fundamental discussion. If you look closely at qdev
in it's current form, what it actually models is a device with GPIO
input and output whereas the GPIO input and output correspond to
qemu_irqs which really model pins that can be raised and lowered.
To me, this is insane and I'm looking to move the GPIO stuff out of
qdev. There are some devices where it makes sense to model the
interactions between pins but not for the vast majority of devices.
I agree, but I don't see the burning need or why it's "insane". Seems
like a minor design issue, can't you just ignore GPIO when you don't
need it?
In a sane object model, the expectation is that you can meaningfully
interact with base classes using the interfaces provided by the base class.
If DeviceState has a GPIO interface, you should be able to use that
interface without knowledge of the subclasses. This implies that all
subclasses implement a GPIO interface and that it can be the primary
interface to interact and connect with devices. Modelling a PCI device
based on a GPIO interface is what I was referring to as insane.
GPIO is just one way for a device to talk, same as
(*bus)_phys_memory_rw() or its netdev or its chardev or its timers.
It doesn't need to have special status within DeviceState, but it
doesn't hurt so much that I can tell.
Everything extra hurts when you're trying to move code in to a library
with unit tests covering the functionality :-)
typedef struct Timer Timer;
void timer_init(DeviceState *, void (*fn)(Timer *));
void timer_update_rel_ns(Timer *);
void timer_cancel(Timer *);
void timer_release(Timer *);
Timer objects get embedded into the device's state and container_of
can be used to get to the original device state. We could also pass
DeviceState. It's not clear to me which is better.
Not embedding the DeviceState is more generic. For example, a device
with a variable number of timers wouldn't be able to embed them in
DeviceState.
Where would they put them? Everything a device does has to be stored in
a DeviceState. It may put them in a container of some form if the
timers are dynamic.
But being able to associate timers with devices seems like a very
good idea to me because it means that you can see which devices are
registering timers.
You might also have the timers auto-cancelled and auto-destroyed on
device removal. But the whole thing seems like a minor coding issue
rather than something fundamental.
The fundamental issue is: every function (minus trivial ones) in the
device models code should have a state reference. That state reference
should inherit from a DeviceState. If this statement isn't true, then
the device has been modelled in qdev incorrectly.
Using this test, quite a lot of the "converted" devices are being
modelled incorrectly.
Regards,
Anthony Liguori
