Skip to contentSkip to conversation

vITS support

RFC PATCH In review Pavel Fedin 5 participants 17 messages Nov 24, 2015 – Jun 29, 2016

lore
Share & tools
lore

Pavel Fedin

author
#0 lore raw

v3 cover vITS support

This series introduces support for in-kernel GICv3 ITS emulation. It is based on kernel API which is not released yet, therefore i post it as an RFC.

Kernel patch sets which implement this functionality are:
- [PATCH v3 00/16] KVM: arm64: GICv3 ITS emulation
http://www.spinics.net/lists/kvm/msg121878.html
- [PATCH v3 0/7] KVM: arm/arm64: gsi routing support
http://www.spinics.net/lists/kvm/msg119567.html

v2 => v3:
- Really added unmigratable flag, was overlooked in v2
- Fixed checkpatch issue with initializing static variable to zero

v1 => v2:
- Added registers and reset method
- Added unmigratable flag
- Rebased on top of current master, use kvm_arch_fixup_msi_route() now

Pavel Fedin (5):
hw/intc: Implement ITS base class
kernel: Add vGICv3 ITS definitions
kvm_arm: Pass requester ID to MSI routing functions
kvm_arm: Implement support for ITS emulation by KVM arm/virt: Add ITS to the virt board

 hw/arm/virt.c                          |  47 ++++++++--
 hw/intc/Makefile.objs                  |   2 +
 hw/intc/arm_gicv3_its_common.c         | 155 +++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_its_kvm.c            |  88 +++++++++++++++++++
 include/hw/intc/arm_gicv3_its_common.h |  72 +++++++++++++++
 linux-headers/asm-arm64/kvm.h          |   1 +
 linux-headers/linux/kvm.h              |   9 +-
 target-arm/kvm.c                       |   6 ++
 target-arm/kvm_arm.h                   |  13 +++
 target-arm/machine.c                   |  16 ++++
 10 files changed, 401 insertions(+), 8 deletions(-)
 create mode 100644 hw/intc/arm_gicv3_its_common.c
 create mode 100644 hw/intc/arm_gicv3_its_kvm.c
 create mode 100644 include/hw/intc/arm_gicv3_its_common.h
Signature · 1 line
2.4.4
author
#1 lore raw

RFC PATCH v3 1/5 hw/intc: Implement ITS base class

This is the basic skeleton for both KVM and software-emulated ITS. Since we already prepare status structure, we also introduce complete VMState description. But, because we currently have no migratable implementations, we also set unmigratable flag.

Signed-off-by Pavel Fedin <[email protected]>
---
 hw/intc/Makefile.objs                  |   1 +
 hw/intc/arm_gicv3_its_common.c         | 155 +++++++++++++++++++++++++++++++++
 include/hw/intc/arm_gicv3_its_common.h |  72 +++++++++++++++
 target-arm/kvm_arm.h                   |  10 +++
 target-arm/machine.c                   |  16 ++++
 5 files changed, 254 insertions(+)
 create mode 100644 hw/intc/arm_gicv3_its_common.c
 create mode 100644 include/hw/intc/arm_gicv3_its_common.h
hw/intc/Makefile.objs +1 -0
index 004b0c2..2d6543b 100644
@@ -13,6 +13,7 @@ common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_its_common.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
hw/intc/arm_gicv3_its_common.c new file +155 -0

155 lines not on this page · raw message

include/hw/intc/arm_gicv3_its_common.h new file +72 -0

72 lines not on this page · raw message

target-arm/kvm_arm.h +10 -0
index b516041..0ec221b 100644
@@ -215,4 +215,14 @@ static inline const char *gic_class_name(void)
  */
 const char *gicv3_class_name(void);
 
+/**
+ * its_class_name
+ *
+ * Return name of ITS class to use depending on whether KVM acceleration is
+ * in use, or NULL if the chosen implementation is not available.
+ *
+ * Returns: class name to use or NULL
+ */
+const char *its_class_name(void);
+
 #endif
target-arm/machine.c +16 -0
index 36a0d15..6c59c53 100644
@@ -346,3 +346,19 @@ const char *gicv3_class_name(void)
 
     exit(1);
 }
