qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH RESENT] msix: allow byte and word reading from m


From: Bernhard Kohl
Subject: [Qemu-devel] Re: [PATCH RESENT] msix: allow byte and word reading from mmio
Date: Mon, 15 Nov 2010 17:40:30 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Mnenhy/0.8.3 Thunderbird/3.0.10

Am 15.11.2010 11:42, schrieb ext Michael S. Tsirkin:
On Thu, Aug 19, 2010 at 02:56:51PM +0200, Bernhard Kohl wrote:
It's legal that the guest reads a single byte or word from mmio.
Interesting. The spec seems to say this:

        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.

I will remove the first statement from the commit message and add
something like the comment you proposed below.

I have an OS which reads single bytes and it works fine on real
hardware. Maybe this happens due to casting.
What do you mean by casting?

I did not yet locate the line of code where this happens in the guest.
As all other accesses are dword, I assume that there is some masking,
shifting or casting to a byte variable in the source code, and the
compiler generates a byte access from this.

Signed-off-by: Bernhard Kohl<address@hidden>
I guess we can merge this, but we need a comment I think since this
seems to contradict the spec, and people were sending patches relying on
this.

Does something like the following describe the situation correctly?

/* 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.
  */

Yes, that's is exactly the situation with my guest.
I will add this comment.

Does you guest also do 16 bit reads?  Are accesses at least aligned?

I will check this with my guest and the readw function disabled
in the patch. This will take some time.

---
  hw/msix.c |   20 ++++++++++++++++----
  1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index d99403a..7dac7f7 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -100,10 +100,22 @@ 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)
+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)
@@ -198,7 +210,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. */
--
1.7.2.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]