[PATCH v7 03/15] irq: move IRQ routing into irq.c

[Andre Przywara]

The current IRQ routing code in x86/irq.c is mostly implementing a
generic KVM interface which other architectures may use too.
Move the code to set up an MSI route into the generic irq.c file and
guard it with the KVM_CAP_IRQ_ROUTING capability to return an error
if the kernel does not support interrupt routing.
This also removes the dummy implementations for all other
architectures and only leaves the x86 specific code in x86/irq.c.

Signed-off-by: Andre Przywara 
---
 Makefile  |  4 +--
 arm/irq.c |  9 --
 hw/pci-shmem.c|  2 ++
 include/kvm/irq.h |  5 
 irq.c | 83 +++
 mips/irq.c| 10 ---
 powerpc/irq.c | 31 -
 virtio/pci.c  | 21 +-
 x86/irq.c | 45 --
 9 files changed, 110 insertions(+), 100 deletions(-)
 delete mode 100644 arm/irq.c
 delete mode 100644 mips/irq.c
 delete mode 100644 powerpc/irq.c

diff --git a/Makefile b/Makefile
index e4a4002..8ca887f 100644
--- a/Makefile
+++ b/Makefile
@@ -138,7 +138,6 @@ ifeq ($(ARCH), powerpc)
DEFINES += -DCONFIG_PPC
OBJS+= powerpc/boot.o
OBJS+= powerpc/ioport.o
-   OBJS+= powerpc/irq.o
OBJS+= powerpc/kvm.o
OBJS+= powerpc/cpu_info.o
OBJS+= powerpc/kvm-cpu.o
@@ -153,7 +152,7 @@ ifeq ($(ARCH), powerpc)
 endif
 
 # ARM
-OBJS_ARM_COMMON:= arm/fdt.o arm/gic.o arm/ioport.o arm/irq.o \
+OBJS_ARM_COMMON:= arm/fdt.o arm/gic.o arm/ioport.o \
   arm/kvm.o arm/kvm-cpu.o arm/pci.o arm/timer.o \
   arm/pmu.o
 HDRS_ARM_COMMON:= arm/include
@@ -186,7 +185,6 @@ ifeq ($(ARCH),mips)
ARCH_INCLUDE:= mips/include
OBJS+= mips/kvm.o
OBJS+= mips/kvm-cpu.o
-   OBJS+= mips/irq.o
 endif
 ###
 
diff --git a/arm/irq.c b/arm/irq.c
deleted file mode 100644
index d8f44df..000
--- a/arm/irq.c
+++ /dev/null
@@ -1,9 +0,0 @@
-#include "kvm/irq.h"
-#include "kvm/kvm.h"
-#include "kvm/util.h"
-
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
-{
-   die(__FUNCTION__);
-   return 0;
-}
diff --git a/hw/pci-shmem.c b/hw/pci-shmem.c
index a1c5ab7..7ce98cb 100644
--- a/hw/pci-shmem.c
+++ b/hw/pci-shmem.c
@@ -136,6 +136,8 @@ int pci_shmem__get_local_irqfd(struct kvm *kvm)
 
if (pci_shmem_pci_device.msix.ctrl & 
cpu_to_le16(PCI_MSIX_FLAGS_ENABLE)) {
gsi = irq__add_msix_route(kvm, _table[0].msg);
+   if (gsi < 0)
+   return gsi;
} else {
gsi = pci_shmem_pci_device.irq_line;
}
diff --git a/include/kvm/irq.h b/include/kvm/irq.h
index 8a78e43..bb71521 100644
--- a/include/kvm/irq.h
+++ b/include/kvm/irq.h
@@ -10,11 +10,16 @@
 
 struct kvm;
 
+extern struct kvm_irq_routing *irq_routing;
+extern int next_gsi;
+
 int irq__alloc_line(void);
 int irq__get_nr_allocated_lines(void);
 
 int irq__init(struct kvm *kvm);
 int irq__exit(struct kvm *kvm);
+
+int irq__allocate_routing_entry(void);
 int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
 
 #endif
diff --git a/irq.c b/irq.c
index 71eaa05..c4b481c 100644
--- a/irq.c
+++ b/irq.c
@@ -1,7 +1,19 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "kvm/kvm.h"
 #include "kvm/irq.h"
 #include "kvm/kvm-arch.h"
 
 static u8 next_line = KVM_IRQ_OFFSET;
+static int allocated_gsis = 0;
+
+int next_gsi;
+
+struct kvm_irq_routing *irq_routing = NULL;
 
 int irq__alloc_line(void)
 {
@@ -12,3 +24,74 @@ int irq__get_nr_allocated_lines(void)
 {
return next_line - KVM_IRQ_OFFSET;
 }
+
+int irq__allocate_routing_entry(void)
+{
+   size_t table_size = sizeof(struct kvm_irq_routing);
+   int nr_entries = 0;
+
+   if (irq_routing)
+   nr_entries = irq_routing->nr;
+
+   if (nr_entries < allocated_gsis)
+   return 0;
+
+   allocated_gsis = ALIGN(nr_entries + 1, 32);
+   table_size += sizeof(struct kvm_irq_routing_entry) * allocated_gsis;
+   irq_routing = realloc(irq_routing, table_size);
+
+   if (irq_routing == NULL)
+   return ENOMEM;
+
+   irq_routing->nr = nr_entries;
+
+   return 0;
+}
+
+static bool check_for_irq_routing(struct kvm *kvm)
+{
+   static int has_irq_routing = 0;
+
+   if (has_irq_routing == 0) {
+   if (kvm__supports_extension(kvm, KVM_CAP_IRQ_ROUTING))
+   has_irq_routing = 1;
+   else
+   has_irq_routing = -1;
+   }
+
+   return has_irq_routing > 0;
+}
+
+int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
+{
+   int r;
+
+   if (!check_for_irq_routing(kvm))
+   return -ENXIO;
+
+   r = irq__allocate_routing_entry();
+   if (r)
+  

[PATCH v7 12/15] arm: setup SPI IRQ routing tables

[Andre Przywara]

Since we soon start using GSI routing on ARM platforms too, we have
to setup the initial SPI routing table. Before the first call to
KVM_SET_GSI_ROUTING, the kernel holds this table internally, but this
is overwritten with the ioctl, so we have to explicitly set it up
here.
The routing is actually not used for IRQs triggered by KVM_IRQ_LINE,
but it needs to be here anyway. We use a simple 1:1 mapping.

Signed-off-by: Andre Przywara 
---
 arm/gic.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/arm/gic.c b/arm/gic.c
index 12139fe..a0b392e 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -19,6 +19,8 @@
 #define KVM_VGIC_V3_ADDR_TYPE_REDIST 3
 #endif
 
+#define IRQCHIP_GIC 0
+
 static int gic_fd = -1;
 static u64 gic_redists_base;
 static u64 gic_redists_size;
@@ -41,6 +43,34 @@ int irqchip_parser(const struct option *opt, const char 
*arg, int unset)
return 0;
 }
 
+static int irq__routing_init(struct kvm *kvm)
+{
+   int r;
+   int irqlines = ALIGN(irq__get_nr_allocated_lines(), 32);
+
+   /*
+* This describes the default routing that the kernel uses without
+* any routing explicitly set up via KVM_SET_GSI_ROUTING. So we
+* don't need to commit these setting right now. The first actual
+* user (MSI routing) will engage these mappings then.
+*/
+   for (next_gsi = 0; next_gsi < irqlines; next_gsi++) {
+   r = irq__allocate_routing_entry();
+   if (r)
+   return r;
+
+   irq_routing->entries[irq_routing->nr++] =
+   (struct kvm_irq_routing_entry) {
+   .gsi = next_gsi,
+   .type = KVM_IRQ_ROUTING_IRQCHIP,
+   .u.irqchip.irqchip = IRQCHIP_GIC,
+   .u.irqchip.pin = next_gsi,
+   };
+   }
+
+   return 0;
+}
+
 static int gic__create_its_frame(struct kvm *kvm, u64 its_frame_addr)
 {
struct kvm_create_device its_device = {
@@ -247,6 +277,8 @@ static int gic__init_gic(struct kvm *kvm)
return ret;
}
 
+   irq__routing_init(kvm);
+
if (!ioctl(gic_fd, KVM_HAS_DEVICE_ATTR, _init_attr)) {
ret = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, _init_attr);
if (ret)
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 14/15] arm64: enable GICv3-ITS emulation

[Andre Przywara]

With everything in place for the ITS emulation add a new option to the
--irqchip parameter to allow the user to specify --irqchip=gicv3-its
to enable the ITS emulation.
This will trigger creating the FDT node and an ITS register frame to
tell the kernel we want ITS emulation in the guest.

Signed-off-by: Andre Przywara 
---
 arm/gic.c| 2 ++
 arm/include/arm-common/kvm-config-arch.h | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arm/gic.c b/arm/gic.c
index a0b392e..7fd0768 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -35,6 +35,8 @@ int irqchip_parser(const struct option *opt, const char *arg, 
int unset)
*type = IRQCHIP_GICV2;
} else if (!strcmp(arg, "gicv3")) {
*type = IRQCHIP_GICV3;
+   } else if (!strcmp(arg, "gicv3-its")) {
+   *type = IRQCHIP_GICV3_ITS;
} else {
pr_err("irqchip: unknown type \"%s\"\n", arg);
return -1;
diff --git a/arm/include/arm-common/kvm-config-arch.h 
b/arm/include/arm-common/kvm-config-arch.h
index ed626b5..b48e720 100644
--- a/arm/include/arm-common/kvm-config-arch.h
+++ b/arm/include/arm-common/kvm-config-arch.h
@@ -27,7 +27,7 @@ int irqchip_parser(const struct option *opt, const char *arg, 
int unset);
"Force virtio devices to use PCI as their default " 
\
"transport"),   
\
 OPT_CALLBACK('\0', "irqchip", &(cfg)->irqchip, 
\
-"[gicv2|gicv3]",   \
+"[gicv2|gicv3|gicv3-its]", 
\
 "Type of interrupt controller to emulate in the guest",
\
 irqchip_parser, NULL),
 
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 13/15] extend GSI IRQ routing to take a device ID

[Andre Przywara]

For ITS emulation we need the device ID along with the MSI payload
and doorbell address to identify an MSI, so we need to put it in the
GSI IRQ routing table too.
There is a per-VM capability by which the kernel signals the need for
a device ID, so check this and put the device ID into the routing
table if needed.
For PCI devices we take the bus/device/function triplet and and that
to the routing setup call.

Signed-off-by: Andre Przywara 
---
 hw/pci-shmem.c|  3 ++-
 include/kvm/irq.h |  2 +-
 irq.c | 24 ++--
 virtio/pci.c  |  6 --
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/hw/pci-shmem.c b/hw/pci-shmem.c
index 7ce98cb..512b5b0 100644
--- a/hw/pci-shmem.c
+++ b/hw/pci-shmem.c
@@ -135,7 +135,8 @@ int pci_shmem__get_local_irqfd(struct kvm *kvm)
return fd;
 
if (pci_shmem_pci_device.msix.ctrl & 
cpu_to_le16(PCI_MSIX_FLAGS_ENABLE)) {
-   gsi = irq__add_msix_route(kvm, _table[0].msg);
+   gsi = irq__add_msix_route(kvm, _table[0].msg,
+ pci_shmem_device.dev_num << 
3);
if (gsi < 0)
return gsi;
} else {
diff --git a/include/kvm/irq.h b/include/kvm/irq.h
index f35eb7e..ee059e3 100644
--- a/include/kvm/irq.h
+++ b/include/kvm/irq.h
@@ -20,7 +20,7 @@ int irq__init(struct kvm *kvm);
 int irq__exit(struct kvm *kvm);
 
 int irq__allocate_routing_entry(void);
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
+int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg, u32 device_id);
 void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg);
 
 #endif
diff --git a/irq.c b/irq.c
index 4386a70..587b91d 100644
--- a/irq.c
+++ b/irq.c
@@ -62,7 +62,21 @@ static bool check_for_irq_routing(struct kvm *kvm)
return has_irq_routing > 0;
 }
 
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
+static bool check_for_msi_devid(struct kvm *kvm)
+{
+   static int needs_devid = 0;
+
+   if (needs_devid == 0) {
+   if (kvm__supports_vm_extension(kvm, KVM_CAP_MSI_DEVID))
+   needs_devid = 1;
+   else
+   needs_devid = -1;
+   }
+
+   return needs_devid > 0;
+}
+
+int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg, u32 device_id)
 {
int r;
 
@@ -73,7 +87,7 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
if (r)
return r;
 
-   irq_routing->entries[irq_routing->nr++] =
+   irq_routing->entries[irq_routing->nr] =
(struct kvm_irq_routing_entry) {
.gsi = next_gsi,
.type = KVM_IRQ_ROUTING_MSI,
@@ -82,6 +96,12 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
.u.msi.data = msg->data,
};
 
+   if (check_for_msi_devid(kvm)) {
+   irq_routing->entries[irq_routing->nr].flags = 
KVM_MSI_VALID_DEVID;
+   irq_routing->entries[irq_routing->nr].u.msi.devid = device_id;
+   }
+   irq_routing->nr++;
+
r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
if (r)
return r;
diff --git a/virtio/pci.c b/virtio/pci.c
index e9f81f7..da9a555 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -192,7 +192,8 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, 
struct virtio_device *v
break;
 
gsi = irq__add_msix_route(kvm,
- >msix_table[vec].msg);
+ >msix_table[vec].msg,
+ vpci->dev_hdr.dev_num << 3);
if (gsi >= 0) {
vpci->config_gsi = gsi;
break;
@@ -210,7 +211,8 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, 
struct virtio_device *v
break;
 
gsi = irq__add_msix_route(kvm,
- >msix_table[vec].msg);
+ >msix_table[vec].msg,
+ vpci->dev_hdr.dev_num << 3);
if (gsi < 0) {
if (gsi == -ENXIO &&
vpci->features & VIRTIO_PCI_F_SIGNAL_MSI)
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 08/15] arm: allow creation of an MSI register frame region

