From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38490) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tt3jG-0004vC-NF for qemu-devel@nongnu.org; Wed, 09 Jan 2013 17:02:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tt3jD-0004aE-64 for qemu-devel@nongnu.org; Wed, 09 Jan 2013 17:02:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34500) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tt3jC-0004Zk-Tm for qemu-devel@nongnu.org; Wed, 09 Jan 2013 17:02:31 -0500 Date: Thu, 10 Jan 2013 00:06:08 +0200 From: "Michael S. Tsirkin" Message-ID: <20130109220608.GA12062@redhat.com> References: <89fdb096d0f60d59e45f78d5b4ef26a743ffbc31.1356872111.git.blauwirbel@gmail.com> <50E9C18D.1070309@suse.de> <20130107161108.GA7062@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] virtio-pci: replace byte swap hack List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org, Anthony Liguori , Andreas =?iso-8859-1?Q?F=E4rber?= , Alexander Graf On Wed, Jan 09, 2013 at 08:39:08PM +0000, Blue Swirl wrote: > On Mon, Jan 7, 2013 at 4:11 PM, Michael S. Tsirkin wro= te: > > On Sun, Jan 06, 2013 at 08:04:39PM +0000, Blue Swirl wrote: > >> On Sun, Jan 6, 2013 at 6:25 PM, Andreas F=E4rber = wrote: > >> > Am 06.01.2013 14:17, schrieb Alexander Graf: > >> >> > >> >> On 30.12.2012, at 13:55, Blue Swirl wrote: > >> >> > >> >>> Remove byte swaps by declaring the config space > >> >>> as native endian. > >> >> > >> >> This is wrong. Virtio-pci config space is split into 2 regions. O= ne with native endianness, the other one with little endian. > >> > > >> > Can that MemoryRegion be split in two? > >> > >> Yes, but unfortunately the offset for the second region depends on i= f > >> MSIX is enabled or not. PCI layer manages these bits without the > >> device seeing any changes. > >> > >> This could be handled by introducing a callback at PCI layer to info= rm > >> interested devices about changes to MSIX setup, or even generalized: > >> inform devices about changes within any set of bits specified by the > >> device. > > > > We already have a generic config_write callback and even use it in > > virtio pci: virtio_write_config. So you could simply do there: > > > > if (region size !=3D VIRTIO_PCI_CONFIG(dev)) { > > resize regions > > } > > > > We would also have to resize to the default setup on > > vm load and on vm reset. > > > > Overall not sure whether this would make the code cleaner or uglier. >=20 > I think it would be a net cleanup. Most of the ugliness comes from the > poor device architecture. >=20 > There could be (unmeasurably) small performance gains since accesses > to the two regions would be dispatched directly to the handlers. But > if the MSIX mode bit is toggled very often compared to the accesses to > config registers, it could actually cause some slow down due to > adjustment to the offset with the memory API. How often does that > happen, once per boot or more often? Are these registers accessed very > often by the guests? datapath accesses the memory a lot, while OTOH mode change happens once per boot normally, so yes, in theory it's a minor optimization. Likely not measureable by itself but if others think it's cleaner to structure code that way I sure won't object to a patch like that. > > > >> > > >> > Andreas > >> > > >> > -- > >> > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany > >> > GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N= =FCrnberg