[Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Pavel Fedin]

This small series is actually an extraction from vITS support RFC
(https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg07074.html).
Nobody reviewed it so far, however some bits of it could actually be
applied early. This series consists only of those parts, which do not
depend on any new unreleased kernel APIs.

Pavel Fedin (3):
  kvm: Make KVM_CAP_SIGNAL_MSI globally available
  hw/pci: Introduce msi_device_id()
  kvm: Pass PCI device pointer to MSI routing functions

 hw/i386/kvm/pci-assign.c |  9 +++++----
 hw/pci/msi.c             |  2 +-
 hw/vfio/pci.c            | 11 ++++++-----
 hw/virtio/virtio-pci.c   |  5 +++--
 include/hw/pci/msi.h     |  5 +++++
 include/sysemu/kvm.h     | 14 ++++++++++++--
 kvm-all.c                | 15 ++++++++-------
 kvm-stub.c               |  5 +++--
 8 files changed, 43 insertions(+), 23 deletions(-)

-- 
2.4.4

[Qemu-devel] [PATCH 1/3] kvm: Make KVM_CAP_SIGNAL_MSI globally available

[Pavel Fedin]

This capability is useful to determine whether we can use KVM ITS
emulation on ARM

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 include/sysemu/kvm.h |  9 +++++++++
 kvm-all.c            | 10 +++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 2a58b4d..d42c464 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -52,6 +52,7 @@ extern bool kvm_msi_via_irqfd_allowed;
 extern bool kvm_gsi_routing_allowed;
 extern bool kvm_gsi_direct_mapping;
 extern bool kvm_readonly_mem_allowed;
+extern bool kvm_direct_msi_allowed;
 
 #if defined CONFIG_KVM || !defined NEED_CPU_H
 #define kvm_enabled()           (kvm_allowed)
@@ -145,6 +146,13 @@ extern bool kvm_readonly_mem_allowed;
  */
 #define kvm_readonly_mem_enabled() (kvm_readonly_mem_allowed)
 
+/**
+ * kvm_direct_msi_enabled:
+ *
+ * Returns: true if KVM allows direct MSI injection.
+ */
+#define kvm_direct_msi_enabled() (kvm_direct_msi_allowed)
+
 #else
 #define kvm_enabled()           (0)
 #define kvm_irqchip_in_kernel() (false)
@@ -157,6 +165,7 @@ extern bool kvm_readonly_mem_allowed;
 #define kvm_gsi_routing_allowed() (false)
 #define kvm_gsi_direct_mapping() (false)
 #define kvm_readonly_mem_enabled() (false)
+#define kvm_direct_msi_enabled() (false)
 #endif
 
 struct kvm_run;
diff --git a/kvm-all.c b/kvm-all.c
index 0be4615..4931b27 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -93,7 +93,6 @@ struct KVMState
     uint32_t *used_gsi_bitmap;
     unsigned int gsi_count;
     QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
-    bool direct_msi;
 #endif
     KVMMemoryListener memory_listener;
 };
@@ -111,6 +110,7 @@ bool kvm_gsi_direct_mapping;
 bool kvm_allowed;
 bool kvm_readonly_mem_allowed;
 bool kvm_vm_attributes_allowed;
+bool kvm_direct_msi_allowed;
 
 static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_INFO(USER_MEMORY),
@@ -979,7 +979,7 @@ void kvm_init_irq_routing(KVMState *s)
     s->irq_routes = g_malloc0(sizeof(*s->irq_routes));
     s->nr_allocated_irq_routes = 0;
 