[Andre Przywara]

The GICv3 ITS expects a separate 64K page to hold ITS registers.
Add a function to reserve such a page in the guest's I/O memory and
use that for the ITS vGIC type.

Signed-off-by: Andre Przywara 
---
 arm/gic.c| 63 
 arm/include/arm-common/gic.h |  1 +
 2 files changed, 64 insertions(+)

diff --git a/arm/gic.c b/arm/gic.c
index 2c1a547..3544211 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Those names are not defined for ARM (yet) */
 #ifndef KVM_VGIC_V3_ADDR_TYPE_DIST
@@ -21,6 +22,8 @@
 static int gic_fd = -1;
 static u64 gic_redists_base;
 static u64 gic_redists_size;
+static u64 gic_msi_base;
+static u64 gic_msi_size = 0;
 
 int irqchip_parser(const struct option *opt, const char *arg, int unset)
 {
@@ -38,6 +41,56 @@ int irqchip_parser(const struct option *opt, const char 
*arg, int unset)
return 0;
 }
 
+static int gic__create_its_frame(struct kvm *kvm, u64 its_frame_addr)
+{
+   struct kvm_create_device its_device = {
+   .type = KVM_DEV_TYPE_ARM_VGIC_ITS,
+   .flags  = 0,
+   };
+   struct kvm_device_attr its_attr = {
+   .group  = KVM_DEV_ARM_VGIC_GRP_ADDR,
+   .attr   = KVM_VGIC_ITS_ADDR_TYPE,
+   .addr   = (u64)(unsigned long)_frame_addr,
+   };
+   struct kvm_device_attr its_init_attr = {
+   .group  = KVM_DEV_ARM_VGIC_GRP_CTRL,
+   .attr   = KVM_DEV_ARM_VGIC_CTRL_INIT,
+   };
+   int err;
+
+   err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, _device);
+   if (err) {
+   fprintf(stderr,
+   "GICv3 ITS requested, but kernel does not support 
it.\n");
+   fprintf(stderr, "Try --irqchip=gicv3 instead\n");
+   return err;
+   }
+
+   err = ioctl(its_device.fd, KVM_HAS_DEVICE_ATTR, _attr);
+   if (err) {
+   close(its_device.fd);
+   its_device.fd = -1;
+   return err;
+   }
+
+   err = ioctl(its_device.fd, KVM_SET_DEVICE_ATTR, _attr);
+   if (err)
+   return err;
+
+   return ioctl(its_device.fd, KVM_SET_DEVICE_ATTR, _init_attr);
+}
+
+static int gic__create_msi_frame(struct kvm *kvm, enum irqchip_type type,
+u64 msi_frame_addr)
+{
+   switch (type) {
+   case IRQCHIP_GICV3_ITS:
+   return gic__create_its_frame(kvm, msi_frame_addr);
+   default:/* No MSI frame needed */
+   return 0;
+   }
+}
+
 static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 {
int err;
@@ -67,6 +120,7 @@ static int gic__create_device(struct kvm *kvm, enum 
irqchip_type type)
dist_attr.attr  = KVM_VGIC_V2_ADDR_TYPE_DIST;
break;
case IRQCHIP_GICV3:
+   case IRQCHIP_GICV3_ITS:
gic_device.type = KVM_DEV_TYPE_ARM_VGIC_V3;
dist_attr.attr  = KVM_VGIC_V3_ADDR_TYPE_DIST;
break;
@@ -82,6 +136,7 @@ static int gic__create_device(struct kvm *kvm, enum 
irqchip_type type)
case IRQCHIP_GICV2:
err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, _if_attr);
break;
+   case IRQCHIP_GICV3_ITS:
case IRQCHIP_GICV3:
err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, _attr);
break;
@@ -93,6 +148,10 @@ static int gic__create_device(struct kvm *kvm, enum 
irqchip_type type)
if (err)
goto out_err;
 
+   err = gic__create_msi_frame(kvm, type, gic_msi_base);
+   if (err)
+   goto out_err;
+
return 0;
 
 out_err:
@@ -136,9 +195,13 @@ int gic__create(struct kvm *kvm, enum irqchip_type type)
switch (type) {
case IRQCHIP_GICV2:
break;
+   case IRQCHIP_GICV3_ITS:
+   gic_msi_size = KVM_VGIC_V3_ITS_SIZE;
+   /* fall through */
case IRQCHIP_GICV3:
gic_redists_size = kvm->cfg.nrcpus * ARM_GIC_REDIST_SIZE;
gic_redists_base = ARM_GIC_DIST_BASE - gic_redists_size;
+   gic_msi_base = gic_redists_base - gic_msi_size;
break;
default:
return -ENODEV;
diff --git a/arm/include/arm-common/gic.h b/arm/include/arm-common/gic.h
index b43a180..433dd23 100644
--- a/arm/include/arm-common/gic.h
+++ b/arm/include/arm-common/gic.h
@@ -24,6 +24,7 @@
 enum irqchip_type {
IRQCHIP_GICV2,
IRQCHIP_GICV3,
+   IRQCHIP_GICV3_ITS,
 };
 
 struct kvm;
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 10/15] add kvm__check_vm_capability

[Andre Przywara]

KVM capabilities can be per-VM, in this case the ioctl should be
issued on the VM file descriptor, not on the system fd.
Since this feature is guarded by a (system) capability itself, wrap
the call into a function of its own.

Signed-off-by: Andre Przywara 
---
 include/kvm/kvm.h |  1 +
 kvm.c | 28 
 2 files changed, 29 insertions(+)

diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h
index 4a76ec2..a76a25d 100644
--- a/include/kvm/kvm.h
+++ b/include/kvm/kvm.h
@@ -129,6 +129,7 @@ static inline bool host_ptr_in_ram(struct kvm *kvm, void *p)
 }
 
 bool kvm__supports_extension(struct kvm *kvm, unsigned int extension);
+bool kvm__supports_vm_extension(struct kvm *kvm, unsigned int extension);
 
 static inline void kvm__set_thread_name(const char *name)
 {
diff --git a/kvm.c b/kvm.c
index 7fa76f7..665ed14 100644
--- a/kvm.c
+++ b/kvm.c
@@ -93,6 +93,34 @@ const char *kvm__get_dir(void)
return kvm_dir;
 }
 
+bool kvm__supports_vm_extension(struct kvm *kvm, unsigned int extension)
+{
+   static int supports_vm_ext_check = 0;
+   int ret;
+
+   switch (supports_vm_ext_check) {
+   case 0:
+   ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
+   KVM_CAP_CHECK_EXTENSION_VM);
+   if (ret <= 0) {
+   supports_vm_ext_check = -1;
+   return false;
+   }
+   supports_vm_ext_check = 1;
+   /* fall through */
+   case 1:
+   break;
+   case -1:
+   return false;
+   }
+
+   ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, extension);
+   if (ret < 0)
+   return false;
+
+   return ret;
+}
+
 bool kvm__supports_extension(struct kvm *kvm, unsigned int extension)
 {
int ret;
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 02/15] arm: use new phandle allocation functions

[Andre Przywara]

To refer to the GIC FDT node, we used to pass the GIC phandle to most
of the functions dealing with FDT nodes.
Since we now have a global phandle reference, use that to refer to the
GIC handle in various places and get rid of the now unneeded parameter
passing.

Signed-off-by: Andre Przywara 
---
 arm/aarch32/arm-cpu.c | 4 ++--
 arm/aarch64/arm-cpu.c | 5 +++--
 arm/fdt.c | 6 +++---
 arm/gic.c | 2 +-
 arm/include/arm-common/gic.h  | 2 +-
 arm/include/arm-common/kvm-cpu-arch.h | 3 +--
 arm/include/arm-common/pci.h  | 2 +-
 arm/pci.c | 3 ++-
 8 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/arm/aarch32/arm-cpu.c b/arm/aarch32/arm-cpu.c
index d8d6293..27a8e17 100644
--- a/arm/aarch32/arm-cpu.c
+++ b/arm/aarch32/arm-cpu.c
@@ -8,11 +8,11 @@
 #include 
 #include 
 
-static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
+static void generate_fdt_nodes(void *fdt, struct kvm *kvm)
 {
int timer_interrupts[4] = {13, 14, 11, 10};
 
-   gic__generate_fdt_nodes(fdt, gic_phandle, IRQCHIP_GICV2);
+   gic__generate_fdt_nodes(fdt, IRQCHIP_GICV2);
timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
 }
 
diff --git a/arm/aarch64/arm-cpu.c b/arm/aarch64/arm-cpu.c
index c21c0bb..d7572b7 100644
--- a/arm/aarch64/arm-cpu.c
+++ b/arm/aarch64/arm-cpu.c
@@ -10,10 +10,11 @@
 #include 
 #include 
 
-static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
+static void generate_fdt_nodes(void *fdt, struct kvm *kvm)
 {
int timer_interrupts[4] = {13, 14, 11, 10};
-   gic__generate_fdt_nodes(fdt, gic_phandle, kvm->cfg.arch.irqchip);
+
+   gic__generate_fdt_nodes(fdt, kvm->cfg.arch.irqchip);
timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
pmu__generate_fdt_nodes(fdt, kvm);
 }
diff --git a/arm/fdt.c b/arm/fdt.c
index 8bcfffb..f6c8a4c 100644
--- a/arm/fdt.c
+++ b/arm/fdt.c
@@ -125,7 +125,7 @@ static int setup_fdt(struct kvm *kvm)
 kvm->arch.dtb_guest_start);
void (*generate_mmio_fdt_nodes)(void *, struct device_header *,
void (*)(void *, u8, enum irq_type));
-   void (*generate_cpu_peripheral_fdt_nodes)(void *, struct kvm *, u32)
+   void (*generate_cpu_peripheral_fdt_nodes)(void *, struct kvm *)
= kvm->cpus[0]->generate_fdt_nodes;
 
/* Create new tree without a reserve map */
@@ -166,7 +166,7 @@ static int setup_fdt(struct kvm *kvm)
/* CPU and peripherals (interrupt controller, timers, etc) */
generate_cpu_nodes(fdt, kvm);
if (generate_cpu_peripheral_fdt_nodes)
-   generate_cpu_peripheral_fdt_nodes(fdt, kvm, gic_phandle);
+   generate_cpu_peripheral_fdt_nodes(fdt, kvm);
 
/* Virtio MMIO devices */
dev_hdr = device__first_dev(DEVICE_BUS_MMIO);
@@ -185,7 +185,7 @@ static int setup_fdt(struct kvm *kvm)
}
 
/* PCI host controller */
-   pci__generate_fdt_nodes(fdt, gic_phandle);
+   pci__generate_fdt_nodes(fdt);
 
/* PSCI firmware */
_FDT(fdt_begin_node(fdt, "psci"));
diff --git a/arm/gic.c b/arm/gic.c
index b60437e..2c1a547 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -194,7 +194,7 @@ static int gic__init_gic(struct kvm *kvm)
 }
 late_init(gic__init_gic)
 
-void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
+void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type)
 {
const char *compatible;
u64 reg_prop[] = {
diff --git a/arm/include/arm-common/gic.h b/arm/include/arm-common/gic.h
index 4fde5ac..b43a180 100644
--- a/arm/include/arm-common/gic.h
+++ b/arm/include/arm-common/gic.h
@@ -30,6 +30,6 @@ struct kvm;
 
 int gic__alloc_irqnum(void);
 int gic__create(struct kvm *kvm, enum irqchip_type type);
-void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type);
+void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type);
 
 #endif /* ARM_COMMON__GIC_H */
