From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: KVM call minutes 2013-01-29 - Port I/O Date: Wed, 30 Jan 2013 22:08:12 +0200 Message-ID: <20130130200812.GD6001@redhat.com> References: <871ud4gfoa.fsf@elfo.elfo> <5109065B.4060803@suse.de> <51094024.20803@redhat.com> <87vcae8wbk.fsf@codemonkey.ws> <51095049.7090407@suse.de> <87r4l260kp.fsf@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Cc: KVM devel mailing list , Juan Quintela , qemu-devel , Alexander Graf , Alon Levy , qemu-ppc , Gerd Hoffmann , =?iso-8859-1?Q?Herv=E9?= Poussineau , Andreas =?iso-8859-1?Q?F=E4rber?= , David Gibson To: Anthony Liguori Return-path: Content-Disposition: inline In-Reply-To: <87r4l260kp.fsf@codemonkey.ws> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org List-Id: kvm.vger.kernel.org On Wed, Jan 30, 2013 at 11:29:58AM -0600, Anthony Liguori wrote: > Andreas F=E4rber writes: >=20 > > Am 30.01.2013 17:33, schrieb Anthony Liguori: > >> Gerd Hoffmann writes: > >>=20 > >>>> 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 becaus= e 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. > >>=20 > >> The best way to handle this would be to remodel how we do VGA. > >>=20 > >> 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... >=20 > Nope. You can use composition: >=20 > QXLDevice is-a VGACommonState >=20 > 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-inheritenc= e > > due to arising complications, I thought. Wouldn't object if someone > > wanted to do the dirty implementation work though. ;) >=20 > There is no need for MI. >=20 > > Another such example is EHCI, with PCIDevice and SysBusDevice fronten= ds, > > 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, ...). >=20 > Yes, this is all about chipset modelling. Chipsets should derive from > device and then be embedded in the appropriate bus device. >=20 > For instance. >=20 > SerialState is-a DeviceState >=20 > 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). >=20 > Regards, >=20 > Anthony Liguori >=20 > >> The VGA accessors should be exposed as a memory region but the sub c= lass > >> ought to be responsible for actually adding it to a subregion. > >>=20 > >>> > >>> That twist makes it a bit hard to convert vga ... > >>> > >>> Anyone knows how one would do that with the memory api instead? I t= hink > >>> 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 h= ow to > >>> forward the access to the vga code though. > >>> > >>=20 > >> That should be possible with priorities, but I think it's wrong. Th= ere > >> 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 insid= e > > the Class (rather than static const) might be a short-term solution f= or > > overriding read/write handlers of a particular VGA MemoryRegion. :) > > > > Cheers, > > Andreas > > > >> This isn't really a memory API issue, it's a modeling issue. > >>=20 > >> Regards, > >>=20 > >> Anthony Liguori > >>=20 > >>> Anyone has clues / suggestions? > >>> > >>> thanks, > >>> Gerd > > > > --=20 > > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany > > GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FC= rnberg