+
+const char *its_class_name(void)
+{
+    if (kvm_irqchip_in_kernel()) {
+#ifdef TARGET_AARCH64
+        /* KVM implementation requires this capability */
+        if (kvm_direct_msi_enabled()) {
+            return "arm-its-kvm";
+        }
+#endif
+        return NULL;
+    } else {
+        /* Software emulation is not implemented yet */
+        return NULL;
+    }
+}
Signature · 1 line
2.4.4
#2 lore raw

Re: [Qemu-devel] [RFC PATCH v3 1/5] hw/intc: Implement ITS base class

Quoted 6 lines · Pavel Fedin

This is the basic skeleton for both KVM and software-emulated ITS. Since we already prepare status structure, we also introduce complete VMState description. But, because we currently have no migratable implementations, we also set unmigratable flag.

Signed-off-by Pavel Fedin <[email protected]>

This mostly looks good, I just have a few minor points:

hw/intc/arm_gicv3_its_common.c line 63

Quoted 4 lines
hw/intc/arm_gicv3_its_common.c new file +4 -0
+static uint64_t gicv3_its_trans_read(void *opaque, hwaddr offset, unsigned size)
+{
+    qemu_log_mask(LOG_GUEST_ERROR, "ITS read at offset 0x%jX\n", offset);
+    return ~0ULL;

I don't think %j is the right format string character here or below:
(we don't use it anywhere else in QEMU and the docs say it is for uintmax_t/intmax_t types).

hw/intc/arm_gicv3_its_common.c line 91

Quoted 5 lines
hw/intc/arm_gicv3_its_common.c new file +4 -0
+}
 
+static const MemoryRegionOps gicv3_its_trans_ops = {
+    .read = gicv3_its_trans_read,
+    .write_with_attrs = gicv3_its_trans_write,

Please make both read and write accessor be the _with_attrs form. I know the read accessor doesn't need the attributes, but I think it's neater to have them both the same within a given memory region.

hw/intc/arm_gicv3_its_common.c line 94

Quoted 3 lines
hw/intc/arm_gicv3_its_common.c new file +3 -0
+    .impl = {
+         .min_access_size = 4,
+         .max_access_size = 4,

The spec says that 16-bit access to bits [15:0] of GITS_TRANSLATER must be supported.

Quoted 11 lines
hw/intc/arm_gicv3_its_common.c new file +11 -0
+     },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
+
+    memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), ops, s,
+                          "control", ITS_CONTROL_SIZE);
+    memory_region_init_io(&s->iomem_its, OBJECT(s), &gicv3_its_trans_ops, s,

hw/intc/arm_gicv3_its_common.c line 115

Quoted 10 lines
hw/intc/arm_gicv3_its_common.c new file +10 -0
+                          "translation", ITS_TRANS_SIZE);
+
+    /* Our two regions are always adjacent, therefore we now combine them
+     * into a single one in order to make our users' life easier.
+     */
+    memory_region_init(&s->iomem_main, OBJECT(s), "gicv3_its", ITS_SIZE);
+    memory_region_add_subregion(&s->iomem_main, 0, &s->iomem_its_cntrl);
+    memory_region_add_subregion(&s->iomem_main, ITS_CONTROL_SIZE,
+                                &s->iomem_its);
+    sysbus_init_mmio(sbd, &s->iomem_main);

So we have two memory subregions:
* the control register page, whose ops are passed in by the subclass
* the translation register page, whose only register is implemented here by looking up the subclass and calling its send_msi method

Does that complexity gain us anything later? There doesn't really seem to be anything much actually in common here, which would suggest just having the subclasses create their own mmio region(s) (which could then just directly implement the right behaviour for GITS_TRANSLATER). Or does the emulated ITS have extra stuff that makes this worthwhile?

Quoted 25 lines
target-arm/kvm_arm.h +10 -0
@@ -215,4 +215,14 @@ static inline const char *gic_class_name(void)
  */
 const char *gicv3_class_name(void);
 
+/**
+ * its_class_name
+ *
+ * Return name of ITS class to use depending on whether KVM acceleration is
+ * in use, or NULL if the chosen implementation is not available.
+ *
+ * Returns: class name to use or NULL
+ */
+const char *its_class_name(void);
+
 #endif
