On 08/10/2011 07:26 PM, Richard Henderson wrote:
On 08/10/2011 09:24 AM, Richard Henderson wrote:
> Of course, as far as I can see, this variable is only used by
> the VGA devices. Surely we can arrange to pass down some address
> space during setup of the VGA?
... Which seems to be what you've done in patch 23.
So what's the point of this patch?
Some more about this:
We have a few dual (or even tripe) interface devices, of which vga is an
example. This requires a common interface when we add a memory region
to a device's bus. Let's enumerate the options:
1. just ignore the parent bus
this is cpu_register_physical_memory()
2. pass the right MemoryRegion to the device
this series
3. Integrate qdev and the memory API
introduce qdev_add_memory(), have vga.c call that.
4. Have the specialized device constructor (isa-vga.c's vga_initfn()
pass down a callback to the generaized device (vga_init_vbe()) that
wraps the bus-specific memory registration function:
static void isa_vga_add_region(void *opaque, ...)
{
ISAVGAState *d = opaque;
isa_add_region(&d->dev, ...);
}
vga_initfn()
{
...
vga_init_vbe(s, isa_vga_add_region, d);
...
}
1 and 2 are layering violations in that they don't allow bookkeeping by
the specific bus. 3 allows it to some extent but I still feel it is
wrong. 4 is correct in this sense but takes some work.
It may be that no bookkeeping is actually required and we can do 3, but
that will have to wait until qdev/memory integration. If not we'll
switch to 4.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
