From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41895 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PIOcg-00014u-SO for qemu-devel@nongnu.org; Tue, 16 Nov 2010 11:43:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PIOcf-0006gN-Ff for qemu-devel@nongnu.org; Tue, 16 Nov 2010 11:43:10 -0500 Received: from demumfd002.nsn-inter.net ([93.183.12.31]:10610) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PIOcf-0006gI-6N for qemu-devel@nongnu.org; Tue, 16 Nov 2010 11:43:09 -0500 Message-ID: <4CE2B49A.7000701@nsn.com> Date: Tue, 16 Nov 2010 17:43:06 +0100 From: Bernhard Kohl MIME-Version: 1.0 References: <20101116131452.GA23963@redhat.com> In-Reply-To: <20101116131452.GA23963@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH comment tweaked] msix: allow byte and word reading from mmio List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mst@redhat.com Cc: qemu-devel@nongnu.org Am 16.11.2010 14:14, schrieb mst@redhat.com: > Although explicitly disallowed by the PCI spec, some guests read a > single byte or word from mmio. Likely a guest OS bug, but I have an OS > which reads single bytes and it works fine on real hardware. > > Signed-off-by: Bernhard Kohl > Signed-off-by: Michael S. Tsirkin > --- > > OK so it could like something like the below. Yes, this looks good for me. > However, my question is: > do we need to put this in or can the guest simply be fixed? > I tried to locate the code where the readw occurs, but not successful. It only occurs during booting our OS, and the virtio-net driver seems to be OK. With 4 virtio NICs we have the following readw accesses, thats all! 3 accesses per NIC and the first NIC appears twice. MSI-X: msix_mmio_readw dev=0x9767c58 addr=0000000000000008 MSI-X: msix_mmio_readw dev=0x9767c58 addr=0000000000000018 MSI-X: msix_mmio_readw dev=0x9767c58 addr=0000000000000028 MSI-X: msix_mmio_readw dev=0x9772c40 addr=0000000000000008 MSI-X: msix_mmio_readw dev=0x9772c40 addr=0000000000000018 MSI-X: msix_mmio_readw dev=0x9772c40 addr=0000000000000028 MSI-X: msix_mmio_readw dev=0x977dc38 addr=0000000000000008 MSI-X: msix_mmio_readw dev=0x977dc38 addr=0000000000000018 MSI-X: msix_mmio_readw dev=0x977dc38 addr=0000000000000028 MSI-X: msix_mmio_readw dev=0x9788d90 addr=0000000000000008 MSI-X: msix_mmio_readw dev=0x9788d90 addr=0000000000000018 MSI-X: msix_mmio_readw dev=0x9788d90 addr=0000000000000028 MSI-X: msix_mmio_readw dev=0x9767c58 addr=0000000000000008 MSI-X: msix_mmio_readw dev=0x9767c58 addr=0000000000000018 MSI-X: msix_mmio_readw dev=0x9767c58 addr=0000000000000028 Is it possible to add a stack back tace printing to the readw function? > hw/msix.c | 31 +++++++++++++++++++++++++++---- > 1 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/hw/msix.c b/hw/msix.c > index f66d255..38dff59 100644 > --- a/hw/msix.c > +++ b/hw/msix.c > @@ -102,10 +102,28 @@ static uint32_t msix_mmio_readl(void *opaque, target_phys_addr_t addr) > return pci_get_long(page + offset); > } > > -static uint32_t msix_mmio_read_unallowed(void *opaque, target_phys_addr_t addr) > + /* Note: > + * PCI spec requires that all MSI-X table accesses are either DWORD or QWORD, > + * size aligned. Some guests seem to violate this rule for read accesses, > + * performing single byte reads. Since it's easy to support this, let's do so. > + * Also support 16 bit size aligned reads, just in case. > + */ > +static uint32_t msix_mmio_readw(void *opaque, target_phys_addr_t addr) > { > - fprintf(stderr, "MSI-X: only dword read is allowed!\n"); > - return 0; > + PCIDevice *dev = opaque; > + unsigned int offset = addr& (MSIX_PAGE_SIZE - 1)& ~0x1; > + void *page = dev->msix_table_page; > + > + return pci_get_word(page + offset); > +} > + > +static uint32_t msix_mmio_readb(void *opaque, target_phys_addr_t addr) > +{ > + PCIDevice *dev = opaque; > + unsigned int offset = addr& (MSIX_PAGE_SIZE - 1); > + void *page = dev->msix_table_page; > + > + return pci_get_byte(page + offset); > } > > static uint8_t msix_pending_mask(int vector) > @@ -192,6 +210,11 @@ static void msix_mmio_writel(void *opaque, target_phys_addr_t addr, > msix_handle_mask_update(dev, vector); > } > > +/* PCI spec: > + * For all accesses to MSI-X Table and MSI-X PBA fields, software must use > + * aligned full DWORD or aligned full QWORD transactions; otherwise, the result > + * is undefined. > + */ > static void msix_mmio_write_unallowed(void *opaque, target_phys_addr_t addr, > uint32_t val) > { > @@ -203,7 +226,7 @@ static CPUWriteMemoryFunc * const msix_mmio_write[] = { > }; > > static CPUReadMemoryFunc * const msix_mmio_read[] = { > - msix_mmio_read_unallowed, msix_mmio_read_unallowed, msix_mmio_readl > + msix_mmio_readb, msix_mmio_readw, msix_mmio_readl > }; > > /* Should be called from device's map method. */ >