target-arm/machine.c +0 -0
index 36a0d15..6c59c53 100644
@@ -346,3 +346,19 @@ const char *gicv3_class_name(void)
 
     exit(1);
 }

target-arm/machine.c const char *gicv3_class_name(void) · line 358

Quoted 10 lines
target-arm/machine.c +10 -0
+
+const char *its_class_name(void)
+{
+    if (kvm_irqchip_in_kernel()) {
+#ifdef TARGET_AARCH64
+        /* KVM implementation requires this capability */
+        if (kvm_direct_msi_enabled()) {
+            return "arm-its-kvm";
+        }
+#endif

Why is this in an #ifdef? In theory we could support the GICv3 in a 32-bit system. If it isn't supported then I'd expect kvm_direct_msi_enabled() to return false on a non-AArch64 system anyway.

target-arm/machine.c const char *gicv3_class_name(void) · line 364

Quoted 6 lines
target-arm/machine.c +6 -0
+        return NULL;
+    } else {
+        /* Software emulation is not implemented yet */
+        return NULL;
+    }
+}

This doesn't seem like it should be in machine.c, which is purely related to code for migration. I would just make it be an inline implementation in kvm-arm.h, like gic_class_name(). (I know gicv3_class_name is in machine.c, but I must have missed that it was in an odd file when that patch went through review. Sorry.)

thanks
-- PMM

author
#3 lore raw

Re: [Qemu-devel] [RFC PATCH v3 1/5] hw/intc: Implement ITS base class

Hello!

Quoted 14 lines

hw/intc/arm_gicv3_its_common.c line 115

Quoted 8 lines
hw/intc/arm_gicv3_its_common.c new file +8 -0
+    /* Our two regions are always adjacent, therefore we now combine them
+     * into a single one in order to make our users' life easier.
+     */
+    memory_region_init(&s->iomem_main, OBJECT(s), "gicv3_its", ITS_SIZE);
+    memory_region_add_subregion(&s->iomem_main, 0, &s->iomem_its_cntrl);
+    memory_region_add_subregion(&s->iomem_main, ITS_CONTROL_SIZE,
+                                &s->iomem_its);
+    sysbus_init_mmio(sbd, &s->iomem_main);

So we have two memory subregions:
* the control register page, whose ops are passed in by the subclass
* the translation register page, whose only register is implemented here by looking up the subclass and calling its send_msi method

Quoted 5 lines

Does that complexity gain us anything later? There doesn't really seem to be anything much actually in common here, which would suggest just having the subclasses create their own mmio region(s) (which could then just directly implement the right behaviour for GITS_TRANSLATER).

I started from this, but decided to move region creation into base class, because:
1. We always have this region, both for KVM and for SW implementation, and it operates in the same way.
2. We always need to do address validation, as well as have gicv3_its_trans_read() stub.
3. Also, in the next revision i'll implement 16-bit handling here.

So, i decided not to duplicate these things.

