Re: [Qemu-devel] KVM call minutes 2013-01-29 - Port I/O

[Anthony Liguori <anthony@codemonkey.ws>]

Andreas Färber <afaerber@suse.de> writes:

> Am 30.01.2013 17:33, schrieb Anthony Liguori:
>> Gerd Hoffmann <kraxel@redhat.com> writes:
>> 
>>>> hw/qxl.c:    portio_list_add(qxl_vga_port_list,
>>>> pci_address_space_io(dev), 0x3b0);
>>>> hw/vga.c:        portio_list_add(vga_port_list, address_space_io, 0x3b0);
>>>
>>> That reminds me I should solve this in a more elegant way.
>>>
>>> qxl takes over the vga io ports.  The reason it does this is because qxl
>>> switches into vga mode in case the vga ports are accessed while not in
>>> vga mode.  After doing the check (and possibly switching mode) the vga
>>> handler is called to actually handle it.
>> 
>> The best way to handle this would be to remodel how we do VGA.
>> 
>> Make VGACommonState a proper QOM object and use it as the base class for
>> QXL, CirrusVGA, QEMUVGA (std-vga), and VMwareVGA.
>
> That would require polymorphism since we already need to derive from
> PCIDevice or ISADevice respectively for interfacing with the bus...

Nope.  You can use composition:

QXLDevice is-a VGACommonState

QXLPCI is-a PCIDevice
       has-a QXLDevice

> Modern object-oriented languages have tried to avoid multi-inheritence
> due to arising complications, I thought. Wouldn't object if someone
> wanted to do the dirty implementation work though. ;)

There is no need for MI.

> Another such example is EHCI, with PCIDevice and SysBusDevice frontends,
> sharing an EHCIState struct and having helper functions operating on
> that core state only. Quite a few device share such a pattern today
> actually (serial, m48t59, ...).

Yes, this is all about chipset modelling.  Chipsets should derive from
device and then be embedded in the appropriate bus device.

For instance.

SerialState is-a DeviceState

ISASerialState is-a ISADevice, has-a SerialState
MMIOSerialState is-a SysbusDevice, has-a SerialState

This is what we're doing in practice, we just aren't modeling the
chipsets and we're open coding the relationships (often in subtley
different ways).

Regards,

Anthony Liguori

>> The VGA accessors should be exposed as a memory region but the sub class
>> ought to be responsible for actually adding it to a subregion.
>> 
>>>
>>> That twist makes it a bit hard to convert vga ...
>>>
>>> Anyone knows how one would do that with the memory api instead? I think
>>> taking over the ports is easy as the memory regions have priorities so I
>>> can simply register a region with higher priority. I have no clue how to
>>> forward the access to the vga code though.
>>>
>> 
>> That should be possible with priorities, but I think it's wrong.  There
>> aren't two VGA devices.  QXL is-a VGA device and the best way to
>> override behavior of base VGA device is through polymorphism.
>
> In this particular case QXL is-a PCI VGA device though, so we can
> decouple it from core VGA modeling. Placing the MemoryRegionOps inside
> the Class (rather than static const) might be a short-term solution for
> overriding read/write handlers of a particular VGA MemoryRegion. :)
>
> Cheers,
> Andreas
>
>> This isn't really a memory API issue, it's a modeling issue.
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>>> Anyone has clues / suggestions?
>>>
>>> thanks,
>>>   Gerd
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg