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

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

[Pavel Fedin]

On ARM architecture ITS (Interrupt Translation Service), additionally to
normal MSI data, uses also side-band device IDs. This series prepares the
infrastructure to handling them.

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.

v1 => v2:
- Improved commit messages, more explanations
- msi_device_id() function made architecture-specific

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     |  1 +
 include/sysemu/kvm.h     | 14 ++++++++++++--
 kvm-all.c                | 15 ++++++++-------
 kvm-stub.c               |  5 +++--
 stubs/Makefile.objs      |  1 +
 stubs/msi.c              | 16 ++++++++++++++++
 target-arm/Makefile.objs |  1 +
 target-arm/msi.c         | 16 ++++++++++++++++
 12 files changed, 73 insertions(+), 23 deletions(-)
 create mode 100644 stubs/msi.c
 create mode 100644 target-arm/msi.c

-- 
2.4.4

[Qemu-devel] [PATCH v2 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 v2 2/3] hw/pci: Introduce msi_device_id()

[Pavel Fedin]

For GICv3 ITS implementation we are going to use IDs in KVM IRQ routing
code. This patch introduces reusable convenient way to obtain it from the
device pointer.

Since device ID is an architecture-specific thing, the function can be
overridden on per-architecture basis. Default stub just returns 0.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 hw/pci/msi.c             |  2 +-
 include/hw/pci/msi.h     |  1 +
 stubs/Makefile.objs      |  1 +
 stubs/msi.c              | 16 ++++++++++++++++
 target-arm/Makefile.objs |  1 +
 target-arm/msi.c         | 16 ++++++++++++++++
 6 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 stubs/msi.c
 create mode 100644 target-arm/msi.c

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..3e557d5 100644
--- a/include/hw/pci/msi.h
+++ b/include/hw/pci/msi.h
@@ -42,6 +42,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector);
 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);
+uint16_t msi_device_id(PCIDevice *dev);
 
 static inline bool msi_present(const PCIDevice *dev)
 {
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 85e4e81..d2261d1 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -22,6 +22,7 @@ stub-obj-y += migr-blocker.o
 stub-obj-y += mon-is-qmp.o
 stub-obj-y += mon-printf.o
 stub-obj-y += monitor-init.o
+stub-obj-$(CONFIG_PCI) += msi.o
 stub-obj-y += notify-event.o
 stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
 stub-obj-y += qtest.o
diff --git a/stubs/msi.c b/stubs/msi.c
new file mode 100644
index 0000000..99808c0
--- /dev/null
+++ b/stubs/msi.c
@@ -0,0 +1,16 @@
+/*
+ * QEMU architecture-specific MSI functions
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ * Written by Pavel Fedin
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "hw/pci/msi.h"
+
+uint16_t msi_device_id(PCIDevice *dev)
+{
+    return 0;
+}
diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index 9460b40..9e75c66 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -1,5 +1,6 @@
 obj-y += arm-semi.o
 obj-$(CONFIG_SOFTMMU) += machine.o
+obj-$(CONFIG_PCI) += msi.o
 obj-$(CONFIG_KVM) += kvm.o
 obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
 obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
diff --git a/target-arm/msi.c b/target-arm/msi.c
new file mode 100644
index 0000000..8da01af
--- /dev/null
+++ b/target-arm/msi.c
@@ -0,0 +1,16 @@
+/*
+ * QEMU ARM specific MSI functions
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ * Written by Pavel Fedin
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "hw/pci/msi.h"
+
+uint16_t msi_device_id(PCIDevice *dev)
+{
+    return (pci_bus_num(dev->bus) << 8) | dev->devfn;
+}
-- 
2.4.4

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

[Pavel Fedin]

In-kernel ITS emulation on ARM64 will require to supply device IDs. These
IDs can now be retrieved from the device pointer using new msi_device_id()
function.

This patch adds pci_dev pointer to KVM GSI routing functions and makes
callers passing it.

x86 architecture does not use device IDs, but hw/i386/kvm/pci-assign.c also
passes PCI device pointer instead of NULL for consistency with the rest of
the code.

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 v2 0/3] Make KVM/MSI code device-ID-aware

[Michael S. Tsirkin]

On Wed, Oct 14, 2015 at 10:53:14AM +0300, Pavel Fedin wrote:
> On ARM architecture ITS (Interrupt Translation Service), additionally to
> normal MSI data, uses also side-band device IDs. This series prepares the
> infrastructure to handling them.
> 
> 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 confused by pci changes.  It's all arm specific stuff leaking
out to pci core.

Maybe it was a mistake to put stream id in struct MemTxAttrs: there's
still nothing using it.

How about we stick PCIDevice * (or even DeviceState *) there?
Would that be enough to get rid of stream_id completely?

> v1 => v2:
> - Improved commit messages, more explanations
> - msi_device_id() function made architecture-specific
> 
> 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     |  1 +
>  include/sysemu/kvm.h     | 14 ++++++++++++--
>  kvm-all.c                | 15 ++++++++-------
>  kvm-stub.c               |  5 +++--
>  stubs/Makefile.objs      |  1 +
>  stubs/msi.c              | 16 ++++++++++++++++
>  target-arm/Makefile.objs |  1 +
>  target-arm/msi.c         | 16 ++++++++++++++++
>  12 files changed, 73 insertions(+), 23 deletions(-)
>  create mode 100644 stubs/msi.c
>  create mode 100644 target-arm/msi.c
> 
> -- 
> 2.4.4

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

[Pavel Fedin]

 Hello!

> I'm confused by pci changes.  It's all arm specific stuff leaking
> out to pci core.

 But... How else would you implement it? I tried to do this as abstract as possible. Just pass device pointer to functions which
will use it. Or, would you like to override all these functions for ARM architecture? That would be a huge code duplication. And,
still, they would need PCI device specifier, which they don't have, and what patch 0003 takes care about.

> Maybe it was a mistake to put stream id in struct MemTxAttrs: there's
> still nothing using it.

 It will be used by ITS emulation too. Take a look at RFC, ITS code injects it into the kernel together with MSI data. Also, i know
that it has something to do with SMMU too, so, if we ever implement ARM SMMU, we'll likely have to deal with them too.
 Presence of device ID pretty well emulates what the hardware does - it just sends this ID along with PCI transaction.
 We will use it as soon as we either:
a) Get kernel API approved
b) Get software GICv3 emulation from Shlomo, then i'll be able to rebase my old software-emulated ITS on top of that.

> How about we stick PCIDevice * (or even DeviceState *) there?

 "there" = where? In MemTxAttrs? Would it be good to have 64-bit pointer instead of 16-bit ID there? It is passed by value, and
current version fits into register even on 32-bit host.

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

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

[Michael S. Tsirkin]

On Wed, Oct 14, 2015 at 11:46:02AM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > I'm confused by pci changes.  It's all arm specific stuff leaking
> > out to pci core.
> 
>  But... How else would you implement it? I tried to do this as abstract as possible. Just pass device pointer to functions which
> will use it. Or, would you like to override all these functions for ARM architecture? That would be a huge code duplication. And,
> still, they would need PCI device specifier, which they don't have, and what patch 0003 takes care about.
> 
> > Maybe it was a mistake to put stream id in struct MemTxAttrs: there's
> > still nothing using it.
> 
>  It will be used by ITS emulation too. Take a look at RFC, ITS code injects it into the kernel together with MSI data. Also, i know
> that it has something to do with SMMU too, so, if we ever implement ARM SMMU, we'll likely have to deal with them too.
>  Presence of device ID pretty well emulates what the hardware does - it just sends this ID along with PCI transaction.
>  We will use it as soon as we either:
> a) Get kernel API approved
> b) Get software GICv3 emulation from Shlomo, then i'll be able to rebase my old software-emulated ITS on top of that.
> 
> > How about we stick PCIDevice * (or even DeviceState *) there?
> 
>  "there" = where? In MemTxAttrs? Would it be good to have 64-bit pointer instead of 16-bit ID there? It is passed by value, and
> current version fits into register even on 32-bit host.

OK, so we could rename stream id to requester id. That at least would
make sense at the pci level, and would make it unique per hierarchy.

But I suspect all these optimizations break later when we need to support
multiple root complex devices.


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

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

[Pavel Fedin]

 Hello!

> OK, so we could rename stream id to requester id. That at least would
> make sense at the pci level, and would make it unique per hierarchy.

 Yes, we can, i'm fine with the rename. Additionally, "requester ID" is the official term used in GICv3 architecture manual. When i
implemented it i didn't have arch manual, therefore had to invent the name.

> But I suspect all these optimizations break later when we need to support
> multiple root complex devices.

 Not necessarily. IIRC we have three bits used for other purposes in MemTxAttrs. So, we could extend our RID up to 24 bits and still
be fine. Even if we use 32 bits, our MemTxAttrs would still fit into register on 64-bit platforms, and into two registers on 32-bit
platforms. The same as MSIMessage (don't remember exact name).

 Should i post v3, including the rename, or will you just rename stream_id to requester_id yourself?

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

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

[Michael S. Tsirkin]

On Wed, Oct 14, 2015 at 02:49:14PM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > OK, so we could rename stream id to requester id. That at least would
> > make sense at the pci level, and would make it unique per hierarchy.
> 
>  Yes, we can, i'm fine with the rename. Additionally, "requester ID" is the official term used in GICv3 architecture manual. When i
> implemented it i didn't have arch manual, therefore had to invent the name.
> 
> > But I suspect all these optimizations break later when we need to support
> > multiple root complex devices.
> 
>  Not necessarily. IIRC we have three bits used for other purposes in MemTxAttrs. So, we could extend our RID up to 24 bits and still
> be fine. Even if we use 32 bits, our MemTxAttrs would still fit into register on 64-bit platforms, and into two registers on 32-bit
> platforms. The same as MSIMessage (don't remember exact name).
> 
>  Should i post v3, including the rename, or will you just rename stream_id to requester_id yourself?
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia


Pls do it.

-- 
MST