Quoted 5 lines
Quoted 5 lines
target-arm/machine.c +5 -0
+
+const char *its_class_name(void)
+{
+    if (kvm_irqchip_in_kernel()) {
+#ifdef TARGET_AARCH64
Quoted 8 lines

target-arm/machine.c const char *gicv3_class_name(void) · line 358

Quoted 5 lines
target-arm/machine.c +5 -0
+        /* KVM implementation requires this capability */
+        if (kvm_direct_msi_enabled()) {
+            return "arm-its-kvm";
+        }
+#endif

Why is this in an #ifdef? In theory we could support the GICv3 in a 32-bit system.

Only for code consistency, because "arm-its-kvm" class is compiled only for TARGET_AARCH64, because only there we have the relevant kernel API definitions. So, i did not want to refer to nonexistent class.

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

#4 lore raw

Re: [Qemu-devel] [RFC PATCH v3 1/5] hw/intc: Implement ITS base class

In reply to Pavel Fedin

hw/intc/arm_gicv3_its_common.c line 56

Quoted 12 lines · Pavel Fedin
hw/intc/arm_gicv3_its_common.c new file +12 -0
+static const VMStateDescription vmstate_its = {
+    .name = "arm_gicv3_its",
+    .pre_save = gicv3_its_pre_save,
+    .post_load = gicv3_its_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(ctlr, GICv3ITSState),
+        VMSTATE_UINT64(cbaser, GICv3ITSState),
+        VMSTATE_UINT64(cwriter, GICv3ITSState),
+        VMSTATE_UINT64(creadr, GICv3ITSState),
+        VMSTATE_UINT64_ARRAY(baser, GICv3ITSState, 8),
+        VMSTATE_END_OF_LIST()
+    }

lack a comma here.

Review

Quoted 2 lines
(quoted diff) +2 -0
+    .unmigratable = true,
+};

hw/intc/arm_gicv3_its_common.c:57:5: error: request for member ‘unmigratable’ in something not a structure or union
.unmigratable = true,
^

Signature · 1 line
Shannon
author
#5 lore raw

RFC PATCH v3 2/5 kernel: Add vGICv3 ITS definitions

In reply to Pavel Fedin

This temporary patch adds kernel API definitions. Use proper header update procedure after these features are released.

Signed-off-by Pavel Fedin <[email protected]>
---
 linux-headers/asm-arm64/kvm.h | 1 +
 linux-headers/linux/kvm.h     | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
linux-headers/asm-arm64/kvm.h +1 -0
index d3714c0..a4f3230 100644
@@ -87,6 +87,7 @@ struct kvm_regs {
 /* Supported VGICv3 address types  */
 #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
 #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
+#define KVM_VGIC_V3_ADDR_TYPE_ITS	4
 
 #define KVM_VGIC_V3_DIST_SIZE		SZ_64K
 #define KVM_VGIC_V3_REDIST_SIZE		(2 * SZ_64K)
linux-headers/linux/kvm.h +7 -2
index dcc410e..32938d7 100644
@@ -843,7 +843,10 @@ struct kvm_irq_routing_msi {
 	__u32 address_lo;
 	__u32 address_hi;
 	__u32 data;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 devid;
+	};
 };
 
 struct kvm_irq_routing_s390_adapter {

@@ -982,12 +985,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+#define KVM_MSI_VALID_DEVID	(1U << 0)
 struct kvm_msi {
 	__u32 address_lo;
 	__u32 address_hi;
 	__u32 data;
 	__u32 flags;
-	__u8  pad[16];
+	__u32 devid;
+	__u8  pad[12];
 };
 
 struct kvm_arm_device_addr {
Signature · 1 line
2.4.4
author
#6 lore raw

RFC PATCH v3 3/5 kvm_arm: Pass requester ID to MSI routing functions

In reply to Pavel Fedin

Introduce global kvm_arm_msi_use_devid flag and pass device IDs in kvm_arch_fixup_msi_route(). Device IDs are required by the ITS.

Signed-off-by Pavel Fedin <[email protected]>
---
 target-arm/kvm.c     | 6 ++++++
 target-arm/kvm_arm.h | 3 +++
 2 files changed, 9 insertions(+)
target-arm/kvm.c +6 -0
index 79ef4c6..0e46930 100644
@@ -23,6 +23,7 @@
 #include "cpu.h"
 #include "internals.h"
 #include "hw/arm/arm.h"
+#include "hw/pci/pci.h"
 #include "exec/memattrs.h"
 
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {

@@ -30,6 +31,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 };
 
 static bool cap_has_mp_state;
+bool kvm_arm_msi_use_devid;
 
 int kvm_arm_vcpu_init(CPUState *cs)
 {

@@ -607,6 +609,10 @@ int kvm_arm_vgic_probe(void)
 int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
                              uint64_t address, uint32_t data, PCIDevice *dev)
 {
+    if (kvm_arm_msi_use_devid) {
+        route->flags = KVM_MSI_VALID_DEVID;
+        route->u.msi.devid = pci_requester_id(dev);
+    }
     return 0;
 }
target-arm/kvm_arm.h +3 -0
index 0ec221b..181ff37 100644
@@ -120,6 +120,9 @@ bool write_kvmstate_to_list(ARMCPU *cpu);
 void kvm_arm_reset_vcpu(ARMCPU *cpu);
 
 #ifdef CONFIG_KVM
+
+extern bool kvm_arm_msi_use_devid;
+
 /**
  * kvm_arm_create_scratch_host_vcpu:
  * @cpus_to_try: array of QEMU_KVM_ARM_TARGET_* values (terminated with
Signature · 1 line
2.4.4
#7 lore raw

Re: [Qemu-devel] [RFC PATCH v3 3/5] kvm_arm: Pass requester ID to MSI routing functions

Quoted 30 lines · Pavel Fedin

Introduce global kvm_arm_msi_use_devid flag and pass device IDs in kvm_arch_fixup_msi_route(). Device IDs are required by the ITS.

Signed-off-by Pavel Fedin <[email protected]>
---
 target-arm/kvm.c     | 6 ++++++
 target-arm/kvm_arm.h | 3 +++
 2 files changed, 9 insertions(+)
target-arm/kvm.c +2 -0
index 79ef4c6..0e46930 100644
@@ -23,6 +23,7 @@
 #include "cpu.h"
 #include "internals.h"
 #include "hw/arm/arm.h"
+#include "hw/pci/pci.h"
 #include "exec/memattrs.h"
 
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {

@@ -30,6 +31,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 };
 
 static bool cap_has_mp_state;
+bool kvm_arm_msi_use_devid;
 
 int kvm_arm_vcpu_init(CPUState *cs)
 {

target-arm/kvm.c int kvm_arm_vgic_probe(void) · line 613

Quoted 5 lines
target-arm/kvm.c +2 -0
@@ -607,6 +609,10 @@ int kvm_arm_vgic_probe(void)
 int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
                              uint64_t address, uint32_t data, PCIDevice *dev)
 {
+    if (kvm_arm_msi_use_devid) {
+        route->flags = KVM_MSI_VALID_DEVID;

I think we should OR in this flag (it doesn't make any difference right now since all the callers to this function set the flags to 0 before the call, but conceptually I think it makes more sense).

target-arm/kvm.c int kvm_arm_vgic_probe(void) · line 615

Quoted 2 lines
target-arm/kvm.c +2 -0
+        route->u.msi.devid = pci_requester_id(dev);
+    }

Is there anything that would go wrong if we just always set the u.msi.devid and the VALID_DEVID flag? (ie do we need the kvm_arm_msi_use_devid bool?)

Quoted 18 lines
target-arm/kvm.c +0 -0
     return 0;
 }
target-arm/kvm_arm.h +3 -1
index 0ec221b..181ff37 100644
@@ -120,6 +120,9 @@ bool write_kvmstate_to_list(ARMCPU *cpu);
 void kvm_arm_reset_vcpu(ARMCPU *cpu);
 
 #ifdef CONFIG_KVM
+
+extern bool kvm_arm_msi_use_devid;
+
 /**
  * kvm_arm_create_scratch_host_vcpu:
  * @cpus_to_try: array of QEMU_KVM_ARM_TARGET_* values (terminated with
--
Quoted 2 lines

2.4.4

thanks
-- PMM

author
#8 lore raw

Re: [Qemu-devel] [RFC PATCH v3 3/5] kvm_arm: Pass requester ID to MSI routing functions

Hello!

Quoted 6 lines

target-arm/kvm.c int kvm_arm_vgic_probe(void) · line 615

Quoted 2 lines
target-arm/kvm.c +2 -0
+        route->u.msi.devid = pci_requester_id(dev);
+    }

Is there anything that would go wrong if we just always set the u.msi.devid and the VALID_DEVID flag? (ie do we need the kvm_arm_msi_use_devid bool?)

Current kernels always make sure that flags == 0, or they simply return -EINVAL. Therefore, for backwards compatibility we set the flag only if we know that we need it.

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

author
#9 lore raw

RFC PATCH v3 4/5 kvm_arm: Implement support for ITS emulation by KVM

In reply to Pavel Fedin

This patch relies on new kernel API which is not released yet.

Signed-off-by Pavel Fedin <[email protected]>
---
 hw/intc/Makefile.objs          |  1 +
 hw/intc/arm_gicv3_its_common.c |  2 +-
 hw/intc/arm_gicv3_its_kvm.c    | 88 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 hw/intc/arm_gicv3_its_kvm.c
hw/intc/Makefile.objs +1 -0
index 2d6543b..8d51111 100644
@@ -19,6 +19,7 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o
 obj-$(CONFIG_APIC) += apic.o apic_common.o
 obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
 obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_kvm.o
+obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_its_kvm.o
 obj-$(CONFIG_STELLARIS) += armv7m_nvic.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
 obj-$(CONFIG_GRLIB) += grlib_irqmp.o
hw/intc/arm_gicv3_its_common.c +1 -1
index 873d62a..c040011 100644
@@ -70,7 +70,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
     if (offset == 0x0040) {
         GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
         GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
-        int ret = c->send_msi(s, le32_to_cpu(value), attrs.stream_id);
+        int ret = c->send_msi(s, le32_to_cpu(value), attrs.requester_id);
 
         if (ret) {
             qemu_log_mask(LOG_GUEST_ERROR,
hw/intc/arm_gicv3_its_kvm.c new file +88 -0

88 lines not on this page · raw message

Signature · 1 line
2.4.4
#10 lore raw

Re: [Qemu-devel] [RFC PATCH v3 4/5] kvm_arm: Implement support for ITS emulation by KVM

Quoted 27 lines · Pavel Fedin

This patch relies on new kernel API which is not released yet.

Signed-off-by Pavel Fedin <[email protected]>
---
 hw/intc/Makefile.objs          |  1 +
 hw/intc/arm_gicv3_its_common.c |  2 +-
 hw/intc/arm_gicv3_its_kvm.c    | 88 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 hw/intc/arm_gicv3_its_kvm.c
hw/intc/Makefile.objs +1 -0
index 2d6543b..8d51111 100644
@@ -19,6 +19,7 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o
 obj-$(CONFIG_APIC) += apic.o apic_common.o
 obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
 obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_kvm.o
+obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_its_kvm.o
 obj-$(CONFIG_STELLARIS) += armv7m_nvic.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
 obj-$(CONFIG_GRLIB) += grlib_irqmp.o
hw/intc/arm_gicv3_its_common.c +0 -0
index 873d62a..c040011 100644

hw/intc/arm_gicv3_its_common.c static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset, · line 73

Quoted 5 lines
hw/intc/arm_gicv3_its_common.c +1 -1
@@ -70,7 +70,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
     if (offset == 0x0040) {
         GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
         GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
-        int ret = c->send_msi(s, le32_to_cpu(value), attrs.stream_id);
+        int ret = c->send_msi(s, le32_to_cpu(value), attrs.requester_id);

Should this hunk have been squashed into the earlier patch which added this code ?

Some of my comments on earlier patches have knock-on effects on this code if you follow them, but otherwise this code looks good.

thanks
-- PMM

author
#11 lore raw

RFC PATCH v3 5/5 arm/virt: Add ITS to the virt board

In reply to Pavel Fedin

If supported by the configuration, ITS will be added automatically.

This patch also renames v2m_phandle to msi_phandle because it's now used by both MSI implementations.

Signed-off-by Pavel Fedin <[email protected]>
---
 hw/arm/virt.c | 47 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)
hw/arm/virt.c +41 -6
index 9c6792c..2196bbe 100644
@@ -70,7 +70,7 @@ typedef struct VirtBoardInfo {
     int fdt_size;
     uint32_t clock_phandle;
     uint32_t gic_phandle;
-    uint32_t v2m_phandle;
+    uint32_t msi_phandle;
 } VirtBoardInfo;
 
 typedef struct {

@@ -351,9 +351,22 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
     }
 }
 
+static void fdt_add_its_gic_node(VirtBoardInfo *vbi)
+{
+    vbi->msi_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
+    qemu_fdt_add_subnode(vbi->fdt, "/intc/its");
+    qemu_fdt_setprop_string(vbi->fdt, "/intc/its", "compatible",
+                            "arm,gic-v3-its");
+    qemu_fdt_setprop(vbi->fdt, "/intc/its", "msi-controller", NULL, 0);
+    qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc/its", "reg",
+                                 2, vbi->memmap[VIRT_GIC_ITS].base,
+                                 2, vbi->memmap[VIRT_GIC_ITS].size);
+    qemu_fdt_setprop_cell(vbi->fdt, "/intc/its", "phandle", vbi->msi_phandle);
+}
+
 static void fdt_add_v2m_gic_node(VirtBoardInfo *vbi)
 {
-    vbi->v2m_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
+    vbi->msi_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
     qemu_fdt_add_subnode(vbi->fdt, "/intc/v2m");
     qemu_fdt_setprop_string(vbi->fdt, "/intc/v2m", "compatible",
                             "arm,gic-v2m-frame");

@@ -361,7 +374,7 @@ static void fdt_add_v2m_gic_node(VirtBoardInfo *vbi)
     qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc/v2m", "reg",
                                  2, vbi->memmap[VIRT_GIC_V2M].base,
                                  2, vbi->memmap[VIRT_GIC_V2M].size);
-    qemu_fdt_setprop_cell(vbi->fdt, "/intc/v2m", "phandle", vbi->v2m_phandle);
+    qemu_fdt_setprop_cell(vbi->fdt, "/intc/v2m", "phandle", vbi->msi_phandle);
 }
 
 static void fdt_add_gic_node(VirtBoardInfo *vbi, int type)

@@ -397,6 +410,26 @@ static void fdt_add_gic_node(VirtBoardInfo *vbi, int type)
     qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
 }
 
+static void create_its(VirtBoardInfo *vbi, DeviceState *gicdev)
+{
+    const char *itsclass = its_class_name();
+    DeviceState *dev;
+
+    if (!itsclass) {
+        /* Do nothing if not supported */
+        return;
+    }
+
+    dev = qdev_create(NULL, itsclass);
+
+    object_property_set_link(OBJECT(dev), OBJECT(gicdev), "parent-gicv3",
+                             &error_abort);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vbi->memmap[VIRT_GIC_ITS].base);
+
+    fdt_add_its_gic_node(vbi);
+}
+
 static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
 {
     int i;

@@ -480,7 +513,9 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool secure)
 
     fdt_add_gic_node(vbi, type);
 