-    if (!s->direct_msi) {
+    if (!kvm_direct_msi_allowed) {
         for (i = 0; i < KVM_MSI_HASHTAB_SIZE; i++) {
             QTAILQ_INIT(&s->msi_hashtab[i]);
         }
@@ -1113,7 +1113,7 @@ static int kvm_irqchip_get_virq(KVMState *s)
      * number can succeed even though a new route entry cannot be added.
      * When this happens, flush dynamic MSI entries to free IRQ route entries.
      */
-    if (!s->direct_msi && s->irq_routes->nr == s->gsi_count) {
+    if (!kvm_direct_msi_allowed && s->irq_routes->nr == s->gsi_count) {
         kvm_flush_dynamic_msi_routes(s);
     }
 
@@ -1150,7 +1150,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
     struct kvm_msi msi;
     KVMMSIRoute *route;
 
-    if (s->direct_msi) {
+    if (kvm_direct_msi_allowed) {
         msi.address_lo = (uint32_t)msg.address;
         msi.address_hi = msg.address >> 32;
         msi.data = le32_to_cpu(msg.data);
@@ -1598,7 +1598,7 @@ static int kvm_init(MachineState *ms)
 #endif
 
 #ifdef KVM_CAP_IRQ_ROUTING
-    s->direct_msi = (kvm_check_extension(s, KVM_CAP_SIGNAL_MSI) > 0);
+    kvm_direct_msi_allowed = (kvm_check_extension(s, KVM_CAP_SIGNAL_MSI) > 0);
 #endif
 
     s->intx_set_mask = kvm_check_extension(s, KVM_CAP_PCI_2_3);
-- 
2.4.4

[Qemu-devel] [PATCH 2/3] hw/pci: Introduce msi_device_id()

[Pavel Fedin]

We will need to get device IDs in KVM IRQ routing code too. This patch
introduces reusable convenient way to obtain it from the device pointer.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 hw/pci/msi.c         | 2 +-
 include/hw/pci/msi.h | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index f9c0484..f18bb56 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -294,7 +294,7 @@ void msi_send_message(PCIDevice *dev, MSIMessage msg)
 {
     MemTxAttrs attrs = {};
 
-    attrs.stream_id = (pci_bus_num(dev->bus) << 8) | dev->devfn;
+    attrs.stream_id = msi_device_id(dev);
     address_space_stl_le(&dev->bus_master_as, msg.address, msg.data,
                          attrs, NULL);
 }
diff --git a/include/hw/pci/msi.h b/include/hw/pci/msi.h
index 50e452b..748bca1 100644
--- a/include/hw/pci/msi.h
+++ b/include/hw/pci/msi.h
@@ -43,6 +43,11 @@ void msi_send_message(PCIDevice *dev, MSIMessage msg);
 void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len);
 unsigned int msi_nr_vectors_allocated(const PCIDevice *dev);
 
+static inline uint16_t msi_device_id(PCIDevice *dev)
+{
+    return (pci_bus_num(dev->bus) << 8) | dev->devfn;
+}
+
 static inline bool msi_present(const PCIDevice *dev)
 {
     return dev->cap_present & QEMU_PCI_CAP_MSI;
-- 
2.4.4

[Qemu-devel] [PATCH 3/3] kvm: Pass PCI device pointer to MSI routing functions

[Pavel Fedin]

In-kernel ITS emulation requires to supply device IDs. These IDs can be
retrieved from the device pointer using msi_device_id() function.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 hw/i386/kvm/pci-assign.c |  9 +++++----
 hw/vfio/pci.c            | 11 ++++++-----
 hw/virtio/virtio-pci.c   |  5 +++--
 include/sysemu/kvm.h     |  5 +++--
 kvm-all.c                |  5 +++--
 kvm-stub.c               |  5 +++--
 6 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 44beee3..f9db250 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -979,7 +979,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
         MSIMessage msg = msi_get_message(pci_dev, 0);
         int virq;
 
-        virq = kvm_irqchip_add_msi_route(kvm_state, msg);
+        virq = kvm_irqchip_add_msi_route(kvm_state, msg, pci_dev);
         if (virq < 0) {
             perror("assigned_dev_update_msi: kvm_irqchip_add_msi_route");
             return;
@@ -1017,7 +1017,7 @@ static void assigned_dev_update_msi_msg(PCIDevice *pci_dev)
     }
 
     kvm_irqchip_update_msi_route(kvm_state, assigned_dev->msi_virq[0],
-                                 msi_get_message(pci_dev, 0));
+                                 msi_get_message(pci_dev, 0), pci_dev);
 }
 
 static bool assigned_dev_msix_masked(MSIXTableEntry *entry)
@@ -1083,7 +1083,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
 
         msg.address = entry->addr_lo | ((uint64_t)entry->addr_hi << 32);
         msg.data = entry->data;
-        r = kvm_irqchip_add_msi_route(kvm_state, msg);
+        r = kvm_irqchip_add_msi_route(kvm_state, msg, pci_dev);
         if (r < 0) {
             return r;
         }
@@ -1602,7 +1602,8 @@ static void assigned_dev_msix_mmio_write(void *opaque, hwaddr addr,
                 msg.data = entry->data;
 
                 ret = kvm_irqchip_update_msi_route(kvm_state,
-                                                   adev->msi_virq[i], msg);
+                                                   adev->msi_virq[i], msg,
+                                                   pci_dev);
                 if (ret) {
                     error_report("Error updating irq routing entry (%d)", ret);
                 }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index dcabb6d..8fadbcf 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -424,7 +424,7 @@ static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
         return;
     }
 
-    virq = kvm_irqchip_add_msi_route(kvm_state, *msg);
+    virq = kvm_irqchip_add_msi_route(kvm_state, *msg, &vdev->pdev);
     if (virq < 0) {
         event_notifier_cleanup(&vector->kvm_interrupt);
         return;
@@ -449,9 +449,10 @@ static void vfio_remove_kvm_msi_virq(VFIOMSIVector *vector)
     event_notifier_cleanup(&vector->kvm_interrupt);
 }
 
-static void vfio_update_kvm_msi_virq(VFIOMSIVector *vector, MSIMessage msg)
+static void vfio_update_kvm_msi_virq(VFIOMSIVector *vector, MSIMessage msg,
+                                     PCIDevice *pdev)
 {
-    kvm_irqchip_update_msi_route(kvm_state, vector->virq, msg);
+    kvm_irqchip_update_msi_route(kvm_state, vector->virq, msg, pdev);
 }
 
 static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
@@ -486,7 +487,7 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
         if (!msg) {
             vfio_remove_kvm_msi_virq(vector);
         } else {
-            vfio_update_kvm_msi_virq(vector, *msg);
+            vfio_update_kvm_msi_virq(vector, *msg, pdev);
         }
     } else {
         vfio_add_kvm_msi_virq(vdev, vector, msg, true);
@@ -760,7 +761,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
         }
 
         msg = msi_get_message(&vdev->pdev, i);
-        vfio_update_kvm_msi_virq(vector, msg);
+        vfio_update_kvm_msi_virq(vector, msg, &vdev->pdev);
     }
 }
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index e5c406d..f55dd2b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -590,7 +590,7 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
     int ret;
 
     if (irqfd->users == 0) {
-        ret = kvm_irqchip_add_msi_route(kvm_state, msg);
+        ret = kvm_irqchip_add_msi_route(kvm_state, msg, &proxy->pci_dev);
         if (ret < 0) {
             return ret;
         }
@@ -726,7 +726,8 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
     if (proxy->vector_irqfd) {
         irqfd = &proxy->vector_irqfd[vector];
         if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address) {
-            ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg);
+            ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg,
+                                               &proxy->pci_dev);
             if (ret < 0) {
                 return ret;
             }
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index d42c464..4d26571 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -447,8 +447,9 @@ static inline void cpu_clean_state(CPUState *cpu)
     }
 }
 
