[Qemu-devel] MSI broken?

[Qemu-devel] MSI broken?

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

  Hi,

What is the status if the recently merged MSI support?

I'm trying to use it to add msi support to the intel-hda driver (current 
wip patch attached).  Everything works fine up to the point where it 
comes to delivering the interrupt to the guest.  msi_notify pretends to 
signal the guest:

msi_notify:243 intel-hda:30 notify vector 0x0 address: 0xfee01008 data: 
0x4151

The guest never ever receives this interrupt though:

[root@localhost ~]# grep hda /proc/interrupts
  43:          0   PCI-MSI-edge      hda_intel

Ideas anyone?

thanks,
   Gerd


[-- Attachment #2: 0001-intel-hda-msi-support-wip.patch --]
[-- Type: text/plain, Size: 2975 bytes --]

>From b2ca133855b78e5b6cfc3e52220769a031485939 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed, 3 Nov 2010 10:14:48 +0100
Subject: [PATCH] intel-hda: msi support [wip]

---
 hw/intel-hda.c |   30 ++++++++++++++++++++++++++++--
 1 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 126cf54..63cccbe 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -19,6 +19,7 @@
 
 #include "hw.h"
 #include "pci.h"
+#include "msi.h"
 #include "qemu-timer.h"
 #include "audiodev.h"
 #include "intel-hda.h"
@@ -188,6 +189,7 @@ struct IntelHDAState {
 
     /* properties */
     uint32_t debug;
+    uint32_t msi;
 };
 
 struct IntelHDAReg {
@@ -268,6 +270,7 @@ static void intel_hda_update_int_sts(IntelHDAState *d)
 
 static void intel_hda_update_irq(IntelHDAState *d)
 {
+    int msi = d->msi && msi_enabled(&d->pci);
     int level;
 
     intel_hda_update_int_sts(d);
@@ -276,8 +279,15 @@ static void intel_hda_update_irq(IntelHDAState *d)
     } else {
         level = 0;
     }
-    dprint(d, 2, "%s: level %d\n", __FUNCTION__, level);
-    qemu_set_irq(d->pci.irq[0], level);
+    dprint(d, 2, "%s: level %d [%s]\n", __FUNCTION__,
+           level, msi ? "msi" : "intx");
+    if (msi) {
+        if (level) {
+            msi_notify(&d->pci, 0);
+        }
+    } else {
+        qemu_set_irq(d->pci.irq[0], level);
+    }
 }
 
 static int intel_hda_send_command(IntelHDAState *d, uint32_t verb)
@@ -1132,6 +1142,8 @@ static int intel_hda_init(PCIDevice *pci)
                                           intel_hda_mmio_write, d);
     pci_register_bar(&d->pci, 0, 0x4000, PCI_BASE_ADDRESS_SPACE_MEMORY,
                      intel_hda_map);
+    if (d->msi)
+        msi_init(&d->pci, 0, 1, true, false);
 
     hda_codec_bus_init(&d->pci.qdev, &d->codecs,
                        intel_hda_response, intel_hda_xfer);
@@ -1143,10 +1155,22 @@ static int intel_hda_exit(PCIDevice *pci)
 {
     IntelHDAState *d = DO_UPCAST(IntelHDAState, pci, pci);
 
+    if (d->msi)
+        msi_uninit(&d->pci);
     cpu_unregister_io_memory(d->mmio_addr);
     return 0;
 }
 
+static void intel_hda_write_config(PCIDevice *pci, uint32_t addr,
+                                   uint32_t val, int len)
+{
+    IntelHDAState *d = DO_UPCAST(IntelHDAState, pci, pci);
+
+    pci_default_write_config(pci, addr, val, len);
+    if (d->msi)
+        msi_write_config(pci, addr, val, len);
+}
+
 static int intel_hda_post_load(void *opaque, int version)
 {
     IntelHDAState* d = opaque;
@@ -1230,8 +1254,10 @@ static PCIDeviceInfo intel_hda_info = {
     .qdev.reset   = intel_hda_reset,
     .init         = intel_hda_init,
     .exit         = intel_hda_exit,
+    .config_write = intel_hda_write_config,
     .qdev.props   = (Property[]) {
         DEFINE_PROP_UINT32("debug", IntelHDAState, debug, 0),
+        DEFINE_PROP_UINT32("msi", IntelHDAState, msi, 0),
         DEFINE_PROP_END_OF_LIST(),
     }
 };
-- 
1.7.1

Re: [Qemu-devel] MSI broken?

[Isaku Yamahata]

On Wed, Nov 03, 2010 at 12:03:15PM +0100, Gerd Hoffmann wrote:
>  Hi,
>
> What is the status if the recently merged MSI support?
>
> I'm trying to use it to add msi support to the intel-hda driver (current  
> wip patch attached).  Everything works fine up to the point where it  
> comes to delivering the interrupt to the guest.  msi_notify pretends to  
> signal the guest:
>
> msi_notify:243 intel-hda:30 notify vector 0x0 address: 0xfee01008 data:  
> 0x4151
>
> The guest never ever receives this interrupt though:
>
> [root@localhost ~]# grep hda /proc/interrupts
>  43:          0   PCI-MSI-edge      hda_intel
>
> Ideas anyone?