-    if (type == 2) {
+    if (type == 3) {
+        create_its(vbi, gicdev);
+    } else {
         create_v2m(vbi, pic);
     }
 }

@@ -799,9 +834,9 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
     qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0,
                            nr_pcie_buses - 1);
 
-    if (vbi->v2m_phandle) {
+    if (vbi->msi_phandle) {
         qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent",
-                               vbi->v2m_phandle);
+                               vbi->msi_phandle);
     }
 
     qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
Signature · 1 line
2.4.4
Reviewed-by on v3 5/5
#12 lore raw

Re: [Qemu-devel] [RFC PATCH v3 5/5] arm/virt: Add ITS to the virt board

Quoted 6 lines · Pavel Fedin

If supported by the configuration, ITS will be added automatically.

This patch also renames v2m_phandle to msi_phandle because it's now used by both MSI implementations.

Signed-off-by Pavel Fedin <[email protected]>
Reviewed-by Peter Maydell <[email protected]>

We'll need to add the ACPI bindings at some point but we can do that later.

thanks
-- PMM

#13 lore raw

Re: [Qemu-devel] [RFC PATCH v3 5/5] arm/virt: Add ITS to the virt board

Quoted 11 lines · Peter Maydell
Quoted 6 lines · Pavel Fedin
Quoted 6 lines