-int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg);
-int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg);
+int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg, PCIDevice *dev);
+int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
+                                 PCIDevice *dev);
 void kvm_irqchip_release_virq(KVMState *s, int virq);
 
 int kvm_irqchip_add_adapter_route(KVMState *s, AdapterInfo *adapter);
diff --git a/kvm-all.c b/kvm-all.c
index 4931b27..4632138 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1189,7 +1189,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
     return kvm_set_irq(s, route->kroute.gsi, 1);
 }
 
-int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
+int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg, PCIDevice *dev)
 {
     struct kvm_irq_routing_entry kroute = {};
     int virq;
@@ -1224,7 +1224,8 @@ int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
     return virq;
 }
 
-int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg)
+int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
+                                 PCIDevice *dev)
 {
     struct kvm_irq_routing_entry kroute = {};
 
diff --git a/kvm-stub.c b/kvm-stub.c
index d9ad624..08bcc32 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -115,7 +115,7 @@ int kvm_on_sigbus(int code, void *addr)
 }
 
 #ifndef CONFIG_USER_ONLY
-int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
+int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg, PCIDevice *dev)
 {
     return -ENOSYS;
 }
@@ -128,7 +128,8 @@ void kvm_irqchip_release_virq(KVMState *s, int virq)
 {
 }
 
-int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg)
+int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
+                                 PCIDevice *dev)
 {
     return -ENOSYS;
 }
-- 
2.4.4

Re: [Qemu-devel] [PATCH 2/3] hw/pci: Introduce msi_device_id()

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 03:12:01PM +0300, Pavel Fedin wrote:
> We will need to get device IDs in KVM IRQ routing code too. This patch
> introduces reusable convenient way to obtain it from the device pointer.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>

What make this specific format an msi id?
I think this is some kvm specific hack.
If true it belongs there.

