From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alex Williamson <alex.williamson@redhat.com>
Subject: Re: [PATCH 3/9] pci-assign: Update MSI-X MMIO to Memory API
Date: Tue, 31 Jan 2012 14:13:21 -0700
Message-ID: <1328044401.6937.152.camel@bling.home>
References: <20120128142104.25681.93072.stgit@bling.home>
	 <20120128142151.25681.74407.stgit@bling.home> <4F27E273.7060600@redhat.com>
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: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:62545 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753710Ab2AaVNY (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 31 Jan 2012 16:13:24 -0500
In-Reply-To: <4F27E273.7060600@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.
Hmm, I wonder if we really need to be strict enough to reject byte and
word access.  It doesn't follow the spec, but I don't know that it buys
us anything to be strict about it.  Anyway, I'll look at .valid and
respin.  Thanks,

Alex