On Wed, Jan 30, 2013 at 11:29:58AM -0600, Anthony Liguori wrote: > Andreas Färber <afaerber@xxxxxxx> writes: > > > Am 30.01.2013 17:33, schrieb Anthony Liguori: > >> Gerd Hoffmann <kraxel@xxxxxxxxxx> 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 But why like this? The distinction is artificial, isn't it? > > 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 ISASerialState is not a SerialState? Hmm but why? > 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 -- 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