Patchwork MSI / MSIX injection for Xen HVM

login
register
mail settings
Submitter Wei Liu
Date Feb. 29, 2012, 5:21 p.m.
Message ID <1330536077.10387.57.camel@leeds.uk.xensource.com>
Download mbox | patch
Permalink /patch/143794/
State New
Headers show

Comments

Wei Liu - Feb. 29, 2012, 5:21 p.m.
Hi all

This patch adds MSI / MSIX injection for Xen HVM guest. This is not new,
six months ago we had a discussion in
http://marc.info/?l=qemu-devel&m=130639451725966&w=2


Wei.

-------8<-----
MSI / MSIX injection for Xen.

This is supposed to be used in conjunction with Xen's
hypercall interface for emualted MSI / MSIX injection.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 hw/msi.c   |    7 ++++++-
 hw/msix.c  |    8 +++++++-
 hw/xen.h   |    1 +
 xen-all.c  |    5 +++++
 xen-stub.c |    4 ++++
 5 files changed, 23 insertions(+), 2 deletions(-)
Jan Kiszka - Feb. 29, 2012, 5:47 p.m.
On 2012-02-29 18:21, Wei Liu wrote:
> Hi all
> 
> This patch adds MSI / MSIX injection for Xen HVM guest. This is not new,
> six months ago we had a discussion in
> http://marc.info/?l=qemu-devel&m=130639451725966&w=2

There are some coding style issues, please use checkpatch.pl.

Back then I voted against "if (xen_enabled())" as I was planning for a
msi injection hook that also Xen could use. That may change again, the
final MSI layer design is not settled yet. Therefore, no concerns from
that POV, this work takes longer. We can refactor the Xen hooks during
that run again.

However, you know that you miss those (uncommon) messages that are
injected via DMA? They end up directly in apic_deliver_msi (where KVM
will once pick them up as well).

Jan
Wei Liu - March 1, 2012, 10:20 a.m.
On Wed, 2012-02-29 at 17:47 +0000, Jan Kiszka wrote:
> On 2012-02-29 18:21, Wei Liu wrote:
> > Hi all
> > 
> > This patch adds MSI / MSIX injection for Xen HVM guest. This is not new,
> > six months ago we had a discussion in
> > http://marc.info/?l=qemu-devel&m=130639451725966&w=2
> 
> There are some coding style issues, please use checkpatch.pl.
> 

Ah, looks like my Emacs messed it up...

> Back then I voted against "if (xen_enabled())" as I was planning for a
> msi injection hook that also Xen could use. That may change again, the
> final MSI layer design is not settled yet. Therefore, no concerns from
> that POV, this work takes longer. We can refactor the Xen hooks during
> that run again.
> 

Right, Xen hook is relatively simple.

> However, you know that you miss those (uncommon) messages that are
> injected via DMA? They end up directly in apic_deliver_msi (where KVM
> will once pick them up as well).
> 

Thanks for pointing this out. However I cannot find apic_deliver_msi in
qemu-kvm or upstream qemu tree. I can only find apic_send_msi and
apic_deliver_irq, none of which seems to be KVM-specific. Can you please
give me some pointer on this?


Wei.
Jan Kiszka - March 1, 2012, 11:22 a.m.
On 2012-03-01 11:20, Wei Liu wrote:
>> However, you know that you miss those (uncommon) messages that are
>> injected via DMA? They end up directly in apic_deliver_msi (where KVM
>> will once pick them up as well).
>>
> 
> Thanks for pointing this out. However I cannot find apic_deliver_msi in
> qemu-kvm or upstream qemu tree. I can only find apic_send_msi and
> apic_deliver_irq, none of which seems to be KVM-specific. Can you please
> give me some pointer on this?

Yes, apic_send_msi. I was accidentally looking at my msi refactoring tree.

Jan

Patch

diff --git a/hw/msi.c b/hw/msi.c
index 5d6ceb6..b11eeac 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -20,6 +20,7 @@ 
 
 #include "msi.h"
 #include "range.h"
+#include "xen.h"
 
 /* Eventually those constants should go to Linux pci_regs.h */
 #define PCI_MSI_PENDING_32      0x10
@@ -257,7 +258,11 @@  void msi_notify(PCIDevice *dev, unsigned int vector)
                    "notify vector 0x%x"
                    " address: 0x%"PRIx64" data: 0x%"PRIx32"\n",
                    vector, address, data);
-    stl_le_phys(address, data);
+    if (xen_enabled()) {
+        xen_hvm_inject_msi(address, data);
+    } else {
+        stl_le_phys(address, data);
+    }
 }
 
 /* call this function after updating configs by pci_default_write_config(). */
diff --git a/hw/msix.c b/hw/msix.c
index 3835eaa..1ddd34e 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -19,6 +19,7 @@ 
 #include "msix.h"
 #include "pci.h"
 #include "range.h"
+#include "xen.h"
 
 #define MSIX_CAP_LENGTH 12
 
@@ -365,7 +366,12 @@  void msix_notify(PCIDevice *dev, unsigned vector)
 
     address = pci_get_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR);
     data = pci_get_long(table_entry + PCI_MSIX_ENTRY_DATA);
-    stl_le_phys(address, data);
+
+    if (xen_enabled()) {
+	    xen_hvm_inject_msi(address, data);
+    } else {
+	    stl_le_phys(address, data);
+    }
 }
 
 void msix_reset(PCIDevice *dev)
diff --git a/hw/xen.h b/hw/xen.h
index b46879c..e5926b7 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -34,6 +34,7 @@  static inline int xen_enabled(void)
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 void xen_piix3_set_irq(void *opaque, int irq_num, int level);
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
+void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
 void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
 
 qemu_irq *xen_interrupt_controller_init(void);
diff --git a/xen-all.c b/xen-all.c
index b0ed1ed..78c6df3 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -122,6 +122,11 @@  void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
     }
 }
 
+void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
+{
+    xc_hvm_inject_msi(xen_xc, xen_domid, addr, data);
+}
+
 static void xen_suspend_notifier(Notifier *notifier, void *data)
 {
     xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 3);
diff --git a/xen-stub.c b/xen-stub.c
index 9ea02d4..8ff2b79 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -29,6 +29,10 @@  void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
 {
 }
 
+void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
+{
+}
+
 void xen_cmos_set_s3_resume(void *opaque, int irq, int level)
 {
 }