Re: [Qemu-devel] [PATCH v2 0/7] APIC/IOAPIC cleanup

[Avi Kivity <avi@redhat.com>]

  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.

> But do we really want to model memory chipsets, a north and south 
> bridge, and long with cache hierarchies?

We do model devices within the north and south bridges.  The aggregation 
into two chips is largely meaningless from a functional point of view, 
as are cache hierarchies.

> It's the trade off between a functional simulator and a cycle accurate 
> simulator.
>

Exactly.  A cpu is a functional unit since it's hot-pluggable and has 
user-visible state.  A north bridge is not hot-pluggable and has no 
user-visible state (apart from its component devices).  A PCI device 
does not have user visible state (except as an aggregation of 
functions), but it is hot-pluggable, thus it should be modelled.

> If we wanted to add per-device locking based on putting a lock in 
> DeviceState that was acquired and released whenever you executed a 
> PIO, how would you do that today?
>
> You would convert cpu_register_ioport_* to take a DeviceState in 
> serial_init_core.  Sure, you could add a layer of indirection in 
> ISASerialDevice, but what about timers?  We would want to implement 
> device based timers to do the same thing but again, when we register 
> the timers we don't have a DeviceState.
>
> All device callbacks should be based on DeviceState * pointers which 
> means if we want to share device code between multiple interfaces (be 
> it ISA, PCI, or a SysBus device), we need to have a bus in between.

How can you do that?  Do you mean that a timer calls 
DeviceState::ops->timer(DeviceState *)?  How do you handle multiple 
timers then?

Much better to call a traditional callback which then uses 
container_of() to locate its state.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.