diff --git a/arm/include/arm-common/kvm-cpu-arch.h 
b/arm/include/arm-common/kvm-cpu-arch.h
index 8a6a6e7..923d2c4 100644
--- a/arm/include/arm-common/kvm-cpu-arch.h
+++ b/arm/include/arm-common/kvm-cpu-arch.h
@@ -25,8 +25,7 @@ struct kvm_cpu {
 
struct kvm_coalesced_mmio_ring  *ring;
 
-   void(*generate_fdt_nodes)(void *fdt, struct kvm* kvm,
- u32 gic_phandle);
+   void(*generate_fdt_nodes)(void *fdt, struct kvm* kvm);
 };
 
 struct kvm_arm_target {
diff --git a/arm/include/arm-common/pci.h b/arm/include/arm-common/pci.h
index ee87725..9008a0e 100644
--- a/arm/include/arm-common/pci.h
+++ b/arm/include/arm-common/pci.h
@@ -1,6 +1,6 @@
 #ifndef ARM_COMMON__PCI_H
 #define ARM_COMMON__PCI_H
 
-void 

[PATCH v7 00/15] kvmtool: arm: ITS emulation and GSI routing support

[Andre Przywara]

Hi,

this series teaches kvmtool how to support KVM's ITS emulation. Also
(as this is somewhat related and has been co-developed) it enables GSI
routing for ARM/ARM64, which allows IRQFDs to be used, for instance
to use vhost_net. At the moment this is dependent on the guest
using the ITS emulation, but GICv2M support may be added at later time.

The first 5 patches are generic fixes and refactoring to pave the
road for the rest of the patches. Most importantly patch 3/15 pulls
the GSI routing code from x86 into generic code.
The following 6 patches add ITS emulation support. They reserve and
register the required ITS register frame and populate a DT node with
the necessary data. Also the patches add the device ID to the
KVM_SIGNAL_MSI ioctl.
Patches 12/15 and 13/15 enable IRQ GSI routing for ARM/ARM64.
This is needed to use IRQFDs, which is a prerequisite for vhost
functionality, for instance. The code sets up the (dummy) SPI
routing table and adds the device ID to the routing entry.
Patch 14/15 enables the guest ITS support by extending the existing
--irqchip= parameter to allow "--irqchip=gicv3-its".
The final patch adds a workaround the avoid breaking ARM(32)
compilation, which lacks some symbol defines now used in the (shared)
ARM GIC handling code.

The ITS part relates to the latest kernel ITS emulation series, the
IRQ routing code to Eric's respective series [1].
This code can also be found in my kvmtool git repository [2].

Cheers,
Andre.

[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2016-July/020950.html
[2] git://linux-arm.org/kvmtool.git (branch: its/v7)
http://www.linux-arm.org/git?p=kvmtool.git;a=log;h=refs/heads/its/v7

Andre Przywara (15):
  FDT: introduce global phandle allocation
  arm: use new phandle allocation functions
  irq: move IRQ routing into irq.c
  MSI-X: update GSI routing after changed MSI-X configuration
  virtio: fix endianness check for vhost support
  PCI: Only allocate IRQ routing entry when available
  TEMPORARY: update public headers for GICv3 ITS emulation
  arm: allow creation of an MSI register frame region
  arm: FDT: create MSI controller DT node
  add kvm__check_vm_capability
  PCI: inject PCI device ID on MSI injection
  arm: setup SPI IRQ routing tables
  extend GSI IRQ routing to take a device ID
  arm64: enable GICv3-ITS emulation
  arm: gic: allow 32-bit compilation

 Makefile |   5 +-
 arm/aarch32/arm-cpu.c|   4 +-
 arm/aarch64/arm-cpu.c|   5 +-
 arm/aarch64/include/asm/kvm.h|   2 +
 arm/fdt.c|   8 +-
 arm/gic.c| 132 +-
 arm/include/arm-common/gic.h |   3 +-
 arm/include/arm-common/kvm-config-arch.h |   2 +-
 arm/include/arm-common/kvm-cpu-arch.h|   3 +-
 arm/include/arm-common/pci.h |   2 +-
 arm/irq.c|   9 ---
 arm/pci.c|  13 ++-
 hw/pci-shmem.c   |   5 +-
 include/kvm/fdt.h|  10 +--
 include/kvm/irq.h|   8 +-
 include/kvm/kvm.h|   1 +
 include/kvm/virtio.h |   9 ++-
 include/linux/kvm.h  |  13 ++-
 irq.c| 134 +++
 kvm-fdt.c|  26 ++
 kvm.c|  28 +++
 mips/irq.c   |  10 ---
 powerpc/irq.c|  31 ---
 virtio/net.c |   2 +-
 virtio/pci.c |  87 +---
 x86/include/asm/kvm.h|   6 +-
 x86/irq.c|  45 ++-
 27 files changed, 466 insertions(+), 137 deletions(-)
 delete mode 100644 arm/irq.c
 create mode 100644 kvm-fdt.c
 delete mode 100644 mips/irq.c
 delete mode 100644 powerpc/irq.c

-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 05/15] virtio: fix endianness check for vhost support

[Andre Przywara]

Currently we deny any VHOST_* functionality if the architecture
supports guests with different endianness than the host. Most of the
time even on those architectures the endianness of guest and host are
the same, though, so we are denying the glory of VHOST needlessly.
Switch from compile time determination to a run time scheme, which
takes the actual endianness of the guest into account.
For this we change the semantics of VIRTIO_ENDIAN_HOST to return the
actual endianness of the host (the endianness of kvmtool at compile
time, really). The actual check in vhost_net now compares this against
the guest endianness.
This enables vhost support on ARM and ARM64.

Signed-off-by: Andre Przywara 
---
 include/kvm/virtio.h | 9 +++--
 virtio/net.c | 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 768ee96..66530fd 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -17,10 +17,15 @@
 #define VIRTIO_PCI_O_CONFIG0
 #define VIRTIO_PCI_O_MSIX  1
 
-#define VIRTIO_ENDIAN_HOST 0
 #define VIRTIO_ENDIAN_LE   (1 << 0)
 #define VIRTIO_ENDIAN_BE   (1 << 1)
 
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define VIRTIO_ENDIAN_HOST VIRTIO_ENDIAN_LE
+#else
+#define VIRTIO_ENDIAN_HOST VIRTIO_ENDIAN_BE
+#endif
+
 struct virt_queue {
struct vringvring;
u32 pfn;
@@ -40,7 +45,7 @@ struct virt_queue {
 #define VIRTIO_RING_ENDIAN VIRTIO_ENDIAN_HOST
 #endif
 
-#if (VIRTIO_RING_ENDIAN & (VIRTIO_ENDIAN_LE | VIRTIO_ENDIAN_BE))
+#if VIRTIO_RING_ENDIAN != VIRTIO_ENDIAN_HOST
 
 static inline __u16 __virtio_g2h_u16(u16 endian, __u16 val)
 {
diff --git a/virtio/net.c b/virtio/net.c
index 6d1be65..e94e37a 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -531,7 +531,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 
page_size, u32 align,
}
 
if (queue->endian != VIRTIO_ENDIAN_HOST)
-   die_perror("VHOST requires VIRTIO_ENDIAN_HOST");
+   die_perror("VHOST requires the same endianness in guest and 
host");
 
state.num = queue->vring.num;
r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_NUM, );
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 04/15] MSI-X: update GSI routing after changed MSI-X configuration

[Andre Przywara]

When we set up GSI routing to map MSIs to KVM's GSI numbers, we
write the current device's MSI setup into the kernel routing table.
However the device driver in the guest can use PCI configuration space
accesses to change the MSI configuration (address and/or payload data).
Whenever this happens after we have setup the routing table already,
we must amend the previously sent data.
So when MSI-X PCI config space accesses write address or payload,
find the associated GSI number and the matching routing table entry
and update the kernel routing table (only if the data has changed).

This fixes vhost-net, where the queue's IRQFD was setup before the
MSI vectors.

Signed-off-by: Andre Przywara 
---
 include/kvm/irq.h |  1 +
 irq.c | 31 +++
 virtio/pci.c  | 36 +---
 3 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/include/kvm/irq.h b/include/kvm/irq.h
index bb71521..f35eb7e 100644
--- a/include/kvm/irq.h
+++ b/include/kvm/irq.h
@@ -21,5 +21,6 @@ int irq__exit(struct kvm *kvm);
 
 int irq__allocate_routing_entry(void);
 int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
+void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg);
 
 #endif
diff --git a/irq.c b/irq.c
index c4b481c..4386a70 100644
--- a/irq.c
+++ b/irq.c
@@ -89,6 +89,37 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
return next_gsi++;
 }
 
+static bool update_data(u32 *ptr, u32 newdata)
+{
+   if (*ptr == newdata)
+   return false;
+
+   *ptr = newdata;
+   return true;
+}
+
+void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg)
+{
+   struct kvm_irq_routing_msi *entry;
+   unsigned int i;
+   bool changed;
+
+   for (i = 0; i < irq_routing->nr; i++)
+   if (gsi == irq_routing->entries[i].gsi)
+   break;
+   if (i == irq_routing->nr)
+   return;
+
+   entry = _routing->entries[i].u.msi;
+
+   changed  = update_data(>address_hi, msg->address_hi);
+   changed |= update_data(>address_lo, msg->address_lo);
+   changed |= update_data(>data, msg->data);
+
+   if (changed)
+   ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
+}
+
 int __attribute__((weak)) irq__exit(struct kvm *kvm)
 {
free(irq_routing);
diff --git a/virtio/pci.c b/virtio/pci.c
index 072e5b7..b3b4aac 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -152,6 +152,30 @@ static bool virtio_pci__io_in(struct ioport *ioport, 
struct kvm_cpu *vcpu, u16 p
return ret;
 }
 
+static void update_msix_map(struct virtio_pci *vpci,
+   struct msix_table *msix_entry, u32 vecnum)
+{
+   u32 gsi, i;
+
+   /* Find the GSI number used for that vector */
+   if (vecnum == vpci->config_vector) {
+   gsi = vpci->config_gsi;
+   } else {
+   for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++)
+   if (vpci->vq_vector[i] == vecnum)
+   break;
+   if (i == VIRTIO_PCI_MAX_VQ)
+   return;
+   gsi = vpci->gsis[i];
+   }
+
+   if (gsi == 0)
+   return;
+
+   msix_entry = _entry[vecnum];
+   irq__update_msix_route(vpci->kvm, gsi, _entry->msg);
+}
+
 static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device 
*vdev, u16 port,
void *data, int size, int offset)
 {
@@ -270,10 +294,16 @@ static void virtio_pci__msix_mmio_callback(struct kvm_cpu 
*vcpu,
offset  = vpci->msix_io_block;
}
 
-   if (is_write)
-   memcpy(table + addr - offset, data, len);
-   else
+   if (!is_write) {
memcpy(data, table + addr - offset, len);
+   return;
+   }
+
+   memcpy(table + addr - offset, data, len);
+
+   /* Did we just update the address or payload? */
+   if (addr % 0x10 < 0xc)
+   update_msix_map(vpci, table, (addr - offset) / 16);
 }
 
 static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, 
int vec)
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 11/15] PCI: inject PCI device ID on MSI injection

[Andre Przywara]

The ITS emulation requires a unique device ID to be passed along the
MSI payload when kvmtool wants to trigger an MSI in the guest.
According to the proposed changes to the interface add the PCI
bus/device/function triple to the structure passed with the ioctl.
Check the respective capability before actually adding the device ID
to the kvm_msi struct.

Signed-off-by: Andre Przywara 
---
 virtio/pci.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/virtio/pci.c b/virtio/pci.c
index 604b9ec..e9f81f7 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -318,14 +318,28 @@ static void virtio_pci__msix_mmio_callback(struct kvm_cpu 
*vcpu,
update_msix_map(vpci, table, (addr - offset) / 16);
 }
 
-static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, 
int vec)
+static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci,
+  int vec)
 {
+   static int needs_devid = 0;
struct kvm_msi msi = {
.address_lo = vpci->msix_table[vec].msg.address_lo,
.address_hi = vpci->msix_table[vec].msg.address_hi,
.data = vpci->msix_table[vec].msg.data,
};
 
+   if (needs_devid == 0) {
+   if (kvm__supports_vm_extension(kvm, KVM_CAP_MSI_DEVID))
+   needs_devid = 1;
+   else
+   needs_devid = -1;
+   }
+
+   if (needs_devid > 0) {
+   msi.flags = KVM_MSI_VALID_DEVID;
+   msi.devid = vpci->dev_hdr.dev_num << 3;
+   }
+
ioctl(kvm->vm_fd, KVM_SIGNAL_MSI, );
 }
 
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 09/15] arm: FDT: create MSI controller DT node

[Andre Przywara]

The ARM GICv3 ITS requires a separate device tree node to describe
the ITS. Add this as a child to the GIC interrupt controller node
to let a guest discover and use the ITS if the user requests it.
Since we now need to specify #address-cells for the GIC node, we
have to add two zeroes to the interrupt map to match that.