If supported by the configuration, ITS will be added automatically.

This patch also renames v2m_phandle to msi_phandle because it's now used by both MSI implementations.

Signed-off-by Pavel Fedin <[email protected]>
Reviewed-by Peter Maydell <[email protected]>

We'll need to add the ACPI bindings at some point but we can do that later.

I can add the ACPI part based on this when this series almost settle down.

Thanks,

Signature · 1 line
Shannon
#14 lore raw
In reply to Pavel Fedin

Dear all,

Quoted 9 lines · Pavel Fedin

This series introduces support for in-kernel GICv3 ITS emulation. It is based on kernel API which is not released yet, therefore i post it as an RFC.

Kernel patch sets which implement this functionality are:
- [PATCH v3 00/16] KVM: arm64: GICv3 ITS emulation
http://www.spinics.net/lists/kvm/msg121878.html
- [PATCH v3 0/7] KVM: arm/arm64: gsi routing support
http://www.spinics.net/lists/kvm/msg119567.html

For testing KVM/ARM gsi routing and KVM ARM PCIe/MSI passthrough with GICv3 ITS I intend to work on the respin of this series. I now have access to a host featuring GICv3 ITS. Does anyone currently work on this topic or intend to do so in short term or can I proceed?

Best Regards

Eric

Quoted 32 lines

