From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 3/9] pci-assign: Update MSI-X MMIO to Memory API Date: Wed, 01 Feb 2012 11:04:38 +0200 Message-ID: <4F290026.8090706@redhat.com> References: <20120128142104.25681.93072.stgit@bling.home> <20120128142151.25681.74407.stgit@bling.home> <4F27E273.7060600@redhat.com> <1328044401.6937.152.camel@bling.home> <1328070142.6937.182.camel@bling.home> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, jan.kiszka@siemens.com, shashidhar.patil@gmail.com To: Alex Williamson Return-path: Received: from mx1.redhat.com ([209.132.183.28]:37154 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752673Ab2BAJEo (ORCPT ); Wed, 1 Feb 2012 04:04:44 -0500 In-Reply-To: <1328070142.6937.182.camel@bling.home> Sender: kvm-owner@vger.kernel.org List-ID: On 02/01/2012 06:22 AM, Alex Williamson wrote: > On Tue, 2012-01-31 at 14:13 -0700, Alex Williamson wrote: > > On Tue, 2012-01-31 at 14:45 +0200, Avi Kivity wrote: > > > On 01/28/2012 04:21 PM, Alex Williamson wrote: > > > > Stop using compatibility mode and at the same time fix available > > > > access sizes. The PCI spec indicates that the MSI-X table may > > > > only be accessed as DWORD or QWORD. > > > > > > > > > > > > static const MemoryRegionOps msix_mmio_ops = { > > > > - .old_mmio = { > > > > - .read = { msix_mmio_readb, msix_mmio_readw, msix_mmio_readl, }, > > > > - .write = { msix_mmio_writeb, msix_mmio_writew, msix_mmio_writel, }, > > > > - }, > > > > + .read = msix_mmio_read, > > > > + .write = msix_mmio_write, > > > > .endianness = DEVICE_NATIVE_ENDIAN, > > > > + .impl = { > > > > + .min_access_size = 4, > > > > + .max_access_size = 8, > > > > + }, > > > > }; > > > > > > > > > > .impl.min_access_size = 4 means the core will convert 1-byte I/O to > > > 4-byte I/O (using rmw if needed). That's not what we want, I think you > > > can leave it at 1 and explicitly ignore small accesses in the callbacks. > > > > > > Have you tested 8-byte I/O? This is the first user. Don't you need to > > > set .valid.max_access_size? > > > > I have not explicitly tested 8-byte I/O, figured it might just work. > > Tested, it gets split into 4-byte accesses by cpu_physical_memory_rw(). > It's trivial to add 8-byte access there and they get all the way through > the memory API intact. I expect there's a lot more than that to make > exec.c quad word access clean though. I'll leave the 8-byte access in > place in case it's a good test case. Thanks, > It doesn't make sense without .valid. -- error compiling committee.c: too many arguments to function