Signed-off-by: Andre Przywara 
---
 arm/gic.c | 23 ++-
 arm/pci.c | 12 ++--
 include/kvm/fdt.h |  2 +-
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/arm/gic.c b/arm/gic.c
index 3544211..12139fe 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -259,7 +259,8 @@ late_init(gic__init_gic)
 
 void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type)
 {
-   const char *compatible;
+   const char *compatible, *msi_compatible = NULL;
+   u64 msi_prop[2];
u64 reg_prop[] = {
cpu_to_fdt64(ARM_GIC_DIST_BASE), 
cpu_to_fdt64(ARM_GIC_DIST_SIZE),
0, 0,   /* to be filled */
@@ -271,6 +272,9 @@ void gic__generate_fdt_nodes(void *fdt, enum irqchip_type 
type)
reg_prop[2] = cpu_to_fdt64(ARM_GIC_CPUI_BASE);
reg_prop[3] = cpu_to_fdt64(ARM_GIC_CPUI_SIZE);
break;
+   case IRQCHIP_GICV3_ITS:
+   msi_compatible = "arm,gic-v3-its";
+   /* fall-through */
case IRQCHIP_GICV3:
compatible = "arm,gic-v3";
reg_prop[2] = cpu_to_fdt64(gic_redists_base);
@@ -286,6 +290,23 @@ void gic__generate_fdt_nodes(void *fdt, enum irqchip_type 
type)
_FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
_FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
+   _FDT(fdt_property_cell(fdt, "#address-cells", 2));
+   _FDT(fdt_property_cell(fdt, "#size-cells", 2));
+
+   if (msi_compatible) {
+   _FDT(fdt_property(fdt, "ranges", NULL, 0));
+
+   _FDT(fdt_begin_node(fdt, "msic"));
+   _FDT(fdt_property_string(fdt, "compatible", msi_compatible));
+   _FDT(fdt_property(fdt, "msi-controller", NULL, 0));
+   _FDT(fdt_property_cell(fdt, "phandle",
+  fdt__get_phandle(PHANDLE_MSI)));
+   msi_prop[0] = cpu_to_fdt64(gic_msi_base);
+   msi_prop[1] = cpu_to_fdt64(gic_msi_size);
+   _FDT(fdt_property(fdt, "reg", msi_prop, sizeof(msi_prop)));
+   _FDT(fdt_end_node(fdt));
+   }
+
_FDT(fdt_end_node(fdt));
 }
 
diff --git a/arm/pci.c b/arm/pci.c
index 9630657..104349a 100644
--- a/arm/pci.c
+++ b/arm/pci.c
@@ -18,6 +18,8 @@ struct of_gic_irq {
 struct of_interrupt_map_entry {
struct of_pci_irq_mask  pci_irq_mask;
u32 gic_phandle;
+   u32 gic_addr_hi;
+   u32 gic_addr_lo;
struct of_gic_irq   gic_irq;
 } __attribute__((packed));
 
@@ -26,7 +28,7 @@ void pci__generate_fdt_nodes(void *fdt)
struct device_header *dev_hdr;
struct of_interrupt_map_entry irq_map[OF_PCI_IRQ_MAP_MAX];
unsigned nentries = 0;
-   u32 gic_phandle = fdt__get_phandle(PHANDLE_GIC);
+   u32 phandle;
/* Bus range */
u32 bus_range[] = { cpu_to_fdt32(0), cpu_to_fdt32(1), };
/* Configuration Space */
@@ -65,7 +67,11 @@ void pci__generate_fdt_nodes(void *fdt)
_FDT(fdt_property(fdt, "bus-range", bus_range, sizeof(bus_range)));
_FDT(fdt_property(fdt, "reg", _reg_prop, sizeof(cfg_reg_prop)));
_FDT(fdt_property(fdt, "ranges", ranges, sizeof(ranges)));
+   phandle = fdt__get_phandle(PHANDLE_MSI);
+   if (FDT_IS_VALID_PHANDLE(phandle))
+   _FDT(fdt_property_cell(fdt, "msi-parent", phandle));
 
+   phandle = fdt__get_phandle(PHANDLE_GIC);
/* Generate the interrupt map ... */
dev_hdr = device__first_dev(DEVICE_BUS_PCI);
while (dev_hdr && nentries < ARRAY_SIZE(irq_map)) {
@@ -84,7 +90,9 @@ void pci__generate_fdt_nodes(void *fdt)
},
.pci_pin= cpu_to_fdt32(pin),
},
-   .gic_phandle= cpu_to_fdt32(gic_phandle),
+   .gic_phandle= cpu_to_fdt32(phandle),
+   .gic_addr_hi= 0,
+   .gic_addr_lo= 0,
.gic_irq = {
.type   = cpu_to_fdt32(GIC_FDT_IRQ_TYPE_SPI),
.num= cpu_to_fdt32(irq - GIC_SPI_IRQ_BASE),
diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
index cd2bb72..8006e8f 100644
--- a/include/kvm/fdt.h
+++ b/include/kvm/fdt.h
@@ -11,7 +11,7 @@
 #define FDT_INVALID_PHANDLE 0
 #define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != 

[PATCH v7 01/15] FDT: introduce global phandle allocation

[Andre Przywara]

Allocating an FDT phandle (a unique identifier) using a static
variable in a static inline function defined in a header file works
only if all users are in the same source file. So trying to allocate
a handle from two different compilation units fails.
Introduce global phandle allocation and reference code to properly
allocate unique phandles.

Signed-off-by: Andre Przywara 
---
 Makefile  |  1 +
 arm/fdt.c |  2 +-
 arm/gic.c |  2 +-
 include/kvm/fdt.h | 10 +-
 kvm-fdt.c | 26 ++
 5 files changed, 34 insertions(+), 7 deletions(-)
 create mode 100644 kvm-fdt.c

diff --git a/Makefile b/Makefile
index 1f0196f..e4a4002 100644
--- a/Makefile
+++ b/Makefile
@@ -98,6 +98,7 @@ OBJS  += kvm-ipc.o
 OBJS   += builtin-sandbox.o
 OBJS   += virtio/mmio.o
 OBJS   += hw/i8042.o
+OBJS   += kvm-fdt.o
 
 # Translate uname -m into ARCH string
 ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
diff --git a/arm/fdt.c b/arm/fdt.c
index 381d48f..8bcfffb 100644
--- a/arm/fdt.c
+++ b/arm/fdt.c
@@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
 {
struct device_header *dev_hdr;
u8 staging_fdt[FDT_MAX_SIZE];
-   u32 gic_phandle = fdt__alloc_phandle();
+   u32 gic_phandle = fdt__get_phandle(PHANDLE_GIC);
u64 mem_reg_prop[]  = {
cpu_to_fdt64(kvm->arch.memory_guest_start),
cpu_to_fdt64(kvm->ram_size),
diff --git a/arm/gic.c b/arm/gic.c
index d6d6dd0..b60437e 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum 
irqchip_type type)
_FDT(fdt_property_cell(fdt, "#interrupt-cells", GIC_FDT_IRQ_NUM_CELLS));
_FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
_FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
-   _FDT(fdt_property_cell(fdt, "phandle", phandle));
+   _FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
_FDT(fdt_end_node(fdt));
 }
 
diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
index 53d85a4..cd2bb72 100644
--- a/include/kvm/fdt.h
+++ b/include/kvm/fdt.h
@@ -8,6 +8,10 @@
 #include 
 
 #define FDT_MAX_SIZE   0x1
+#define FDT_INVALID_PHANDLE 0
+#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
+
+enum phandles {PHANDLE_GIC, PHANDLES_MAX};
 
 /* Those definitions are generic FDT values for specifying IRQ
  * types and are used in the Linux kernel internally as well as in
@@ -33,10 +37,6 @@ enum irq_type {
}   \
} while (0)
 
-static inline u32 fdt__alloc_phandle(void)
-{
-   static u32 phandle = 0;
-   return ++phandle;
-}
+u32 fdt__get_phandle(enum phandles phandle);
 
 #endif /* KVM__FDT_H */
diff --git a/kvm-fdt.c b/kvm-fdt.c
new file mode 100644
index 000..d05f3fe
--- /dev/null
+++ b/kvm-fdt.c
@@ -0,0 +1,26 @@
+/*
+ * Commonly used FDT functions.
+ */
+
+#include 
+#include "kvm/fdt.h"
+#include "kvm/util.h"
+
+u32 phandles[PHANDLES_MAX] = {};
+u32 next_phandle = 1;
+
+u32 fdt__get_phandle(enum phandles phandle)
+{
+   u32 ret;
+
+   if (phandle >= PHANDLES_MAX)
+   return FDT_INVALID_PHANDLE;
+
+   ret = phandles[phandle];
+   if (ret == FDT_INVALID_PHANDLE) {
+   ret = next_phandle++;
+   phandles[phandle] = ret;
+   }
+
+   return ret;
+}
-- 
2.9.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC PATCH v1 1/4] arm/arm64: vgic-new: Introduce 64-bit reg access support

[Marc Zyngier]

On 20/07/16 14:02, vijay.kil...@gmail.com wrote:
> From: Vijaya Kumar K 
> 
> vgic_attr_regs_access() handles only 32-bit register
> value. Introduce union ureg to handle both 32 and 64 bit
> register size.
> 
> Signed-off-by: Vijaya Kumar K 
> ---
>  virt/kvm/arm/vgic/vgic-kvm-device.c |   19 ---
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c 
> b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index cc843fe..cace996 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -19,6 +19,11 @@
>  #include 
>  #include "vgic.h"
>  
> +union ureg {
> + u32 reg32;
> + u64 reg64;
> +};
> +

Please don't. That's pointlessly ugly, and creates type confusion. I
want to see explicit types, all the time.

>  /* common helpers */
>  
>  static int vgic_check_ioaddr(struct kvm *kvm, phys_addr_t *ioaddr,
> @@ -255,7 +260,7 @@ void kvm_register_vgic_device(unsigned long type)
>   */
>  static int vgic_attr_regs_access(struct kvm_device *dev,
>struct kvm_device_attr *attr,
> -  u32 *reg, bool is_write)
> +  union ureg *reg, bool is_write)

Just pass a u64 pointer here...

>  {
>   gpa_t addr;
>   int cpuid, ret, c;
> @@ -293,10 +298,10 @@ static int vgic_attr_regs_access(struct kvm_device *dev,

u32 tmp32;

if (is_write)
tmp32 = *reg;

>  
>   switch (attr->group) {
>   case KVM_DEV_ARM_VGIC_GRP_CPU_REGS:
> - ret = vgic_v2_cpuif_uaccess(vcpu, is_write, addr, reg);
> + ret = vgic_v2_cpuif_uaccess(vcpu, is_write, addr, >reg32);

use tmp32 here.

>   break;
>   case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
> - ret = vgic_v2_dist_uaccess(vcpu, is_write, addr, reg);
> + ret = vgic_v2_dist_uaccess(vcpu, is_write, addr, >reg32);
>   break;
>   default:
>   ret = -EINVAL;

and in the epilogue:

if (!is_write)
*reg = tmp32;

> @@ -328,9 +333,9 @@ static int vgic_v2_set_attr(struct kvm_device *dev,
>   case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
>   case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
>   u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> - u32 reg;
> + union ureg reg;
>  
> - if (get_user(reg, uaddr))
> + if (get_user(reg.reg32, uaddr))
>   return -EFAULT;
>  
>   return vgic_attr_regs_access(dev, attr, , true);
> @@ -353,12 +358,12 @@ static int vgic_v2_get_attr(struct kvm_device *dev,
>   case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
>   case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
>   u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> - u32 reg = 0;
> + union ureg reg;
>  
>   ret = vgic_attr_regs_access(dev, attr, , false);
>   if (ret)
>   return ret;
> - return put_user(reg, uaddr);
> + return put_user(reg.reg32, uaddr);
>   }
>   }
>  
> 

Same thing everywhere.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[RFC PATCH v1 3/4] arm/arm64: vgic-new: Introduce find_reg_by_id()

[vijay . kilari]

From: Vijaya Kumar K 

In order to implement vGICv3 CPU interface access, we will need to perform
table lookup of system registers. We would need both index_to_params() and
find_reg() exported for that purpose, but instead we export a single
function which combines them both.

Signed-off-by: Pavel Fedin 
Signed-off-by: Vijaya Kumar K 
---
 arch/arm64/kvm/sys_regs.c |   22 +++---
 arch/arm64/kvm/sys_regs.h |4 
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index a57d650..16c33ca 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1802,6 +1802,17 @@ static bool index_to_params(u64 id, struct 
sys_reg_params *params)
}
 }
 
+const struct sys_reg_desc *find_reg_by_id(u64 id,
+ struct sys_reg_params *params,
+ const struct sys_reg_desc table[],
+ unsigned int num)
+{
+   if (!index_to_params(id, params))
+   return NULL;
+
+   return find_reg(params, table, num);
+}
+
 /* Decode an index value, and find the sys_reg_desc entry. */
 static const struct sys_reg_desc *index_to_sys_reg_desc(struct kvm_vcpu *vcpu,
u64 id)
@@ -1929,10 +1940,8 @@ static int get_invariant_sys_reg(u64 id, void __user 
*uaddr)
struct sys_reg_params params;
const struct sys_reg_desc *r;
 
-   if (!index_to_params(id, ))
-   return -ENOENT;
-
-   r = find_reg(, invariant_sys_regs, 
ARRAY_SIZE(invariant_sys_regs));
+   r = find_reg_by_id(id, , invariant_sys_regs,
+  ARRAY_SIZE(invariant_sys_regs));
if (!r)
return -ENOENT;
 
@@ -1946,9 +1955,8 @@ static int set_invariant_sys_reg(u64 id, void __user 
*uaddr)
int err;
u64 val = 0; /* Make sure high bits are 0 for 32-bit regs */
 
-   if (!index_to_params(id, ))
-   return -ENOENT;
-   r = find_reg(, invariant_sys_regs, 
ARRAY_SIZE(invariant_sys_regs));
+   r = find_reg_by_id(id, , invariant_sys_regs,
+  ARRAY_SIZE(invariant_sys_regs));
if (!r)
return -ENOENT;
 
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index dbbb01c..9c6ffd0 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -136,6 +136,10 @@ static inline int cmp_sys_reg(const struct sys_reg_desc 
*i1,
return i1->Op2 - i2->Op2;
 }
 
+const struct sys_reg_desc *find_reg_by_id(u64 id,
+ struct sys_reg_params *params,
+ const struct sys_reg_desc table[],
+ unsigned int num);
 
 #define Op0(_x).Op0 = _x
 #define Op1(_x).Op1 = _x
-- 
1.7.9.5

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[RFC PATCH v1 4/4] arm/arm64: vgic-new: Implement VGICv3 CPU interface access

[vijay . kilari]

From: Vijaya Kumar K 

CPU sysregs access size is always 64 bits. MPIDR value
passed along with reg id is used to identify the CPU.

Signed-off-by: Pavel Fedin 
Signed-off-by: Vijaya Kumar K 
---
 arch/arm64/include/uapi/asm/kvm.h   |9 ++
 arch/arm64/kvm/Makefile |1 +
 include/linux/irqchip/arm-gic-v3.h  |4 +
 virt/kvm/arm/vgic/vgic-kvm-device.c |   31 +
 virt/kvm/arm/vgic/vgic-mmio-v2.c|4 +-
 virt/kvm/arm/vgic/vgic-mmio-v3.c|6 +
 virt/kvm/arm/vgic/vgic-sys-reg-v3.c |  225 +++
 virt/kvm/arm/vgic/vgic.h|6 +
 8 files changed, 284 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/kvm.h 
b/arch/arm64/include/uapi/asm/kvm.h
index a6b996e..35c4a99 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -206,7 +206,16 @@ struct kvm_arch_memory_slot {
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS   3
 #define KVM_DEV_ARM_VGIC_GRP_CTRL  4
 #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
+#define KVM_DEV_ARM_VGIC_CPU_SYSREGS6
+
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT   0
+#define   KVM_DEV_ARM_VGIC_SYSREG_MASK (KVM_REG_ARM64_SYSREG_OP0_MASK | \
+   KVM_REG_ARM64_SYSREG_OP1_MASK | \
+   KVM_REG_ARM64_SYSREG_CRN_MASK | \
+   KVM_REG_ARM64_SYSREG_CRM_MASK | \
+   KVM_REG_ARM64_SYSREG_OP2_MASK)
+#define   KVM_DEV_ARM_VGIC_SYSREG(op0, op1, crn, crm, op2) \
+   __ARM64_SYS_REG(op0, op1, crn, crm, op2)
 
 /* Device Control API on vcpu fd */
 #define KVM_ARM_VCPU_PMU_V3_CTRL   0
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index f00b2cd..53400c1 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -29,5 +29,6 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-v3.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
 kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index bfbd707..ae84cde 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -288,6 +288,10 @@
 
 #define ICH_VMCR_CTLR_SHIFT0
 #define ICH_VMCR_CTLR_MASK (0x21f << ICH_VMCR_CTLR_SHIFT)
+#define ICH_VMCR_ENG0_SHIFT0
+#define ICH_VMCR_ENG0  (1 << ICH_VMCR_ENG0_SHIFT)
+#define ICH_VMCR_ENG1_SHIFT1
+#define ICH_VMCR_ENG1  (1 << ICH_VMCR_ENG1_SHIFT)
 #define ICH_VMCR_BPR1_SHIFT18
 #define ICH_VMCR_BPR1_MASK (7 << ICH_VMCR_BPR1_SHIFT)
 #define ICH_VMCR_BPR0_SHIFT21
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c 
b/virt/kvm/arm/vgic/vgic-kvm-device.c
index 996a720..78c50cc 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -266,6 +266,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
int cpuid, ret, c;
struct kvm_vcpu *vcpu, *tmp_vcpu;
int vcpu_lock_idx = -1;
+   u64 regid;
struct vgic_dist *vgic = >kvm->arch.vgic;
 
if (vgic->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) {
@@ -322,6 +323,16 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
else
ret = -EINVAL;
break;
+   case KVM_DEV_ARM_VGIC_CPU_SYSREGS:
+   if (vgic->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
+   regid = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_MASK) |
+   KVM_REG_SIZE_U64;
+   ret = vgic_v3_cpu_sysregs_uaccess(vcpu, is_write,
+ regid, >reg64);
+   } else {
+   ret = -EINVAL;
+   }
+   break;
default:
ret = -EINVAL;
break;
@@ -447,6 +458,15 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
 
return vgic_attr_regs_access(dev, attr, , true);
}
+   case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
+   u64 __user *uaddr = (u64 __user *)(long)attr->addr;
+   union ureg reg;
+
+   if (get_user(reg.reg64, uaddr))
+   return -EFAULT;
+
+   return vgic_attr_regs_access(dev, attr, , true);
+   }
}
return -ENXIO;
 }