v2 => v3:
- Really added unmigratable flag, was overlooked in v2
- Fixed checkpatch issue with initializing static variable to zero

v1 => v2:
- Added registers and reset method
- Added unmigratable flag
- Rebased on top of current master, use kvm_arch_fixup_msi_route() now

Pavel Fedin (5):
hw/intc: Implement ITS base class
kernel: Add vGICv3 ITS definitions
kvm_arm: Pass requester ID to MSI routing functions
kvm_arm: Implement support for ITS emulation by KVM arm/virt: Add ITS to the virt board

 hw/arm/virt.c                          |  47 ++++++++--
 hw/intc/Makefile.objs                  |   2 +
 hw/intc/arm_gicv3_its_common.c         | 155 +++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_its_kvm.c            |  88 +++++++++++++++++++
 include/hw/intc/arm_gicv3_its_common.h |  72 +++++++++++++++
 linux-headers/asm-arm64/kvm.h          |   1 +
 linux-headers/linux/kvm.h              |   9 +-
 target-arm/kvm.c                       |   6 ++
 target-arm/kvm_arm.h                   |  13 +++
 target-arm/machine.c                   |  16 ++++
 10 files changed, 401 insertions(+), 8 deletions(-)
 create mode 100644 hw/intc/arm_gicv3_its_common.c
 create mode 100644 hw/intc/arm_gicv3_its_kvm.c
 create mode 100644 include/hw/intc/arm_gicv3_its_common.h