> ---
>  hw/pci/msi.c         | 2 +-
>  include/hw/pci/msi.h | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/msi.c b/hw/pci/msi.c
> index f9c0484..f18bb56 100644
> --- a/hw/pci/msi.c
> +++ b/hw/pci/msi.c
> @@ -294,7 +294,7 @@ void msi_send_message(PCIDevice *dev, MSIMessage msg)
>  {
>      MemTxAttrs attrs = {};
>  
> -    attrs.stream_id = (pci_bus_num(dev->bus) << 8) | dev->devfn;
> +    attrs.stream_id = msi_device_id(dev);
>      address_space_stl_le(&dev->bus_master_as, msg.address, msg.data,
>                           attrs, NULL);
>  }
> diff --git a/include/hw/pci/msi.h b/include/hw/pci/msi.h
> index 50e452b..748bca1 100644
> --- a/include/hw/pci/msi.h
> +++ b/include/hw/pci/msi.h
> @@ -43,6 +43,11 @@ void msi_send_message(PCIDevice *dev, MSIMessage msg);
>  void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len);
>  unsigned int msi_nr_vectors_allocated(const PCIDevice *dev);
>  
> +static inline uint16_t msi_device_id(PCIDevice *dev)
> +{
> +    return (pci_bus_num(dev->bus) << 8) | dev->devfn;
> +}
> +
>  static inline bool msi_present(const PCIDevice *dev)
>  {
>      return dev->cap_present & QEMU_PCI_CAP_MSI;
> -- 
> 2.4.4

Re: [Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 03:11:59PM +0300, Pavel Fedin wrote:
> This small series is actually an extraction from vITS support RFC
> (https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg07074.html).
> Nobody reviewed it so far, however some bits of it could actually be
> applied early. This series consists only of those parts, which do not
> depend on any new unreleased kernel APIs.

I'm not at all sure we want to keep extending pci-assign with
more functionality. Why not add it to vfio instead?

> 
> Pavel Fedin (3):
>   kvm: Make KVM_CAP_SIGNAL_MSI globally available
>   hw/pci: Introduce msi_device_id()
>   kvm: Pass PCI device pointer to MSI routing functions
> 
>  hw/i386/kvm/pci-assign.c |  9 +++++----
>  hw/pci/msi.c             |  2 +-
>  hw/vfio/pci.c            | 11 ++++++-----
>  hw/virtio/virtio-pci.c   |  5 +++--
>  include/hw/pci/msi.h     |  5 +++++
>  include/sysemu/kvm.h     | 14 ++++++++++++--
>  kvm-all.c                | 15 ++++++++-------
>  kvm-stub.c               |  5 +++--
>  8 files changed, 43 insertions(+), 23 deletions(-)
> 
> -- 
> 2.4.4

Re: [Qemu-devel] [PATCH 2/3] hw/pci: Introduce msi_device_id()

[Pavel Fedin]

 Hello!

> What make this specific format an msi id?
> I think this is some kvm specific hack.

 It is actually architecture-specific. But, we don't have any other arches using it (and probably won't), so at least for now i
decided to put it there in order to keep things simpler. If this ever pops up on any other arch, we'll be able to easily move it to
target-XXX.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Pavel Fedin]

 Hello!

> I'm not at all sure we want to keep extending pci-assign with
> more functionality. Why not add it to vfio instead?

 pci-assign? What exactly do you mean?

 hw/i386/kvm/pci-assign.c is modified only because kvm_irqchip_add_msi_route() now wants pci_dev pointer. Since x86 platform doesn't
use device IDs, it could very well be NULL. Just i decided to keep it consistent with the rest of the code.
 Actually, patch 0003 is all about that - we add pci_dev pointer to KVM GSI routing functions and make callers passing it, that's
all. Currently it's not used because KVM API is not released yet. See my full vITS patchset for information on how it will be used.
And yes, the patchset is a bit obsolete, so it doesn't have msi_device_id() inline, there's copypasted calculation instead.
 What exactly do you suggest to move to vfio?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 04:49:17PM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > I'm not at all sure we want to keep extending pci-assign with
> > more functionality. Why not add it to vfio instead?
> 
>  pci-assign? What exactly do you mean?
> 
>  hw/i386/kvm/pci-assign.c is modified only because kvm_irqchip_add_msi_route() now wants pci_dev pointer. Since x86 platform doesn't
> use device IDs, it could very well be NULL. Just i decided to keep it consistent with the rest of the code.
>  Actually, patch 0003 is all about that - we add pci_dev pointer to KVM GSI routing functions and make callers passing it, that's
> all.

I see, I didn't get that. Pls make this explicit in the commit log.

> Currently it's not used because KVM API is not released yet. See my full vITS patchset for information on how it will be used.
> And yes, the patchset is a bit obsolete, so it doesn't have msi_device_id() inline, there's copypasted calculation instead.
>  What exactly do you suggest to move to vfio?

msi_device_id seems like something kvm specific.
Maybe not vfio, just out of pci. 

> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 

Re: [Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Pavel Fedin]

 Hello!

> msi_device_id seems like something kvm specific.
> Maybe not vfio, just out of pci.

 I've just replied the same to Paolo. msi_device_id is architecture-specific. Just there is no any other arch yet to use it except
ARM64, therefore it is in pci. If needed, it could be moved to target-XXX.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 06:17:55PM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > msi_device_id seems like something kvm specific.
> > Maybe not vfio, just out of pci.
> 
>  I've just replied the same to Paolo. msi_device_id is architecture-specific. Just there is no any other arch yet to use it except
> ARM64, therefore it is in pci. If needed, it could be moved to target-XXX.
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia

Pls move it.