Re: [Qemu-devel] [PATCH 0/7] poison TARGET_xxx for compile once object and header file cleanups

[Blue Swirl]

On Mon, Jun 28, 2010 at 8:20 AM, Paolo Bonzini <address@hidden> wrote:
> On 06/27/2010 09:32 PM, Blue Swirl wrote:
>>
>>  From just clean up or type safety point of view, this is good stuff.
>> But from architectural point of view, we should make it more difficult
>> to use CPUState in device code, not easier. This may still be fine as
>> a temporary measure, before all CPUState references have been removed.
>
> I don't see it at all as a temporary measure.  In theory, only devices
> compiled per-target would need access to CPUState, that's true.

There is no need for any device to have access to CPUState fields. All
such accesses are either poor design or a shortcut taken for
performance. The only device where the design calls for CPUState
access is vmmouse and even there we could avoid the direct access. In
all other cases, only board physical address size and CPU endianness
matters but these should be fixed at some point by a better bus
design.

> However,
> there are conflicting goals which make an opaque CPUState preferrable.
>
> First, converting devices such as APIC to qdev requires knowledge of
> CPUState in qdev, unless you want to keep DEFINE_PROP_PTR (whose removal is
> much more interesting) or sweep it under the void* blanket.  PICs are likely
> to have CPUState members, e.g. hw/pxa2xx_pic.c (and BTW indirect access via
> functions to these fields is making emulation a little bit slower).

One way to clean up PICs would be to use qemu_irq to signal CPU
interrupts, but there are probably others.

>
> Also, for things compiled in libhw that are not part of device code,
> requiring knowledge of CPUState is absolutely not problematic and the only
> alternative loses type-safety and so it is inferior.

Completely untrue. Devices (whether part of libhw etc. or not) have no
need (from architectural point of view) to access CPUState contents.
It's clearly problematic. It's also possible to get type safety
without CPUState references in the devices.

Anyway, as I mentioned earlier, I think it's OK to apply the series
(after the problems are fixed), because of the short term gains in
cleanups and type safety. But the goal must be to make all devices
independent of the CPU model. If we never reach that goal (for
example, because of the performance issues), I guess you would not be
sad.