#15 lore raw
Quoted 13 lines · Auger Eric

Dear all,

Quoted 9 lines · Pavel Fedin

This series introduces support for in-kernel GICv3 ITS emulation. It is based on kernel API which is not released yet, therefore i post it as an RFC.

Kernel patch sets which implement this functionality are:
- [PATCH v3 00/16] KVM: arm64: GICv3 ITS emulation
http://www.spinics.net/lists/kvm/msg121878.html
- [PATCH v3 0/7] KVM: arm/arm64: gsi routing support
http://www.spinics.net/lists/kvm/msg119567.html

Quoted 5 lines

For testing KVM/ARM gsi routing and KVM ARM PCIe/MSI passthrough with GICv3 ITS I intend to work on the respin of this series. I now have access to a host featuring GICv3 ITS. Does anyone currently work on this topic or intend to do so in short term or can I proceed?

Linaro is not working on this feature yet, and we would be happy to see you pick up this work.

Thanks,
-Christoffer

#16 lore raw

Hi Christoffer,

Quoted 14 lines · Christoffer Dall
Quoted 13 lines · Auger Eric

Dear all,

Quoted 9 lines · Pavel Fedin

This series introduces support for in-kernel GICv3 ITS emulation. It is based on kernel API which is not released yet, therefore i post it as an RFC.

Kernel patch sets which implement this functionality are:
- [PATCH v3 00/16] KVM: arm64: GICv3 ITS emulation
http://www.spinics.net/lists/kvm/msg121878.html
- [PATCH v3 0/7] KVM: arm/arm64: gsi routing support
http://www.spinics.net/lists/kvm/msg119567.html

Quoted 7 lines
Quoted 5 lines

For testing KVM/ARM gsi routing and KVM ARM PCIe/MSI passthrough with GICv3 ITS I intend to work on the respin of this series. I now have access to a host featuring GICv3 ITS. Does anyone currently work on this topic or intend to do so in short term or can I proceed?

Linaro is not working on this feature yet, and we would be happy to see you pick up this work.

OK Thanks. Since I did not get any other replies I will proceed then. Adding Bharat in cc since he is interested in that series too.

Best Regards

Eric

Quoted 4 lines

Thanks,
-Christoffer