Let's track it down futher.
Were acpi_mem_writel() and apic_send_msi() in hw/apic.c called or not?
-- 
yamahata

Re: [Qemu-devel] MSI broken?

[Gerd Hoffmann]

On 11/03/10 13:34, Isaku Yamahata wrote:
> On Wed, Nov 03, 2010 at 12:03:15PM +0100, Gerd Hoffmann wrote:
>>   Hi,
>>
>> What is the status if the recently merged MSI support?
>>
>> I'm trying to use it to add msi support to the intel-hda driver (current
>> wip patch attached).  Everything works fine up to the point where it
>> comes to delivering the interrupt to the guest.  msi_notify pretends to
>> signal the guest:
>>
>> msi_notify:243 intel-hda:30 notify vector 0x0 address: 0xfee01008 data:
>> 0x4151
>>
>> The guest never ever receives this interrupt though:
>>
>> [root@localhost ~]# grep hda /proc/interrupts
>>   43:          0   PCI-MSI-edge      hda_intel
>>
>> Ideas anyone?
>
> Let's track it down futher.
> Were acpi_mem_writel() and apic_send_msi() in hw/apic.c called or not?

No such function in my tree (savannah/master).

Looks like some bits needed for msi are not merged yet?

cheers,
   Gerd

Re: [Qemu-devel] MSI broken?

[Isaku Yamahata]

On Wed, Nov 03, 2010 at 01:40:59PM +0100, Gerd Hoffmann wrote:
> On 11/03/10 13:34, Isaku Yamahata wrote:
>> On Wed, Nov 03, 2010 at 12:03:15PM +0100, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>> What is the status if the recently merged MSI support?
>>>
>>> I'm trying to use it to add msi support to the intel-hda driver (current
>>> wip patch attached).  Everything works fine up to the point where it
>>> comes to delivering the interrupt to the guest.  msi_notify pretends to
>>> signal the guest:
>>>
>>> msi_notify:243 intel-hda:30 notify vector 0x0 address: 0xfee01008 data:
>>> 0x4151
>>>
>>> The guest never ever receives this interrupt though:
>>>
>>> [root@localhost ~]# grep hda /proc/interrupts
>>>   43:          0   PCI-MSI-edge      hda_intel
>>>
>>> Ideas anyone?
>>
>> Let's track it down futher.
>> Were acpi_mem_writel() and apic_send_msi() in hw/apic.c called or not?
>
> No such function in my tree (savannah/master).
>
> Looks like some bits needed for msi are not merged yet?

Sorry typo. s/acpi/apic/.

I'm looking at 7d72e76228351d18a856f1e4f5365b59d3205dc3
of git://git.savannah.nongnu.org/qemu.git


>
> cheers,
>   Gerd
>

-- 
yamahata

Re: [Qemu-devel] MSI broken?

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

   Hi,

>>> Let's track it down futher.
>>> Were acpi_mem_writel() and apic_send_msi() in hw/apic.c called or not?
>>
>> No such function in my tree (savannah/master).
>>
>> Looks like some bits needed for msi are not merged yet?
>
> Sorry typo. s/acpi/apic/.

The attached patch gives me:

msi_notify:243 intel-hda:30 notify vector 0x0 address: 0xfee01008 data: 
0x4151
apic_send_msi:775 dest 1 vector 81 dest_mode 0 trigger_mode 0 delivery 1
apic_find_dest:443 no apic found for dest 1

cheers,
   Gerd

[-- Attachment #2: debug.diff --]
[-- Type: text/plain, Size: 833 bytes --]

diff --git a/hw/apic.c b/hw/apic.c
index 63d62c7..9b3ba10 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -439,6 +439,8 @@ static int apic_find_dest(uint8_t dest)
             return i;
     }
 
+    fprintf(stderr, "%s:%d no apic found for dest %d\n",
+            __FUNCTION__, __LINE__, dest);
     return -1;
 }
 
@@ -769,6 +771,8 @@ static void apic_send_msi(target_phys_addr_t addr, uint32 data)
     uint8_t trigger_mode = (data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
     uint8_t delivery = (data >> MSI_DATA_DELIVERY_MODE_SHIFT) & 0x7;
     /* XXX: Ignore redirection hint. */
+    fprintf(stderr, "%s:%d dest %d vector %d dest_mode %d trigger_mode %d delivery %d\n",
+            __FUNCTION__, __LINE__, dest, vector, dest_mode, trigger_mode, delivery);
     apic_deliver_irq(dest, dest_mode, delivery, vector, 0, trigger_mode);
 }