@@ -472,6 +492,16 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
ret = put_user(reg.reg32, uaddr);
return ret;
}
+   case 

[RFC PATCH v1 1/4] arm/arm64: vgic-new: Introduce 64-bit reg access support

[vijay . kilari]

From: Vijaya Kumar K 

vgic_attr_regs_access() handles only 32-bit register
value. Introduce union ureg to handle both 32 and 64 bit
register size.

Signed-off-by: Vijaya Kumar K 
---
 virt/kvm/arm/vgic/vgic-kvm-device.c |   19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c 
b/virt/kvm/arm/vgic/vgic-kvm-device.c
index cc843fe..cace996 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -19,6 +19,11 @@
 #include 
 #include "vgic.h"
 
+union ureg {
+   u32 reg32;
+   u64 reg64;
+};
+
 /* common helpers */
 
 static int vgic_check_ioaddr(struct kvm *kvm, phys_addr_t *ioaddr,
@@ -255,7 +260,7 @@ void kvm_register_vgic_device(unsigned long type)
  */
 static int vgic_attr_regs_access(struct kvm_device *dev,
 struct kvm_device_attr *attr,
-u32 *reg, bool is_write)
+union ureg *reg, bool is_write)
 {
gpa_t addr;
int cpuid, ret, c;
@@ -293,10 +298,10 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 
switch (attr->group) {
case KVM_DEV_ARM_VGIC_GRP_CPU_REGS:
-   ret = vgic_v2_cpuif_uaccess(vcpu, is_write, addr, reg);
+   ret = vgic_v2_cpuif_uaccess(vcpu, is_write, addr, >reg32);
break;
case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
-   ret = vgic_v2_dist_uaccess(vcpu, is_write, addr, reg);
+   ret = vgic_v2_dist_uaccess(vcpu, is_write, addr, >reg32);
break;
default:
ret = -EINVAL;
@@ -328,9 +333,9 @@ static int vgic_v2_set_attr(struct kvm_device *dev,
case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
u32 __user *uaddr = (u32 __user *)(long)attr->addr;
-   u32 reg;
+   union ureg reg;
 
-   if (get_user(reg, uaddr))
+   if (get_user(reg.reg32, uaddr))
return -EFAULT;
 
return vgic_attr_regs_access(dev, attr, , true);
@@ -353,12 +358,12 @@ static int vgic_v2_get_attr(struct kvm_device *dev,
case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
u32 __user *uaddr = (u32 __user *)(long)attr->addr;
-   u32 reg = 0;
+   union ureg reg;
 
ret = vgic_attr_regs_access(dev, attr, , false);
if (ret)
return ret;
-   return put_user(reg, uaddr);
+   return put_user(reg.reg32, uaddr);
}
}
 
-- 
1.7.9.5

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[RFC PATCH v1 2/4] arm/arm64: vgic-new: Add distributor and redistributor access

[vijay . kilari]

From: Vijaya Kumar K 

VGICv3 Distributor and Redistributor registers are accessed using
KVM_DEV_ARM_VGIC_GRP_DIST_REGS and KVM_DEV_ARM_VGIC_GRP_DIST_REGS
with KVM_SET_DEVICE_ATTR and KVM_GET_DEVICE_ATTR ioctls.
These registers are accessed as 32-bit and cpu mpidr
value passed along with register offset is used to identify the
cpu for redistributor registers access.

The draft version of VGIC v3 specification is define here
https://lists.cs.columbia.edu/pipermail/kvmarm/2016-May/020355.html

Signed-off-by: Vijaya Kumar K 
---
 arch/arm64/include/uapi/asm/kvm.h   |3 +
 virt/kvm/arm/vgic/vgic-kvm-device.c |   72 ++--
 virt/kvm/arm/vgic/vgic-mmio-v3.c|  105 +++
 virt/kvm/arm/vgic/vgic-mmio.c   |2 +-
 virt/kvm/arm/vgic/vgic.h|8 +++
 5 files changed, 183 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/kvm.h 
b/arch/arm64/include/uapi/asm/kvm.h
index f209ea1..a6b996e 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -199,10 +199,13 @@ struct kvm_arch_memory_slot {
 #define KVM_DEV_ARM_VGIC_GRP_CPU_REGS  2
 #define   KVM_DEV_ARM_VGIC_CPUID_SHIFT 32
 #define   KVM_DEV_ARM_VGIC_CPUID_MASK  (0xffULL << 
KVM_DEV_ARM_VGIC_CPUID_SHIFT)
+#define   KVM_DEV_ARM_VGIC_V3_CPUID_MASK \
+   (0xULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK (0xULL << 
KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS   3
 #define KVM_DEV_ARM_VGIC_GRP_CTRL  4
+#define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT   0
 
 /* Device Control API on vcpu fd */
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c 
b/virt/kvm/arm/vgic/vgic-kvm-device.c
index cace996..996a720 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -266,10 +266,17 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
int cpuid, ret, c;
struct kvm_vcpu *vcpu, *tmp_vcpu;
int vcpu_lock_idx = -1;
+   struct vgic_dist *vgic = >kvm->arch.vgic;
 
-   cpuid = (attr->attr & KVM_DEV_ARM_VGIC_CPUID_MASK) >>
-KVM_DEV_ARM_VGIC_CPUID_SHIFT;
-   vcpu = kvm_get_vcpu(dev->kvm, cpuid);
+   if (vgic->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) {
+   cpuid = (attr->attr & KVM_DEV_ARM_VGIC_CPUID_MASK) >>
+KVM_DEV_ARM_VGIC_CPUID_SHIFT;
+   vcpu = kvm_get_vcpu(dev->kvm, cpuid);
+   } else {
+   cpuid = (attr->attr & KVM_DEV_ARM_VGIC_V3_CPUID_MASK) >>
+KVM_DEV_ARM_VGIC_CPUID_SHIFT;
+   vcpu = kvm_mpidr_to_vcpu(dev->kvm, cpuid);
+   }
addr = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 
mutex_lock(>kvm->lock);
@@ -301,7 +308,19 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
ret = vgic_v2_cpuif_uaccess(vcpu, is_write, addr, >reg32);
break;
case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
-   ret = vgic_v2_dist_uaccess(vcpu, is_write, addr, >reg32);
+   if (vgic->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2)
+   ret = vgic_v2_dist_uaccess(vcpu, is_write, addr,
+  >reg32);
+   else
+   ret = vgic_v3_dist_uaccess(vcpu, is_write, addr,
+  >reg32);
+   break;
+   case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
+   if (vgic->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+   ret = vgic_v3_redist_uaccess(vcpu, is_write, addr,
+>reg32);
+   else
+   ret = -EINVAL;
break;
default:
ret = -EINVAL;
@@ -411,13 +430,51 @@ struct kvm_device_ops kvm_arm_vgic_v2_ops = {
 static int vgic_v3_set_attr(struct kvm_device *dev,
struct kvm_device_attr *attr)
 {
-   return vgic_set_common_attr(dev, attr);
+   int ret;
+
+   ret = vgic_set_common_attr(dev, attr);
+   if (ret != -ENXIO)
+   return ret;
+
+   switch (attr->group) {
+   case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
+   case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS: {
+   u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+   union ureg reg;
+
+   if (get_user(reg.reg32, uaddr))
+   return -EFAULT;
+
+   return vgic_attr_regs_access(dev, attr, , true);
+   }
+   }
+   return -ENXIO;
 }
 
 static int vgic_v3_get_attr(struct kvm_device *dev,
struct kvm_device_attr *attr)
 {
-   return vgic_get_common_attr(dev, attr);
+   int ret;
+
+   ret = 

[RFC PATCH v1 0/4] arm/arm64: vgic-new: Implement API for vGICv3 live migration

[vijay . kilari]

From: Vijaya Kumar K 

This patchset adds API for saving and restoring
of VGICv3 registers to support live migration with new vgic feature.
This API definition is as per draft version of VGICv3 specification
https://lists.cs.columbia.edu/pipermail/kvmarm/2016-May/020355.html

To test live migration with QEMU, the QEMU patches will
be posted soon.

The patch 3 & 4 are picked from the Pavel's previous implementation.
http://www.spinics.net/lists/kvm/msg122040.html

Vijaya Kumar K (4):
  arm/arm64: vgic-new: Introduce 64-bit reg access support
  arm/arm64: vgic-new: Add distributor and redistributor access
  arm/arm64: vgic-new: Introduce find_reg_by_id()
  arm/arm64: vgic-new: Implement VGICv3 CPU interface access

 arch/arm64/include/uapi/asm/kvm.h   |   12 ++
 arch/arm64/kvm/Makefile |1 +
 arch/arm64/kvm/sys_regs.c   |   22 ++--
 arch/arm64/kvm/sys_regs.h   |4 +
 include/linux/irqchip/arm-gic-v3.h  |4 +
 virt/kvm/arm/vgic/vgic-kvm-device.c |  120 +--
 virt/kvm/arm/vgic/vgic-mmio-v2.c|4 +-
 virt/kvm/arm/vgic/vgic-mmio-v3.c|  111 +
 virt/kvm/arm/vgic/vgic-mmio.c   |2 +-
 virt/kvm/arm/vgic/vgic-sys-reg-v3.c |  225 +++
 virt/kvm/arm/vgic/vgic.h|   14 +++
 11 files changed, 497 insertions(+), 22 deletions(-)
 create mode 100644 virt/kvm/arm/vgic/vgic-sys-reg-v3.c

-- 
1.7.9.5

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v11 0/8] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 1/3: iommu changes

[Dennis Chen]

On Wed, Jul 20, 2016 at 01:03:00PM +0200, Auger Eric wrote:
> Hi Dennis
> On 20/07/2016 11:56, Dennis Chen wrote:
> > Hi Eric,
> > 
> > On Tue, Jul 19, 2016 at 12:55:03PM +, Eric Auger wrote:
> >> This series introduces the msi-iommu api used to:
> >>
> >> - allocate/free resources for MSI IOMMU mapping
> >> - set the MSI iova window aperture
> >> - map/unmap physical addresses onto MSI IOVAs.
> >> - determine whether an msi needs to be iommu mapped
> >> - overwrite an msi_msg PA address with its pre-allocated/mapped IOVA
> >>
> >> Also a new iommu domain attribute, DOMAIN_ATTR_MSI_GEOMETRY is introduced
> >> to report the MSI iova window geometry (aperture and iommu-msi API 
> >> support).
> >>
> >> Currently:
> >> - iommu driver is supposed to allocate/free MSI mapping resources
> >> - VFIO subsystem is supposed to set the MSI IOVA aperture.
> >> - The MSI layer is supposed to allocate/free iova mappings and overwrite
> >>   msi_msg with IOVA at composition time
> >>
> >> More details & context can be found at:
> >> http://www.linaro.org/blog/core-dump/kvm-pciemsi-passthrough-armarm64/
> >>
> >> Best Regards
> >>
> >> Eric
> >>
> >> Git: complete series available at
> >> https://github.com/eauger/linux/tree/v4.7-rc7-passthrough-v11
> >>
> > Why can't I find this new series on your git tree:
> > https://git.linaro.org/people/eric.auger/linux.git
> you are not looking at the right git repo: see github one above.
> > ?
> > Also, do I need to download all the 3-part patches to test the PCIe NIC 
> > passthru
> > as I did on your v9 series?
> Yes you need to take the 3 parts. You should have everything that is
> needed on the above branch. In case you do not work on Cavium, you
> should not cherry-pick
> "vfio: pci: HACK! workaround thunderx pci_try_reset_bus crash"
>
Thanks Eric, I've got all the 3 parts from the github git tree in my local 
repos.
Currently I think the major platform of mine is AMD overdrive. So I will test 
this patch set
on that platform first.

Thanks,
Dennis
> 
> Thanks
> 
> Eric
> > 
> > Thanks,
> > Dennis 
> >>
> >> see part III for wrap-up details.
> >>
> >> History:
> >> v10 -> v11:
> >> - no change in the series, just incremented for consistency
> >> - added a temporary patch in the branch:
> >>   "iommu/iova: FIXUP! validate iova_domain input to put_iova_domain"
> >>   originally sent by Nate and adapted for this use case. This is currently
> >>   under discussion on the ML. The crash typically occurs in case unsafe
> >>   interrupts are discovered while allow_unsafe_interrupts is not set.
> >>
> >> v9 -> v10:
> >> - split error management in iommu_msi_set_aperture
> >>
> >> v8 -> v9:
> >> - rename iommu_domain_msi_geometry programmable flag into 
> >> iommu_msi_supported
> >> - introduce msi_apperture_valid helper and use this instead of 
> >> is_aperture_set
> >>
> >> v7 -> v8:
> >> - The API is retargetted for MSI: renamed msi-iommu
> >>   all "dma-reserved" namings removed
> >> - now implemented upon dma-iommu (get, put, init), ie. reuse iova_cookie,
> >>   and iova API
> >> - msi mapping resources now are guaranteed to exist during the whole iommu
> >>   domain's lifetime. No need to lock to garantee the cookie integrity
> >> - removed alloc/free_reserved_reserved_iova_domain. We now have a single
> >>   function that sets the aperture, looking like iommu_dma_init_domain.
> >> - we now use a list instead of an RB-tree
> >> - prot is not propagated anymore at domain creation due to the retargetting
> >>   for MSI
> >> - iommu_domain pointer removed from doorbell_mapping struct
> >> - replaced DOMAIN_ATTR_MSI_MAPPING by DOMAIN_ATTR_MSI_GEOMETRY
> >>
> >> v6 -> v7:
> >> - fixed known lock bugs and multiple page sized slots matching
> >>   (I only have a single MSI frame made of a single page)
> >> - reserved_iova_cookie now pointing to a struct that encapsulates the
> >>   iova domain handle + protection attribute passed from VFIO (Alex' req)
> >> - 2 new functions exposed: iommu_msi_mapping_translate_msg,
> >>   iommu_msi_mapping_desc_to_domain: not sure this is the right 
> >> location/proto
> >>   though
> >> - iommu_put_reserved_iova now takes a phys_addr_t
> >> - everything now is cleanup on iommu_domain destruction
> >>
> >> RFC v5 -> patch v6:
> >> - split to ease the review process
> >> - in dma-reserved-api use a spin lock instead of a mutex (reported by
> >>   Jean-Philippe)
> >> - revisit iommu_get_reserved_iova API to pass a size parameter upon
> >>   Marc's request
> >> - Consistently use the page order passed when creating the iova domain.
> >> - init reserved_binding_list (reported by Julien)
> >>
> >> RFC v4 -> RFC v5:
> >> - take into account Thomas' comments on MSI related patches
> >>   - split "msi: IOMMU map the doorbell address when needed"
> >>   - increase readability and add comments
> >>   - fix style issues
> >>  - split "iommu: Add DOMAIN_ATTR_MSI_MAPPING attribute"
> >>  - platform ITS now advertises IOMMU_CAP_INTR_REMAP
> >>  - fix compilation 

Re: [PATCH v11 4/8] iommu/msi-iommu: initialization

[Dennis Chen]

Hi Eric,
Some small questions/comments below:

On Tue, Jul 19, 2016 at 12:55:07PM +, Eric Auger wrote:
> iommu_get/put_msi_cookie allocates/frees the resource used to store
> and ref count the MSI doorbell mappings. iommu_msi_set_aperture
> initializes the iova domain used for MSI IOVA allocation and sets the
> iommu domain's msi geometry.
> 
> The implementation relies on dma-iommu API and iova API.
> 
> New msi functions are fully implemented if CONFIG_IOMMU_MSI is set.
> 
> Signed-off-by: Eric Auger 
> 
> ---
> v10:
> - split error management in iommu_msi_set_aperture
> 
> v9:
> - remove is_aperture_set and use iommu_domain_msi_aperture_valid helper
>   instead
> - set iommu domain's msi geometry
> 
> v8:
> - new design where msi-iommu relies on dma-iommu
> - remove the iommu_domain * from the doorbell_mapping struct
> - added is_aperture_set
> 
> v7:
> - fix locking
> - add iova_cache_get/put
> - static inline functions when CONFIG_IOMMU_DMA_RESERVED is not set
> - introduce struct reserved_iova_domain to encapsulate prot info &
>   add prot parameter in alloc_reserved_iova_domain
> 
> v5 -> v6:
> - use spin lock instead of mutex
> 
> v3 -> v4:
> - formerly in "iommu/arm-smmu: implement alloc/free_reserved_iova_domain" &
>   "iommu: add alloc/free_reserved_iova_domain"
> 
> v2 -> v3:
> - remove iommu_alloc_reserved_iova_domain & iommu_free_reserved_iova_domain
>   static implementation in case CONFIG_IOMMU_API is not set
> 
> v1 -> v2:
> - moved from vfio API to IOMMU API
> ---
>  drivers/iommu/Kconfig |   7 
>  drivers/iommu/Makefile|   1 +
>  drivers/iommu/msi-iommu.c | 100 
> ++
>  include/linux/msi-iommu.h |  65 ++
>  4 files changed, 173 insertions(+)
>  create mode 100644 drivers/iommu/msi-iommu.c
>  create mode 100644 include/linux/msi-iommu.h
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index ad08603..5ea1610 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -74,6 +74,11 @@ config IOMMU_DMA
>   select IOMMU_IOVA
>   select NEED_SG_DMA_LENGTH
>  
> +# IOMMU MSI mapping
> +config IOMMU_MSI
> + bool
> + select IOMMU_DMA
> +
>  config FSL_PAMU
>   bool "Freescale IOMMU support"
>   depends on PPC_E500MC || (COMPILE_TEST && PPC)
> @@ -296,6 +301,7 @@ config SPAPR_TCE_IOMMU
>  config ARM_SMMU
>   bool "ARM Ltd. System MMU (SMMU) Support"
>   depends on (ARM64 || ARM) && MMU
> + select IOMMU_MSI
>   select IOMMU_API
>   select IOMMU_IO_PGTABLE_LPAE
>   select ARM_DMA_USE_IOMMU if ARM
> @@ -309,6 +315,7 @@ config ARM_SMMU
>  config ARM_SMMU_V3
>   bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
>   depends on ARM64 && PCI
> + select IOMMU_MSI
>   select IOMMU_API
>   select IOMMU_IO_PGTABLE_LPAE
>   select GENERIC_MSI_IRQ_DOMAIN
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index c6edb31..a381e66 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -2,6 +2,7 @@ obj-$(CONFIG_IOMMU_API) += iommu.o
>  obj-$(CONFIG_IOMMU_API) += iommu-traces.o
>  obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
>  obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
> +obj-$(CONFIG_IOMMU_MSI) += msi-iommu.o
>  obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
>  obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
>  obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
> new file mode 100644
> index 000..de02ede
> --- /dev/null
> +++ b/drivers/iommu/msi-iommu.c
> @@ -0,0 +1,100 @@
> +/*
> + * Reserved IOVA Management
> + *
> + * Copyright (c) 2015 Linaro Ltd.
> + *  www.linaro.org
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct doorbell_mapping {
> + struct kref kref;
> + struct list_headnext;
> + phys_addr_t addr;
> + dma_addr_t  iova;
> + size_t  size;
> +};
> +
> +struct doorbell_mapping_info {
> + struct list_head list; /* list of doorbell mapping entries */
> + spinlock_t lock;
> +};
> +
> +int iommu_get_msi_cookie(struct iommu_domain *domain)
> +{
> + struct doorbell_mapping_info *dmi;
> + int ret;
> +
> + if (domain->msi_cookie || domain->iova_cookie)
> + return -EINVAL;
> +
> + ret = iommu_get_dma_cookie(domain);
> + if (ret)
> + return ret;
> 

Re: [PATCH v11 0/8] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 1/3: iommu changes

[Auger Eric]

Hi Dennis
On 20/07/2016 11:56, Dennis Chen wrote:
> Hi Eric,
> 
> On Tue, Jul 19, 2016 at 12:55:03PM +, Eric Auger wrote:
>> This series introduces the msi-iommu api used to:
>>
>> - allocate/free resources for MSI IOMMU mapping
>> - set the MSI iova window aperture
>> - map/unmap physical addresses onto MSI IOVAs.
>> - determine whether an msi needs to be iommu mapped
>> - overwrite an msi_msg PA address with its pre-allocated/mapped IOVA
>>
>> Also a new iommu domain attribute, DOMAIN_ATTR_MSI_GEOMETRY is introduced
>> to report the MSI iova window geometry (aperture and iommu-msi API support).
>>
>> Currently:
>> - iommu driver is supposed to allocate/free MSI mapping resources
>> - VFIO subsystem is supposed to set the MSI IOVA aperture.
>> - The MSI layer is supposed to allocate/free iova mappings and overwrite
>>   msi_msg with IOVA at composition time
>>
>> More details & context can be found at:
>> http://www.linaro.org/blog/core-dump/kvm-pciemsi-passthrough-armarm64/
>>
>> Best Regards
>>
>> Eric
>>
>> Git: complete series available at
>> https://github.com/eauger/linux/tree/v4.7-rc7-passthrough-v11
>>
> Why can't I find this new series on your git tree:
> https://git.linaro.org/people/eric.auger/linux.git
you are not looking at the right git repo: see github one above.
> ?
> Also, do I need to download all the 3-part patches to test the PCIe NIC 
> passthru
> as I did on your v9 series?
Yes you need to take the 3 parts. You should have everything that is
needed on the above branch. In case you do not work on Cavium, you
should not cherry-pick
"vfio: pci: HACK! workaround thunderx pci_try_reset_bus crash"

Thanks

Eric
> 
> Thanks,
> Dennis 
>>
>> see part III for wrap-up details.
>>
>> History:
>> v10 -> v11:
>> - no change in the series, just incremented for consistency
>> - added a temporary patch in the branch:
>>   "iommu/iova: FIXUP! validate iova_domain input to put_iova_domain"
>>   originally sent by Nate and adapted for this use case. This is currently
>>   under discussion on the ML. The crash typically occurs in case unsafe
>>   interrupts are discovered while allow_unsafe_interrupts is not set.
>>
>> v9 -> v10:
>> - split error management in iommu_msi_set_aperture
>>
>> v8 -> v9:
>> - rename iommu_domain_msi_geometry programmable flag into iommu_msi_supported
>> - introduce msi_apperture_valid helper and use this instead of 
>> is_aperture_set
>>
>> v7 -> v8:
>> - The API is retargetted for MSI: renamed msi-iommu
>>   all "dma-reserved" namings removed
>> - now implemented upon dma-iommu (get, put, init), ie. reuse iova_cookie,
>>   and iova API
>> - msi mapping resources now are guaranteed to exist during the whole iommu
>>   domain's lifetime. No need to lock to garantee the cookie integrity
>> - removed alloc/free_reserved_reserved_iova_domain. We now have a single
>>   function that sets the aperture, looking like iommu_dma_init_domain.
>> - we now use a list instead of an RB-tree
>> - prot is not propagated anymore at domain creation due to the retargetting
>>   for MSI
>> - iommu_domain pointer removed from doorbell_mapping struct
>> - replaced DOMAIN_ATTR_MSI_MAPPING by DOMAIN_ATTR_MSI_GEOMETRY
>>
>> v6 -> v7:
>> - fixed known lock bugs and multiple page sized slots matching
>>   (I only have a single MSI frame made of a single page)
>> - reserved_iova_cookie now pointing to a struct that encapsulates the
>>   iova domain handle + protection attribute passed from VFIO (Alex' req)
>> - 2 new functions exposed: iommu_msi_mapping_translate_msg,
>>   iommu_msi_mapping_desc_to_domain: not sure this is the right location/proto
>>   though
>> - iommu_put_reserved_iova now takes a phys_addr_t
>> - everything now is cleanup on iommu_domain destruction
>>
>> RFC v5 -> patch v6:
>> - split to ease the review process
>> - in dma-reserved-api use a spin lock instead of a mutex (reported by
>>   Jean-Philippe)
>> - revisit iommu_get_reserved_iova API to pass a size parameter upon
>>   Marc's request
>> - Consistently use the page order passed when creating the iova domain.
>> - init reserved_binding_list (reported by Julien)
>>
>> RFC v4 -> RFC v5:
>> - take into account Thomas' comments on MSI related patches
>>   - split "msi: IOMMU map the doorbell address when needed"
>>   - increase readability and add comments
>>   - fix style issues
>>  - split "iommu: Add DOMAIN_ATTR_MSI_MAPPING attribute"
>>  - platform ITS now advertises IOMMU_CAP_INTR_REMAP
>>  - fix compilation issue with CONFIG_IOMMU API unset
>>  - arm-smmu-v3 now advertises DOMAIN_ATTR_MSI_MAPPING
>>
>> RFC v3 -> v4:
>> - Move doorbell mapping/unmapping in msi.c
>> - fix ref count issue on set_affinity: in case of a change in the address
>>   the previous address is decremented
>> - doorbell map/unmap now is done on msi composition. Should allow the use
>>   case for platform MSI controllers
>> - create dma-reserved-iommu.h/c exposing/implementing a new API dedicated
>>   to reserved IOVA management (looking like 

Re: [PATCH v11 0/8] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 1/3: iommu changes

[Dennis Chen]

Hi Eric,

On Tue, Jul 19, 2016 at 12:55:03PM +, Eric Auger wrote:
> This series introduces the msi-iommu api used to:
> 
> - allocate/free resources for MSI IOMMU mapping
> - set the MSI iova window aperture
> - map/unmap physical addresses onto MSI IOVAs.
> - determine whether an msi needs to be iommu mapped
> - overwrite an msi_msg PA address with its pre-allocated/mapped IOVA
> 
> Also a new iommu domain attribute, DOMAIN_ATTR_MSI_GEOMETRY is introduced
> to report the MSI iova window geometry (aperture and iommu-msi API support).
> 
> Currently:
> - iommu driver is supposed to allocate/free MSI mapping resources
> - VFIO subsystem is supposed to set the MSI IOVA aperture.
> - The MSI layer is supposed to allocate/free iova mappings and overwrite
>   msi_msg with IOVA at composition time
> 
> More details & context can be found at:
> http://www.linaro.org/blog/core-dump/kvm-pciemsi-passthrough-armarm64/
> 
> Best Regards
> 
> Eric
> 
> Git: complete series available at
> https://github.com/eauger/linux/tree/v4.7-rc7-passthrough-v11
>
Why can't I find this new series on your git tree:
https://git.linaro.org/people/eric.auger/linux.git
?
Also, do I need to download all the 3-part patches to test the PCIe NIC passthru
as I did on your v9 series?

Thanks,
Dennis 
>
> see part III for wrap-up details.
> 
> History:
> v10 -> v11:
> - no change in the series, just incremented for consistency
> - added a temporary patch in the branch:
>   "iommu/iova: FIXUP! validate iova_domain input to put_iova_domain"
>   originally sent by Nate and adapted for this use case. This is currently
>   under discussion on the ML. The crash typically occurs in case unsafe
>   interrupts are discovered while allow_unsafe_interrupts is not set.
> 
> v9 -> v10:
> - split error management in iommu_msi_set_aperture
> 
> v8 -> v9:
> - rename iommu_domain_msi_geometry programmable flag into iommu_msi_supported
> - introduce msi_apperture_valid helper and use this instead of is_aperture_set
> 
> v7 -> v8:
> - The API is retargetted for MSI: renamed msi-iommu
>   all "dma-reserved" namings removed
> - now implemented upon dma-iommu (get, put, init), ie. reuse iova_cookie,
>   and iova API
> - msi mapping resources now are guaranteed to exist during the whole iommu
>   domain's lifetime. No need to lock to garantee the cookie integrity
> - removed alloc/free_reserved_reserved_iova_domain. We now have a single
>   function that sets the aperture, looking like iommu_dma_init_domain.
> - we now use a list instead of an RB-tree
> - prot is not propagated anymore at domain creation due to the retargetting
>   for MSI
> - iommu_domain pointer removed from doorbell_mapping struct
> - replaced DOMAIN_ATTR_MSI_MAPPING by DOMAIN_ATTR_MSI_GEOMETRY
> 
> v6 -> v7:
> - fixed known lock bugs and multiple page sized slots matching
>   (I only have a single MSI frame made of a single page)
> - reserved_iova_cookie now pointing to a struct that encapsulates the
>   iova domain handle + protection attribute passed from VFIO (Alex' req)
> - 2 new functions exposed: iommu_msi_mapping_translate_msg,
>   iommu_msi_mapping_desc_to_domain: not sure this is the right location/proto
>   though
> - iommu_put_reserved_iova now takes a phys_addr_t
> - everything now is cleanup on iommu_domain destruction
> 
> RFC v5 -> patch v6:
> - split to ease the review process
> - in dma-reserved-api use a spin lock instead of a mutex (reported by
>   Jean-Philippe)
> - revisit iommu_get_reserved_iova API to pass a size parameter upon
>   Marc's request
> - Consistently use the page order passed when creating the iova domain.
> - init reserved_binding_list (reported by Julien)
> 
> RFC v4 -> RFC v5:
> - take into account Thomas' comments on MSI related patches
>   - split "msi: IOMMU map the doorbell address when needed"
>   - increase readability and add comments
>   - fix style issues
>  - split "iommu: Add DOMAIN_ATTR_MSI_MAPPING attribute"
>  - platform ITS now advertises IOMMU_CAP_INTR_REMAP
>  - fix compilation issue with CONFIG_IOMMU API unset
>  - arm-smmu-v3 now advertises DOMAIN_ATTR_MSI_MAPPING
> 
> RFC v3 -> v4:
> - Move doorbell mapping/unmapping in msi.c
> - fix ref count issue on set_affinity: in case of a change in the address
>   the previous address is decremented
> - doorbell map/unmap now is done on msi composition. Should allow the use
>   case for platform MSI controllers
> - create dma-reserved-iommu.h/c exposing/implementing a new API dedicated
>   to reserved IOVA management (looking like dma-iommu glue)
> - series reordering to ease the review:
>   - first part is related to IOMMU
>   - second related to MSI sub-system
>   - third related to VFIO (except arm-smmu IOMMU_CAP_INTR_REMAP removal)
> - expose the number of requested IOVA pages through VFIO_IOMMU_GET_INFO
>   [this partially addresses Marc's comments on iommu_get/put_single_reserved
>size/alignment problematic - which I did not ignore - but I don't know
>how much I can do at the 

Re: [PATCH v11 10/10] genirq/msi: use the MSI doorbell's IOVA when requested

[Thomas Gleixner]

On Tue, 19 Jul 2016, Eric Auger wrote:

First of all - valid for all patches:

Subject: sys/subsys: Sentence starts with an uppercase letter

Now for this particular one:

genirq/msi: use the MSI doorbell's IOVA when requested

> On MSI message composition we now use the MSI doorbell's IOVA in
> place of the doorbell's PA in case the device is upstream to an
> IOMMU that requires MSI addresses to be mapped. The doorbell's
> allocation and mapping happened on an early stage (pci_enable_msi).

This changelog is completely useless. At least I cannot figure out what that
patch actually does. And the implementation is not self explaining either.
 
> @@ -63,10 +63,18 @@ static int msi_compose(struct irq_data *irq_data,
>  {
>   int ret = 0;
>  
> - if (erase)
> + if (erase) {
>   memset(msg, 0, sizeof(*msg));
> - else
> + } else {
> + struct device *dev;
> +
>   ret = irq_chip_compose_msi_msg(irq_data, msg);
> + if (ret)
> + return ret;
> +
> + dev = msi_desc_to_dev(irq_data_get_msi_desc(irq_data));
> + WARN_ON(iommu_msi_msg_pa_to_va(dev, msg));

What the heck is this call doing? And why is there only a WARN_ON and not a
proper error return code handling?

Thanks,

tglx
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v11 09/10] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs

[Thomas Gleixner]

On Tue, 19 Jul 2016, Eric Auger wrote:
>  /**
> + * msi_handle_doorbell_mappings: in case the irq data corresponds to an
> + * MSI that requires iommu mapping, traverse the irq domain hierarchy
> + * to retrieve the doorbells to handle and iommu_map/unmap them according
> + * to @map boolean.
> + *
> + * @data: irq data handle
> + * @map: mapping if true, unmapping if false
> + */


Please run that through the kernel doc generator. It does not work that way.

The format is:

/**
 * function_name - Short function description
 * @arg1:   Description of arg1
 * @argument2:  Description of argument2
 *
 * Long explanation including documentation of the return values.
 */

> +static int msi_handle_doorbell_mappings(struct irq_data *data, bool map)
> +{
> + const struct irq_chip_msi_doorbell_info *dbinfo;
> + struct iommu_domain *domain;
> + struct irq_chip *chip;
> + struct device *dev;
> + dma_addr_t iova;
> + int ret = 0, cpu;
> +
> + while (data) {
> + dev = msi_desc_to_dev(irq_data_get_msi_desc(data));
> + domain = iommu_msi_domain(dev);
> + if (domain) {
> + chip = irq_data_get_irq_chip(data);
> + if (chip->msi_doorbell_info)
> + break;
> + }
> + data = data->parent_data;
> + }

Please split that out into a seperate function

struct irq_data *msi_get_doorbell_info(data)
{
.
if (chip->msi_doorbell_info)
return chip->msi_get_doorbell_info(data);
}
return NULL;
}

   info = msi_get_doorbell_info(data);
   .

> + if (!data)
> + return 0;
> +
> + dbinfo = chip->msi_doorbell_info(data);
> + if (!dbinfo)
> + return -EINVAL;
> +
> + if (!dbinfo->doorbell_is_percpu) {
> + if (!map) {
> + iommu_msi_put_doorbell_iova(domain,
> + dbinfo->global_doorbell);
> + return 0;
> + }
> + return iommu_msi_get_doorbell_iova(domain,
> +dbinfo->global_doorbell,
> +dbinfo->size, dbinfo->prot,
> +);
> + }

You can spare an indentation level with a helper function

if (!dbinfo->doorbell_is_percpu)
return msi_map_global_doorbell(domain, dbinfo);

> +
> + /* percpu doorbells */
> + for_each_possible_cpu(cpu) {
> + phys_addr_t __percpu *db_addr =
> + per_cpu_ptr(dbinfo->percpu_doorbells, cpu);
> +
> + if (!map) {
> + iommu_msi_put_doorbell_iova(domain, *db_addr);
> + } else {
> +
> + ret = iommu_msi_get_doorbell_iova(domain, *db_addr,
> +   dbinfo->size,
> +   dbinfo->prot, );
> + if (ret)
> + return ret;
> + }
> + }

Same here:

for_each_possible_cpu(cpu) {
ret = msi_map_percpu_doorbell(domain, cpu);
if (ret)
return ret;
}
return 0;
 
Hmm?

> +
> + return 0;
> +}
> +
> +/**
>   * msi_domain_alloc_irqs - Allocate interrupts from a MSI interrupt domain
>   * @domain:  The domain to allocate from
>   * @dev: Pointer to device struct of the device for which the interrupts
> @@ -352,17 +423,29 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, 
> struct device *dev,
>  
>   virq = __irq_domain_alloc_irqs(domain, virq, desc->nvec_used,
>  dev_to_node(dev), , false);
> - if (virq < 0) {
> - ret = -ENOSPC;
> - if (ops->handle_error)
> - ret = ops->handle_error(domain, desc, ret);
> - if (ops->msi_finish)
> - ops->msi_finish(, ret);
> - return ret;
> - }
> + if (virq < 0)
> + goto error;
>  
>   for (i = 0; i < desc->nvec_used; i++)
>   irq_set_msi_desc_off(virq, i, desc);
> +
> + for (i = 0; i < desc->nvec_used; i++) {
> + struct irq_data *d = irq_get_irq_data(virq + i);
> +
> + ret = msi_handle_doorbell_mappings(d, true);
> + if (ret)
> + break;
> + }
> + if (ret) {
> + for (; i >= 0; i--) {
> + struct irq_data *d = irq_get_irq_data(virq + i);
> +
> + msi_handle_doorbell_mappings(d, false);
> + }
> +  

Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe

[Thomas Gleixner]

On Tue, 19 Jul 2016, Eric Auger wrote:
> +bool msi_doorbell_safe(void)
> +{
> + struct irqchip_doorbell *db;
> + bool irq_remapping = true;
> +
> + mutex_lock(_doorbell_mutex);
> + list_for_each_entry(db, _doorbell_list, next) {
> + irq_remapping &= db->info.irq_remapping;

db->info.irq_remapping is set in msi_doorbell_register(). So you can keep book
about that there. No need to iterate here.

Thanks,

tglx


___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v11 04/10] genirq/msi-doorbell: allow MSI doorbell (un)registration

[Auger Eric]

Hi Thomas,
On 19/07/2016 16:22, Thomas Gleixner wrote:
> On Tue, 19 Jul 2016, Eric Auger wrote:
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +struct irqchip_doorbell {
>> +struct irq_chip_msi_doorbell_info info;
>> +struct list_head next;
> 
> Again, please align the struct members.
> 
>> +};
>> +
>> +static LIST_HEAD(irqchip_doorbell_list);
>> +static DEFINE_MUTEX(irqchip_doorbell_mutex);
>> +
>> +struct irq_chip_msi_doorbell_info *
>> +msi_doorbell_register_global(phys_addr_t base, size_t size,
>> + int prot, bool irq_remapping)
>> +{
>> +struct irqchip_doorbell *db;
>> +
>> +db = kmalloc(sizeof(*db), GFP_KERNEL);
>> +if (!db)
>> +return ERR_PTR(-ENOMEM);
>> +
>> +db->info.doorbell_is_percpu = false;
> 
> Please use kzalloc and get rid of zero initialization. If you add stuff to the
> struct then initialization will be automatically 0.
OK
> 
>> +void msi_doorbell_unregister_global(struct irq_chip_msi_doorbell_info 
>> *dbinfo)
>> +{
>> +struct irqchip_doorbell *db, *tmp;
>> +
>> +mutex_lock(_doorbell_mutex);
>> +list_for_each_entry_safe(db, tmp, _doorbell_list, next) {
> 
> Why do you need that iterator? 
> 
> db = container_of(dbinfo, struct ., info);
> 
> Hmm?

definitively
> 
>> +if (dbinfo == >info) {
>> +list_del(>next);
>> +kfree(db);
> 
> Please move the kfree() outside of the lock region. It does not matter much
> here, but we really should stop doing random crap in locked regions.
OK

Thanks

Eric
> 
> Thanks,
> 
>   tglx
> 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v11 05/10] genirq/msi-doorbell: msi_doorbell_pages

[Auger Eric]

Hi Thomas,
On 19/07/2016 16:38, Thomas Gleixner wrote:
> On Tue, 19 Jul 2016, Eric Auger wrote:
>> msi_doorbell_pages sum up the number of iommu pages of a given order
> 
> adding () to the function name would make it immediately clear that
> msi_doorbell_pages is a function.
> 
>> +/**
>> + * msi_doorbell_pages: compute the number of iommu pages of size 1 << order
>> + * requested to map all the registered doorbells
>> + *
>> + * @order: iommu page order
>> + */
> 
> Why are you adding the kernel doc to the header and not to the implementation?
> 
>> +int msi_doorbell_pages(unsigned int order);
>> +
>>  #else
>>  
>>  static inline struct irq_chip_msi_doorbell_info *
>> @@ -47,6 +55,12 @@ msi_doorbell_register_global(phys_addr_t base, size_t 
>> size,
>>  static inline void
>>  msi_doorbell_unregister_global(struct irq_chip_msi_doorbell_info *db) {}
>>  
>> +static inline int
>> +msi_doorbell_pages(unsigned int order)
> 
> What's the point of this line break?

> 
>> +{
>> +return 0;
>> +}
>> +
>>  #endif /* CONFIG_MSI_DOORBELL */
>>  
>>  #endif
>> diff --git a/kernel/irq/msi-doorbell.c b/kernel/irq/msi-doorbell.c
>> index 0ff541e..a5bde37 100644
>> --- a/kernel/irq/msi-doorbell.c
>> +++ b/kernel/irq/msi-doorbell.c
>> @@ -60,3 +60,55 @@ void msi_doorbell_unregister_global(struct 
>> irq_chip_msi_doorbell_info *dbinfo)
>>  mutex_unlock(_doorbell_mutex);
>>  }
>>  EXPORT_SYMBOL_GPL(msi_doorbell_unregister_global);
>> +
>> +static int compute_db_mapping_requirements(phys_addr_t addr, size_t size,
>> +   unsigned int order)
>> +{
>> +phys_addr_t offset, granule;
>> +unsigned int nb_pages;
>> +
>> +granule = (uint64_t)(1 << order);
>> +offset = addr & (granule - 1);
>> +size = ALIGN(size + offset, granule);
>> +nb_pages = size >> order;
>> +
>> +return nb_pages;
>> +}
>> +
>> +static int
>> +compute_dbinfo_mapping_requirements(struct irq_chip_msi_doorbell_info 
>> *dbinfo,
>> +unsigned int order)
> 
> I'm sure you can find even longer function names which require more line
> breaks.
> 
>> +{
>> +int ret = 0;
>> +
>> +if (!dbinfo->doorbell_is_percpu) {
>> +ret = compute_db_mapping_requirements(dbinfo->global_doorbell,
>> +  dbinfo->size, order);
>> +} else {
>> +phys_addr_t __percpu *pbase;
>> +int cpu;
>> +
>> +for_each_possible_cpu(cpu) {
>> +pbase = per_cpu_ptr(dbinfo->percpu_doorbells, cpu);
>> +ret += compute_db_mapping_requirements(*pbase,
>> +   dbinfo->size,
>> +   order);
>> +}
>> +}
>> +return ret;
>> +}
>> +
>> +int msi_doorbell_pages(unsigned int order)
>> +{
>> +struct irqchip_doorbell *db;
>> +int ret = 0;
>> +
>> +mutex_lock(_doorbell_mutex);
>> +list_for_each_entry(db, _doorbell_list, next) {
> 
> Pointless braces
> 
>> +ret += compute_dbinfo_mapping_requirements(>info, order);
>> +}
>> +mutex_unlock(_doorbell_mutex);
>> +
>> +return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(msi_doorbell_pages);
> 
> So here is a general rant about your naming choices.
> 
>struct irqchip_doorbell
>struct irq_chip_msi_doorbell_info
> 
>struct irq_chip {
> *(*msi_doorbell_info);
>}
> 
>irqchip_doorbell_mutex
> 
>msi_doorbell_register_global
>msi_doorbell_unregister_global
> 
>msi_doorbell_pages
> 
> This really sucks. Your public functions start sensibly with msi_doorbell.
> 
> Though what is the _global postfix for the register/unregister functions for?
> Are there _private functions in the pipeline?
global is to be opposed to per-cpu (doorbell). Currently gicv2m and
gicv3-its expose a single "global" doorbell and I have not yet coped
with irqchips exposing per-cpu doorbells.
> 
> msi_doorbell_pages() is not telling me what it does. msi_calc_doorbell_pages()
> would describe it right away.
> 
> You doorbell info structure can really do with:
> 
> struct msi_doorbell_info;
> 
> And the wrapper struct around it is fine with:
> 
> struct msi_doorbell;
Yes you're right I will revisit the names and fix all style issues you
reported.

Thank you for your time

Eric
> 
> Thanks,
> 
>   tglx
> 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing

[Auger Eric]

Hi Marc,

On 19/07/2016 18:16, Marc Zyngier wrote:
> On 19/07/16 16:46, Paolo Bonzini wrote:
>>
>>
>> On 19/07/2016 16:56, Marc Zyngier wrote:
>>> On 18/07/16 14:25, Eric Auger wrote:
 This patch adds compilation and link against irqchip.

 Main motivation behind using irqchip code is to enable MSI
 routing code. In the future irqchip routing may also be useful
 when targeting multiple irqchips.

 Routing standard callbacks now are implemented in vgic-irqfd:
 - kvm_set_routing_entry
 - kvm_set_irq
 - kvm_set_msi

 They only are supported with new_vgic code.

 Both HAVE_KVM_IRQCHIP and HAVE_KVM_IRQ_ROUTING are defined.
 KVM_CAP_IRQ_ROUTING is advertised and KVM_SET_GSI_ROUTING is allowed.

 So from now on IRQCHIP routing is enabled and a routing table entry
 must exist for irqfd injection to succeed for a given SPI. This patch
 builds a default flat irqchip routing table (gsi=irqchip.pin) covering
 all the VGIC SPI indexes. This routing table is overwritten by the
 first first user-space call to KVM_SET_GSI_ROUTING ioctl.

 MSI routing setup is not yet allowed.

 Signed-off-by: Eric Auger 

 ---
 v6 -> v7:
 - re-introduce irq.h
 - use kvm_kernel_irq_routing_entry renamed fields: msi_flags, msi_devid
 - moved kvm_vgic_setup_default_irq_routing declaration in arm_vgic.h and
   definition in vgic-irqfd.c
 - correct double / in Makefile
 - remove BUG_ON(!vgic_initialized(kvm) in vgic_irqfd_set_irq since
   in any case we have a lazy_init in update_irq_pending
 - move KVM_IRQCHIP_NUM_PINS in arm_vgic.h
 - use VGIC_MAX_SPI

 v5 -> v6:
 - rebase on top of Andre's v8 + removal of old vgic

 v4 -> v5:
 - vgic_irqfd.c was renamed into vgic-irqfd.c by Andre
 - minor comment changes
 - remove trace_kvm_set_irq since it is called in irqchip
 - remove CONFIG_HAVE_KVM_MSI setting (done in KVM section)
 - despite Christoffer's question, in kvm_set_msi, I kept the copy from
   the input "struct kvm_kernel_irq_routing_entry *e" imposed by the
   irqchip callback API into the struct kvm_msi * passed to
   vits_inject_msi. Since vits_inject_msi is directly called by
   kvm_send_userspace_msi which takes a struct kvm_msi*, makes sense
   to me to keep the copy.
 - squash former [PATCH v4 5/7] KVM: arm/arm64: build a default routing
   table into that patch
 - handle default routing table alloc failure

 v3 -> v4:
 - provide support only for new-vgic
 - code previously in vgic.c now in vgic_irqfd.c

 v2 -> v3:
 - unconditionally set devid and KVM_MSI_VALID_DEVID flag as suggested
   by Andre (KVM_IRQ_ROUTING_EXTENDED_MSI type not used anymore)
 - vgic_irqfd_set_irq now is static
 - propagate flags
 - add comments

 v1 -> v2:
 - fix bug reported by Andre related to msi.flags and msi.devid setting
   in kvm_send_userspace_msi
 - avoid injecting reserved IRQ numbers in vgic_irqfd_set_irq

 RFC -> PATCH
 - reword api.txt:
   x move MSI routing comments in a subsequent patch,
   x clearly state GSI routing does not apply to KVM_IRQ_LINE
 ---
  Documentation/virtual/kvm/api.txt |  12 +++--
  arch/arm/kvm/Kconfig  |   2 +
  arch/arm/kvm/Makefile |   1 +
  arch/arm/kvm/irq.h|  19 +++
  arch/arm64/kvm/Kconfig|   2 +
  arch/arm64/kvm/Makefile   |   1 +
  arch/arm64/kvm/irq.h  |  19 +++
  include/kvm/arm_vgic.h|   7 +++
  virt/kvm/arm/vgic/vgic-init.c |   4 ++
  virt/kvm/arm/vgic/vgic-irqfd.c| 101 
 +++---
  virt/kvm/arm/vgic/vgic.c  |   7 ---
  11 files changed, 146 insertions(+), 29 deletions(-)
  create mode 100644 arch/arm/kvm/irq.h
  create mode 100644 arch/arm64/kvm/irq.h

 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 0065c8e..3bb60d3 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -1433,13 +1433,16 @@ KVM_ASSIGN_DEV_IRQ. Partial deassignment of host 
 or guest IRQ is allowed.
  4.52 KVM_SET_GSI_ROUTING
  
  Capability: KVM_CAP_IRQ_ROUTING
 -Architectures: x86 s390
 +Architectures: x86 s390 arm arm64
  Type: vm ioctl
  Parameters: struct kvm_irq_routing (in)
  Returns: 0 on success, -1 on error
  
  Sets the GSI routing table entries, overwriting any previously set 
 entries.
  
 +On arm/arm64, GSI routing has the following limitation:
 +- GSI routing does not apply to KVM_IRQ_LINE but only to KVM_IRQFD.
 +
  struct kvm_irq_routing {
__u32 nr;
__u32 flags;
 @@ -2368,9 +2371,10 @@ Note that closing the