KVM: Introduce direct MSI message injection for in-kernel irqchips

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space as to manage carefully.

By providing a direct injection with, we can both avoid using up limited

resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- rebased
- dropped KVM_MSI_FLAG_RAISE (but the IOCTL is still extensible)
- check for in-kernel irqchip presence added

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/kvm_main.c               |   21 +++++++++++++++++++++
 5 files changed, 57 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..dd6377e 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SET_MSI
+
+Capability: KVM_CAP_SET_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: 0 on success, -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..2dc24ac 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SET_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SET_MSI */
+#define KVM_SET_MSI               _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/kvm_main.c +21 -0

index a612bc8..ea9887e 100644
@@ -2063,6 +2063,24 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_SET_MSI: {
+		struct kvm_kernel_irq_routing_entry route;
+		struct kvm_msi msi;
+
+		r = -EFAULT;
+		if (copy_from_user(&msi, argp, sizeof msi))
+			goto out;
+		r = -EINVAL;
+		if (!irqchip_in_kernel(kvm) || msi.flags != 0)
+			goto out;
+		route.msi.address_lo = msi.address_lo;
+		route.msi.address_hi = msi.address_hi;
+		route.msi.data = msi.data;
+		r =  kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
+		break;
+	}
+#endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -2191,6 +2209,9 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	case KVM_CAP_SET_BOOT_CPU_ID:
 #endif
 	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SET_MSI:
+#endif
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 	case KVM_CAP_IRQ_ROUTING:

On Wed, Mar 28, 2012 at 10:47 AM, Jan Kiszka <[email protected]> wrote:
[...]

Documentation/virtual/kvm/api.txt See KVM_ASSIGN_DEV_IRQ for the data structure. The target device is specified · line 1491

Quoted 7 lines

+4.61 KVM_SET_MSI

Documentation/virtual/kvm/api.txt +6 -0

+
+Capability: KVM_CAP_SET_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: 0 on success, -1 on error

Is this the actual behavior? It looked to me like the successful return value ended up getting set by __apic_accept_irq(), which claims to "Return 1 if successfully added and 0 if discarded".

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space as to manage carefully.

By providing a direct injection with, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- rebased
- dropped KVM_MSI_FLAG_RAISE (but the IOCTL is still extensible)
- check for in-kernel irqchip presence added

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/kvm_main.c               |   21 +++++++++++++++++++++
 5 files changed, 57 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..dd6377e 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SET_MSI
+
+Capability: KVM_CAP_SET_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: 0 on success, -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..2dc24ac 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SET_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SET_MSI */
+#define KVM_SET_MSI               _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/kvm_main.c +21 -0

index a612bc8..ea9887e 100644
@@ -2063,6 +2063,24 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_SET_MSI: {
+		struct kvm_kernel_irq_routing_entry route;
+		struct kvm_msi msi;
+
+		r = -EFAULT;
+		if (copy_from_user(&msi, argp, sizeof msi))
+			goto out;
+		r = -EINVAL;
+		if (!irqchip_in_kernel(kvm) || msi.flags != 0)
+			goto out;
+		route.msi.address_lo = msi.address_lo;
+		route.msi.address_hi = msi.address_hi;
+		route.msi.data = msi.data;
+		r =  kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
+		break;
+	}
+#endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -2191,6 +2209,9 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	case KVM_CAP_SET_BOOT_CPU_ID:
 #endif
 	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SET_MSI:
+#endif
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 	case KVM_CAP_IRQ_ROUTING:

Signature · 1 line

1.7.3.4

Quoted 10 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space as to manage carefully.

By providing a direct injection with, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

Looks straight-forward to me. Others noted some documentation nits, so I know you are going to repost, anyway. When you do how about renaming SET_MSI -> SEND_MSI or SIGNAL_MSI ?
We don't set anything, as such ...

I know we have kvm_set_msi internally but this is more or less a misnomer.

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space as to manage

carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- align return code doc to reality
- rename SET_MSI -> SIGNAL_MSI

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/kvm_main.c               |   21 +++++++++++++++++++++
 5 files changed, 57 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..225b452 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SIGNAL_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SIGNAL_MSI */
+#define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/kvm_main.c +21 -0

index a612bc8..3aeb7ab 100644
@@ -2063,6 +2063,24 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_SIGNAL_MSI: {
+		struct kvm_kernel_irq_routing_entry route;
+		struct kvm_msi msi;
+
+		r = -EFAULT;
+		if (copy_from_user(&msi, argp, sizeof msi))
+			goto out;
+		r = -EINVAL;
+		if (!irqchip_in_kernel(kvm) || msi.flags != 0)
+			goto out;
+		route.msi.address_lo = msi.address_lo;
+		route.msi.address_hi = msi.address_hi;
+		route.msi.data = msi.data;
+		r =  kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
+		break;
+	}
+#endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -2191,6 +2209,9 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	case KVM_CAP_SET_BOOT_CPU_ID:
 #endif
 	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SIGNAL_MSI:
+#endif
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 	case KVM_CAP_IRQ_ROUTING:

Signature · 1 line

1.7.3.4

Quoted 4 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space as to manage

s/as/has/

Quoted 7 lines

carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

Further, we can ensure better, deterministic worst-case performance for real-time guests with large number of vectors.

Acked-by Michael S. Tsirkin <[email protected]>

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space as to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- align return code doc to reality
- rename SET_MSI -> SIGNAL_MSI

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/kvm_main.c               |   21 +++++++++++++++++++++
 5 files changed, 57 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..225b452 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SIGNAL_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SIGNAL_MSI */
+#define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/kvm_main.c +0 -0

index a612bc8..3aeb7ab 100644
@@ -2063,6 +2063,24 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif

virt/kvm/kvm_main.c +11 -0

+			goto out;
+		route.msi.address_lo = msi.address_lo;
+		route.msi.address_hi = msi.address_hi;
+		route.msi.data = msi.data;
+		r =  kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
+		break;
+	}
+#endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -2191,6 +2209,9 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	case KVM_CAP_SET_BOOT_CPU_ID:
 #endif
 	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SIGNAL_MSI:
+#endif
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 	case KVM_CAP_IRQ_ROUTING:

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v4:
- Fix the build by factoring out kvm_send_userspace_msi:
irqchip_in_kernel is not generically available. But abstracting it for all arch is tricky and therefore left to the poor people who have to introduce non-x86 irqchip support to this x86-focused corner.

Lesson (probably not) learned: Never underestimate the complexity of trivial changes.

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 include/linux/kvm_host.h          |    2 ++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/irq_comm.c               |   14 ++++++++++++++
 virt/kvm/kvm_main.c               |   14 ++++++++++++++
 7 files changed, 66 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..225b452 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SIGNAL_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SIGNAL_MSI */
+#define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

include/linux/kvm_host.h +2 -0

index 6cf158c..35c69d6 100644
@@ -774,6 +774,8 @@ int kvm_set_irq_routing(struct kvm *kvm,
 			unsigned flags);
 void kvm_free_irq_routing(struct kvm *kvm);
 
+int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
+
 #else
 
 static inline void kvm_free_irq_routing(struct kvm *kvm) {}

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/irq_comm.c +14 -0

index 9f614b4..a6a0365 100644
@@ -138,6 +138,20 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
 }
 
+int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
+{
+	struct kvm_kernel_irq_routing_entry route;
+
+	if (!irqchip_in_kernel(kvm) || msi->flags != 0)
+		return -EINVAL;
+
+	route.msi.address_lo = msi->address_lo;
+	route.msi.address_hi = msi->address_hi;
+	route.msi.data = msi->data;
+
+	return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
+}
+
 /*
  * Return value:
  *  < 0   Interrupt was ignored (masked or not delivered for other reasons)

virt/kvm/kvm_main.c +0 -0

index a612bc8..5addfa8 100644

virt/kvm/kvm_main.c +7 -0

+		r = kvm_send_userspace_msi(kvm, &msi);
+		break;
+	}
+#endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -2191,6 +2202,9 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	case KVM_CAP_SET_BOOT_CPU_ID:
 #endif
 	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SIGNAL_MSI:
+#endif
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 	case KVM_CAP_IRQ_ROUTING:

Signature · 1 line

1.7.3.4

Quoted 169 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v4:
- Fix the build by factoring out kvm_send_userspace_msi:
irqchip_in_kernel is not generically available. But abstracting it for all arch is tricky and therefore left to the poor people who have to introduce non-x86 irqchip support to this x86-focused corner.

Lesson (probably not) learned: Never underestimate the complexity of trivial changes.

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 include/linux/kvm_host.h          |    2 ++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/irq_comm.c               |   14 ++++++++++++++
 virt/kvm/kvm_main.c               |   14 ++++++++++++++
 7 files changed, 66 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..225b452 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SIGNAL_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SIGNAL_MSI */
+#define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

include/linux/kvm_host.h +2 -0

index 6cf158c..35c69d6 100644
@@ -774,6 +774,8 @@ int kvm_set_irq_routing(struct kvm *kvm,
 			unsigned flags);
 void kvm_free_irq_routing(struct kvm *kvm);
 
+int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
+
 #else
 
 static inline void kvm_free_irq_routing(struct kvm *kvm) {}

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/irq_comm.c +14 -0

index 9f614b4..a6a0365 100644
@@ -138,6 +138,20 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
 }
 
+int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
+{
+	struct kvm_kernel_irq_routing_entry route;
+
+	if (!irqchip_in_kernel(kvm) || msi->flags != 0)
+		return -EINVAL;
+
+	route.msi.address_lo = msi->address_lo;
+	route.msi.address_hi = msi->address_hi;
+	route.msi.data = msi->data;
+
+	return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
+}
+
 /*
  * Return value:
  *  < 0   Interrupt was ignored (masked or not delivered for other reasons)

virt/kvm/kvm_main.c +2 -0

index a612bc8..5addfa8 100644
@@ -2063,6 +2063,17 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_SIGNAL_MSI: {

virt/kvm/kvm_main.c static long kvm_vm_ioctl(struct file *filp, · line 2072

Quoted 5 lines

virt/kvm/kvm_main.c +5 -0

+		struct kvm_msi msi;
+
+		r = -EFAULT;
+		if (copy_from_user(&msi, argp, sizeof msi))
+			goto out;

Wait a second, we should validate flags: you say
"this field should be 0" so we should check it.

Quoted 20 lines

virt/kvm/kvm_main.c +7 -0

+		r = kvm_send_userspace_msi(kvm, &msi);
+		break;
+	}
+#endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -2191,6 +2202,9 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	case KVM_CAP_SET_BOOT_CPU_ID:
 #endif
 	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SIGNAL_MSI:
+#endif
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 	case KVM_CAP_IRQ_ROUTING:

Signature · 1 line

1.7.3.4

Quoted 170 lines · Michael S. Tsirkin

Quoted 169 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v4:
- Fix the build by factoring out kvm_send_userspace_msi:
irqchip_in_kernel is not generically available. But abstracting it for all arch is tricky and therefore left to the poor people who have to introduce non-x86 irqchip support to this x86-focused corner.

Lesson (probably not) learned: Never underestimate the complexity of trivial changes.

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 include/linux/kvm_host.h          |    2 ++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/irq_comm.c               |   14 ++++++++++++++
 virt/kvm/kvm_main.c               |   14 ++++++++++++++
 7 files changed, 66 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..225b452 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SIGNAL_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SIGNAL_MSI */
+#define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

include/linux/kvm_host.h +2 -0

index 6cf158c..35c69d6 100644
@@ -774,6 +774,8 @@ int kvm_set_irq_routing(struct kvm *kvm,
 			unsigned flags);
 void kvm_free_irq_routing(struct kvm *kvm);
 
+int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
+
 #else
 
 static inline void kvm_free_irq_routing(struct kvm *kvm) {}

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/irq_comm.c +14 -0

index 9f614b4..a6a0365 100644
@@ -138,6 +138,20 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
 }
 
+int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
+{
+	struct kvm_kernel_irq_routing_entry route;
+
+	if (!irqchip_in_kernel(kvm) || msi->flags != 0)
+		return -EINVAL;
+
+	route.msi.address_lo = msi->address_lo;
+	route.msi.address_hi = msi->address_hi;
+	route.msi.data = msi->data;
+
+	return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
+}
+
 /*
  * Return value:
  *  < 0   Interrupt was ignored (masked or not delivered for other reasons)

virt/kvm/kvm_main.c +2 -0

index a612bc8..5addfa8 100644
@@ -2063,6 +2063,17 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_SIGNAL_MSI: {

Quoted 8 lines

virt/kvm/kvm_main.c static long kvm_vm_ioctl(struct file *filp, · line 2072

Quoted 5 lines

virt/kvm/kvm_main.c +5 -0

+		struct kvm_msi msi;
+
+		r = -EFAULT;
+		if (copy_from_user(&msi, argp, sizeof msi))
+			goto out;

Wait a second, we should validate flags: you say
"this field should be 0" so we should check it.

Yep, see kvm_send_userspace_msi.

Jan

Quoted 20 lines

Quoted 20 lines

virt/kvm/kvm_main.c +7 -0

+		r = kvm_send_userspace_msi(kvm, &msi);
+		break;
+	}
+#endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)

@@ -2191,6 +2202,9 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	case KVM_CAP_SET_BOOT_CPU_ID:
 #endif
 	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SIGNAL_MSI:
+#endif
 		return 1;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
 	case KVM_CAP_IRQ_ROUTING:

Signature · 1 line

1.7.3.4

Signature · 2 lines

Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Documentation/virtual/kvm/api.txt +11 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+

Quoted 28 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Documentation/virtual/kvm/api.txt +11 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+

Documentation/virtual/kvm/api.txt See KVM_ASSIGN_DEV_IRQ for the data structure. The target device is specified · line 1505

Quoted 10 lines

Documentation/virtual/kvm/api.txt +9 -0

+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
 

There are two ways in which this can be generalized:

struct kvm_general_irq {
      __u32 type; // line | MSI
      __u32 op;  // raise/lower/trigger
      union {
                 ... line;
                 struct kvm_msi msi;
      }
};

so we have a single ioctl for all interrupt handling. This allows eventual removal of the line-oriented ioctls.

The other alternative is to have a dma interface, similar to the kvm_run mmio interface but with the kernel acting as destination. The advantage here is that we can handle dma from a device to any kernel-emulated device, not just the APIC MSI range. A downside is that we can't return values related to interrupt coalescing.

A performance note: delivering an interrupt needs to search all vcpus for an APIC ID match. The previous plan was to cache (or pre-calculate) this lookup in the irq routing table. Now it looks like we'll need a separate cache for this.

(yes, I said on the call I don't anticipate objections but preparing to apply a patch always triggers more critical thinking)

Quoted 4 lines

so we have a single ioctl for all interrupt handling. This allows eventual removal of the line-oriented ioctls.

Quoted 5 lines

The other alternative is to have a dma interface, similar to the kvm_run mmio interface but with the kernel acting as destination. The advantage here is that we can handle dma from a device to any kernel-emulated device, not just the APIC MSI range. A downside is that we can't return values related to interrupt coalescing.

Due to lacking injection feedback, I'm in favor of option 1. Will have a look.

Quoted 5 lines

A performance note: delivering an interrupt needs to search all vcpus for an APIC ID match. The previous plan was to cache (or pre-calculate) this lookup in the irq routing table. Now it looks like we'll need a separate cache for this.

As this is non-existent until today, we don't regress here. And it can still be added on top later on, transparently.

Quoted 4 lines · Jan Kiszka

Quoted 4 lines

so we have a single ioctl for all interrupt handling. This allows eventual removal of the line-oriented ioctls.

Quoted 8 lines

Quoted 5 lines

The other alternative is to have a dma interface, similar to the kvm_run mmio interface but with the kernel acting as destination. The advantage here is that we can handle dma from a device to any kernel-emulated device, not just the APIC MSI range. A downside is that we can't return values related to interrupt coalescing.

Due to lacking injection feedback, I'm in favor of option 1. Will have a look.

I wonder if we can create a side channel for it. Lack of a kernel DMA API is a hole in the current code, though we haven't been bitten by it yet. An example is a guest that is swapping its own page tables; right now the shadow mmu doesn't notice those writes (when the page tables are swapped in) and will deliver incorrect results. Of course no guest does that, so it doesn't happen in practice.

Quoted 6 lines

Quoted 5 lines

A performance note: delivering an interrupt needs to search all vcpus for an APIC ID match. The previous plan was to cache (or pre-calculate) this lookup in the irq routing table. Now it looks like we'll need a separate cache for this.

Quoted 2 lines

As this is non-existent until today, we don't regress here. And it can still be added on top later on, transparently.

Yes, it's just a note, not an objection. The cache lookup will be slower than the gsi lookup (hash table vs. array) but still O(1) vs. the current O(n).

Quoted 10 lines

Quoted 9 lines

Quoted 6 lines

A performance note: delivering an interrupt needs to search all vcpus for an APIC ID match. The previous plan was to cache (or pre-calculate) this lookup in the irq routing table. Now it looks like we'll need a separate cache for this.

As this is non-existent until today, we don't regress here. And it can still be added on top later on, transparently.

Quoted 3 lines

Yes, it's just a note, not an objection. The cache lookup will be slower than the gsi lookup (hash table vs. array) but still O(1) vs. the current O(n).

If you are concerned about performance in this path, wouldn't a DMA interface for MSI injection be counterproductive?

Quoted 51 lines · Avi Kivity

Quoted 28 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Documentation/virtual/kvm/api.txt +11 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+

Documentation/virtual/kvm/api.txt See KVM_ASSIGN_DEV_IRQ for the data structure. The target device is specified · line 1505

Quoted 10 lines

Documentation/virtual/kvm/api.txt +9 -0

+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
 

There are two ways in which this can be generalized:

struct kvm_general_irq {
      __u32 type; // line | MSI
      __u32 op;  // raise/lower/trigger
      union {
                 ... line;
                 struct kvm_msi msi;
      }
};

Quoted 8 lines

so we have a single ioctl for all interrupt handling. This allows eventual removal of the line-oriented ioctls.

The other alternative is to have a dma interface, similar to the kvm_run mmio interface but with the kernel acting as destination. The advantage here is that we can handle dma from a device to any kernel-emulated device, not just the APIC MSI range. A downside is that we can't return values related to interrupt coalescing.

Due to lacking injection feedback, I'm in favor of option 1. Will have a look.

Quoted 1 line

Quoted 4 lines

A performance note: delivering an interrupt needs to search all vcpus for an APIC ID match. The previous plan was to cache (or pre-calculate) this lookup in the irq routing table. Now it looks like we'll need a separate cache for this.

As this is non-existent until today, we don't regress here. And it can still be added on top later on, transparently.

Quoted 4 lines

(yes, I said on the call I don't anticipate objections but preparing to apply a patch always triggers more critical thinking)

Well, we make progress, though slower than I was hoping. :)

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Quoted 7 lines

Quoted 6 lines

A performance note: delivering an interrupt needs to search all vcpus for an APIC ID match. The previous plan was to cache (or pre-calculate) this lookup in the irq routing table. Now it looks like we'll need a separate cache for this.

Quoted 2 lines

As this is non-existent until today, we don't regress here. And it can still be added on top later on, transparently.

I always worry about hash collisions and the cost of calculating good hash functions.

We could instead return an index in the cache on injection, maintain in userspace and use it for fast path on the next injection.

Will make it easy to use an array index instead of a hash here, and fallback to a slower ID lookup on mismatch.

Until we do have this fast path we can just fill this value with zeros, so kernel patch (almost) does not need to change for this - just the header.

Quoted 7 lines · Michael S. Tsirkin

Quoted 7 lines

Quoted 6 lines

A performance note: delivering an interrupt needs to search all vcpus for an APIC ID match. The previous plan was to cache (or pre-calculate) this lookup in the irq routing table. Now it looks like we'll need a separate cache for this.

Quoted 8 lines

Quoted 2 lines

As this is non-existent until today, we don't regress here. And it can still be added on top later on, transparently.

I always worry about hash collisions and the cost of calculating good hash functions.

We could instead return an index in the cache on injection, maintain in userspace and use it for fast path on the next injection.

Ahem, that is almost the existing routing table to a T.

Quoted 2 lines

Will make it easy to use an array index instead of a hash here, and fallback to a slower ID lookup on mismatch.

Need a free ioctl so we can reuse IDs.

Quoted 3 lines

Until we do have this fast path we can just fill this value with zeros, so kernel patch (almost) does not need to change for this - just the header.

Partially implemented interfaces invite breakage.

Signature · 3 lines

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Quoted 17 lines · Avi Kivity

Quoted 7 lines · Michael S. Tsirkin

Quoted 7 lines

Quoted 6 lines

A performance note: delivering an interrupt needs to search all vcpus for an APIC ID match. The previous plan was to cache (or pre-calculate) this lookup in the irq routing table. Now it looks like we'll need a separate cache for this.

Quoted 8 lines

Quoted 2 lines

As this is non-existent until today, we don't regress here. And it can still be added on top later on, transparently.

I always worry about hash collisions and the cost of calculating good hash functions.

We could instead return an index in the cache on injection, maintain in userspace and use it for fast path on the next injection.

Quoted 6 lines

Ahem, that is almost the existing routing table to a T.

Quoted 2 lines

Will make it easy to use an array index instead of a hash here, and fallback to a slower ID lookup on mismatch.

Need a free ioctl so we can reuse IDs.

No, it could be kernel controlled not userspace controlled. We get both and address and an index:

if (table[u.i].addr == u.addr && table[u.i].data == u.data) {
	return table[u.i].id;
}

u.i = find_lru_idx(&table);
table[u.i].addr = u.addr;
table[u.i].data = u.data;
table[u.i].id = find_id(u.addr, u.data);
return table[u.i].id;

Quoted 5 lines

Quoted 3 lines

Until we do have this fast path we can just fill this value with zeros, so kernel patch (almost) does not need to change for this - just the header.

Partially implemented interfaces invite breakage.

Hmm true. OK scrap this idea then, it's not clear
whether we are going to optimize this anyway.

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

Until we do have this fast path we can just fill this value with zeros, so kernel patch (almost) does not need to change for this - just the header.

Partially implemented interfaces invite breakage.

Quoted 3 lines

Hmm true. OK scrap this idea then, it's not clear
whether we are going to optimize this anyway.

Also, the problem is that keeping that ID in userspace requires an infrastructure like the MSIRoutingCache that I proposed originally. Not much won /wrt invasiveness there.

So we should really do the routing
optimization in the kernel - one day.

Quoted 11 lines · Jan Kiszka

Quoted 10 lines

Quoted 6 lines

Quoted 4 lines

Until we do have this fast path we can just fill this value with zeros, so kernel patch (almost) does not need to change for this - just the header.

Partially implemented interfaces invite breakage.

Hmm true. OK scrap this idea then, it's not clear
whether we are going to optimize this anyway.

Quoted 3 lines

Also, the problem is that keeping that ID in userspace requires an infrastructure like the MSIRoutingCache that I proposed originally. Not much won /wrt invasiveness there.

Internal qemu refactorings are not a driver for kvm interface changes.

Quoted 3 lines

So we should really do the routing
optimization in the kernel - one day.

No, we need to make a choice:

explicit handles: array lookup, more expensive setup
no handles: hash loopup, more expensive, but no setup, and no artificial limits

Quoted 12 lines · Avi Kivity

Quoted 11 lines · Jan Kiszka

Quoted 10 lines

Quoted 6 lines

Quoted 4 lines

Until we do have this fast path we can just fill this value with zeros, so kernel patch (almost) does not need to change for this - just the header.

Partially implemented interfaces invite breakage.

Hmm true. OK scrap this idea then, it's not clear
whether we are going to optimize this anyway.

Quoted 5 lines

Quoted 3 lines

Also, the problem is that keeping that ID in userspace requires an infrastructure like the MSIRoutingCache that I proposed originally. Not much won /wrt invasiveness there.

Internal qemu refactorings are not a driver for kvm interface changes.

No, but qemu demonstrates the applicability and handiness of the kernel interfaces.

Quoted 9 lines

Quoted 3 lines

So we should really do the routing
optimization in the kernel - one day.

No, we need to make a choice:

explicit handles: array lookup, more expensive setup
no handles: hash loopup, more expensive, but no setup, and no artificial limits

...and I think we should head for option 2.

Quoted 13 lines · Jan Kiszka

Quoted 12 lines · Avi Kivity

Quoted 11 lines · Jan Kiszka

Quoted 10 lines

Quoted 6 lines

Quoted 4 lines

Until we do have this fast path we can just fill this value with zeros, so kernel patch (almost) does not need to change for this - just the header.

Partially implemented interfaces invite breakage.

Hmm true. OK scrap this idea then, it's not clear
whether we are going to optimize this anyway.

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Also, the problem is that keeping that ID in userspace requires an infrastructure like the MSIRoutingCache that I proposed originally. Not much won /wrt invasiveness there.

Internal qemu refactorings are not a driver for kvm interface changes.

No, but qemu demonstrates the applicability and handiness of the kernel interfaces.

True.

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

So we should really do the routing
optimization in the kernel - one day.

No, we need to make a choice:

Quoted 5 lines

Quoted 3 lines

explicit handles: array lookup, more expensive setup
no handles: hash loopup, more expensive, but no setup, and no artificial limits

...and I think we should head for option 2.

I'm not so sure anymore. Sorry about the U turn, but remind me why? In the long term it will be slower.

Quoted 3 lines

I'm not so sure anymore. Sorry about the U turn, but remind me why? In the long term it will be slower.

Likely not measurably slower. If you look at a message through the arch glasses, you can usually spot the destination directly, specifically if a message targets a single processor - no need for hashing and table lookups in the common case.

In contrast, the maintenance costs for the current explicit route based model are significant as we see now.

Quoted 8 lines · Jan Kiszka

Quoted 3 lines

I'm not so sure anymore. Sorry about the U turn, but remind me why? In the long term it will be slower.

Likely not measurably slower. If you look at a message through the arch glasses, you can usually spot the destination directly, specifically if a message targets a single processor - no need for hashing and table lookups in the common case.

Not on x86. The APIC ID is guest-provided.

In x2apic mode it can be
quite large.

Quoted 4 lines

In contrast, the maintenance costs for the current explicit route based model are significant as we see now.

You mean in amount of code in userspace? That doesn't get solved since we need to keep compatibility.

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land. This path is provide in a way that allows for use with other interrupt sources as well. Besides MSIs also external interrupt lines can be manipulated through this interface, obsoleting KVM_IRQ_LINE_STATUS.

Signed-off-by Jan Kiszka <[email protected]>

---

This picks up Avi's first suggestion as I still think it is the better option to provide a direct MSI injection channel.

Quoted 12 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land. This path is provide in a way that allows for use with other interrupt sources as well. Besides MSIs also external interrupt lines can be manipulated through this interface, obsoleting KVM_IRQ_LINE_STATUS.

Quoted 5 lines

Signed-off-by Jan Kiszka <[email protected]>

---

This picks up Avi's first suggestion as I still think it is the better option to provide a direct MSI injection channel.

Ping. What's now the preferred approach? I'd like to make some progress with this topic.

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land. This path is provide in a way that allows for use with other interrupt sources as well. Besides MSIs also external interrupt lines can be manipulated through this interface, obsoleting KVM_IRQ_LINE_STATUS.

Signed-off-by Jan Kiszka <[email protected]>

---

This picks up Avi's first suggestion as I still think it is the better option to provide a direct MSI injection channel.

Quoted 12 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land. This path is provide in a way that allows for use with other interrupt sources as well. Besides MSIs also external interrupt lines can be manipulated through this interface, obsoleting KVM_IRQ_LINE_STATUS.

Quoted 5 lines

Signed-off-by Jan Kiszka <[email protected]>

---

This picks up Avi's first suggestion as I still think it is the better option to provide a direct MSI injection channel.

My main objection to the previous patch was that it's not needed; qemu has to work with older kernels that have neither this nor the other patch. Given that, why do anything further? It won't be cleaner because the ugly path will remain for compatibility.

Is there any concrete problem that this solves, that cannot be solved by a pure (but ugly) user space solution?

wrt the patch itself, it seems fine. I have to agree that the single-purpose ioctl looks cleaner (but may be less suitable for a syscall based API).

Just to add some confusion: is this future proof wrt iommu/interrupt remapping emulation? If you have a single iommu that intercepts all of the MSI space then there's no problem, but if there are multiple iommus, or if some devices are "in front of" the iommu, then we need to identify the source of the message as well. So we'd need an MBZ field for MSI injection, which can later be filled with the source ID.

Quoted 24 lines · Avi Kivity

Quoted 12 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land. This path is provide in a way that allows for use with other interrupt sources as well. Besides MSIs also external interrupt lines can be manipulated through this interface, obsoleting KVM_IRQ_LINE_STATUS.

Quoted 5 lines

Signed-off-by Jan Kiszka <[email protected]>

---

This picks up Avi's first suggestion as I still think it is the better option to provide a direct MSI injection channel.

My main objection to the previous patch was that it's not needed; qemu has to work with older kernels that have neither this nor the other patch. Given that, why do anything further? It won't be cleaner because the ugly path will remain for compatibility.

Quoted 2 lines

Is there any concrete problem that this solves, that cannot be solved by a pure (but ugly) user space solution?

As I explained a few times: We avoid taking the lengthy path in userspace, GSI exhaustion, sporadic routing flushes etc. when running on a modern kernel. The "ugly" part in userspace is rather small as we can refrain from optimizing it. Specifically we do not need to touch QEMU all over the MSI path just for KVM.

Quoted 4 lines

wrt the patch itself, it seems fine. I have to agree that the single-purpose ioctl looks cleaner (but may be less suitable for a syscall based API).

Why would some MSI-only injection IOCTL be problematic for a syscall model?

I rather suspect the generic IOCTL may have some limitations as it /might/ be used for mixing broadcasts with solely VCPU-targeted IRQ injections one day, on some arch.

Quoted 7 lines

Just to add some confusion: is this future proof wrt iommu/interrupt remapping emulation? If you have a single iommu that intercepts all of the MSI space then there's no problem, but if there are multiple iommus, or if some devices are "in front of" the iommu, then we need to identify the source of the message as well. So we'd need an MBZ field for MSI injection, which can later be filled with the source ID.

We will need hierarchical dispatching, using additional source information. But that information is totally unrelated to the pseudo GSI currently used by KVM for the final delivery step to the APIC bus. In fact, the whole message content can be unrelated as it can be modified and even coalesced with other sources along its way to the KVM injection API.

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Quoted 6 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

Quoted 4 lines

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Applied to queue (for 3.5).

Thanks for your patience.

Quoted 12 lines · Avi Kivity

Quoted 10 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Quoted 3 lines

Applied to queue (for 3.5).

Thanks for your patience.

Oops, that was now unexpectedly fast.

Extending and slightly reformatting the API docs I noticed some inconsistency. Will send fixes soon. Can you fold this into my patch, or just apply it on top?

Quoted 13 lines · Jan Kiszka

Quoted 12 lines · Avi Kivity

Quoted 10 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space, IRQ routes are a limited resource that user space has to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Quoted 5 lines

Quoted 3 lines

Applied to queue (for 3.5).

Thanks for your patience.

Oops, that was now unexpectedly fast.

I hope you don't mean the ~ 1 month timeframe for the whole thing.

Quoted 6 lines

Extending and slightly reformatting the API docs I noticed some inconsistency. Will send fixes soon. Can you fold this into my patch, or just apply it on top?

Since it's just in queue, not next, will fold into parent patch.

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space,

IRQ routes are a limited resource that user space as to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- align return code doc to reality
- rename SET_MSI -> SIGNAL_MSI

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/kvm_main.c               |   21 +++++++++++++++++++++
 5 files changed, 57 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..225b452 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SIGNAL_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SIGNAL_MSI */
+#define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/kvm_main.c +0 -0

index a612bc8..3aeb7ab 100644

Quoted 3 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space,

The MSI message format is configured on device configuration, and once its settled, does not change. This should be an unfrequent operation, no? (i am trying to understand what you mean by "on the fly" here).

If that is the case, the real problem is that irq routing tables do not handle large numbers of vectors? And isnt that limitation also an issue if you'd like to add more IOAPICs, for example?

Quoted 117 lines

IRQ routes are a limited resource that user space as to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- align return code doc to reality
- rename SET_MSI -> SIGNAL_MSI

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/kvm_main.c               |   21 +++++++++++++++++++++
 5 files changed, 57 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..225b452 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SIGNAL_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SIGNAL_MSI */
+#define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/kvm_main.c +0 -0

index a612bc8..3aeb7ab 100644

virt/kvm/kvm_main.c static long kvm_vm_ioctl(struct file *filp, · line 2069

Quoted 7 lines

virt/kvm/kvm_main.c +4 -0

@@ -2063,6 +2063,24 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_SIGNAL_MSI: {
+		struct kvm_kernel_irq_routing_entry route;
+		struct kvm_msi msi;

Zero them (future proof).

virt/kvm/kvm_main.c static long kvm_vm_ioctl(struct file *filp, · line 2080

Quoted 11 lines

virt/kvm/kvm_main.c +11 -0

+
+		r = -EFAULT;
+		if (copy_from_user(&msi, argp, sizeof msi))
+			goto out;
+		r = -EINVAL;
+		if (!irqchip_in_kernel(kvm) || msi.flags != 0)
+			goto out;
+		route.msi.address_lo = msi.address_lo;
+		route.msi.address_hi = msi.address_hi;
+		route.msi.data = msi.data;
+		r =  kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);

extra whitespace.

virt/kvm/kvm_main.c static long kvm_vm_ioctl(struct file *filp, · line 2082

Quoted 2 lines

virt/kvm/kvm_main.c +2 -0

+		break;
+	}

Have you checked that direct MSI injection does not make use of IRQ routing data structures, such as for acking?

irqchip_in_kernel(kvm) returns true before kvm->irq_routing is actually in place. With kvm_set_irq there is no problem, but now there is another path into injection.

The real purpose of this is not entirely clear (and as Avi mentioned two interfaces should be avoided if possible).

Quoted 8 lines · Marcelo Tosatti

Quoted 3 lines · Jan Kiszka

Currently, MSI messages can only be injected to in-kernel irqchips by defining a corresponding IRQ route for each message. This is not only unhandy if the MSI messages are generated "on the fly" by user space,

The MSI message format is configured on device configuration, and once its settled, does not change. This should be an unfrequent operation, no? (i am trying to understand what you mean by "on the fly" here).

The point is that you need to track those changes and/or provide mechanism to cache routing information related to a specific MSI source. That either means patching the full path from the source to the sink or paying some price on injection.

Quoted 4 lines

If that is the case, the real problem is that irq routing tables do not handle large numbers of vectors? And isnt that limitation also an issue if you'd like to add more IOAPICs, for example?

For sure, more IRQ lines will also contribute to the shortage of GSIs. Yet another reason to avoid wasting them on userspace generated MSIs.

Quoted 141 lines

Quoted 117 lines

IRQ routes are a limited resource that user space as to manage carefully.

By providing a direct injection path, we can both avoid using up limited resources and simplify the necessary steps for user land.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- align return code doc to reality
- rename SET_MSI -> SIGNAL_MSI

 Documentation/virtual/kvm/api.txt |   21 +++++++++++++++++++++
 arch/x86/kvm/Kconfig              |    1 +
 include/linux/kvm.h               |   11 +++++++++++
 virt/kvm/Kconfig                  |    3 +++
 virt/kvm/kvm_main.c               |   21 +++++++++++++++++++++
 5 files changed, 57 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +21 -0

index 81ff39f..ed27d1b 100644
@@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure.  The target device is specified
 by assigned_dev_id.  In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is
 evaluated.
 
+4.61 KVM_SIGNAL_MSI
+
+Capability: KVM_CAP_SIGNAL_MSI
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_msi (in)
+Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
+
+Directly inject a MSI message. Only valid with in-kernel irqchip that handles
+MSI messages.
+
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
+No flags are defined so far. The corresponding field must be 0.
+
 4.62 KVM_CREATE_SPAPR_TCE
 
 Capability: KVM_CAP_SPAPR_TCE

arch/x86/kvm/Kconfig +1 -0

index 1a7fe86..a28f338 100644
@@ -36,6 +36,7 @@ config KVM
 	select TASKSTATS
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
+	select HAVE_KVM_MSI
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent

include/linux/kvm.h +11 -0

index 7a9dd4b..225b452 100644
@@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_SYNC_REGS 74
 #define KVM_CAP_PCI_2_3 75
 #define KVM_CAP_KVMCLOCK_CTRL 76
+#define KVM_CAP_SIGNAL_MSI 77
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -715,6 +716,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+struct kvm_msi {
+	__u32 address_lo;
+	__u32 address_hi;
+	__u32 data;
+	__u32 flags;
+	__u8  pad[16];
+};
+
 /*
  * ioctls for VM fds
  */

@@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PCI_2_3 */
 #define KVM_ASSIGN_SET_INTX_MASK  _IOW(KVMIO,  0xa4, \
 				       struct kvm_assigned_pci_dev)
+/* Available with KVM_CAP_SIGNAL_MSI */
+#define KVM_SIGNAL_MSI            _IOW(KVMIO,  0xa5, struct kvm_msi)
 
 /*
  * ioctls for vcpu fds

virt/kvm/Kconfig +3 -0

index f63ccb0..28694f4 100644
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
        bool
+
+config HAVE_KVM_MSI
+       bool

virt/kvm/kvm_main.c +0 -0

index a612bc8..3aeb7ab 100644

virt/kvm/kvm_main.c static long kvm_vm_ioctl(struct file *filp, · line 2069

Quoted 7 lines

virt/kvm/kvm_main.c +4 -0

@@ -2063,6 +2063,24 @@ static long kvm_vm_ioctl(struct file *filp,
 		mutex_unlock(&kvm->lock);
 		break;
 #endif
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_SIGNAL_MSI: {
+		struct kvm_kernel_irq_routing_entry route;
+		struct kvm_msi msi;

Zero them (future proof).

virt/kvm/kvm_main.c static long kvm_vm_ioctl(struct file *filp, · line 2080

Quoted 11 lines

virt/kvm/kvm_main.c +11 -0

+
+		r = -EFAULT;
+		if (copy_from_user(&msi, argp, sizeof msi))
+			goto out;
+		r = -EINVAL;
+		if (!irqchip_in_kernel(kvm) || msi.flags != 0)
+			goto out;
+		route.msi.address_lo = msi.address_lo;
+		route.msi.address_hi = msi.address_hi;
+		route.msi.data = msi.data;
+		r =  kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);

extra whitespace.

Quoted 5 lines

virt/kvm/kvm_main.c static long kvm_vm_ioctl(struct file *filp, · line 2082

Quoted 2 lines

virt/kvm/kvm_main.c +2 -0

+		break;
+	}

Have you checked that direct MSI injection does not make use of IRQ routing data structures, such as for acking?

See kvm_set_msi: The routing structure is only read in the context of that function, no reference is kept.

Quoted 6 lines

irqchip_in_kernel(kvm) returns true before kvm->irq_routing is actually in place. With kvm_set_irq there is no problem, but now there is another path into injection.

The real purpose of this is not entirely clear (and as Avi mentioned two interfaces should be avoided if possible).

See [1] for an implementation of one of Avi's proposals.

Quoted 5 lines

Quoted 2 lines

Have you checked that direct MSI injection does not make use of IRQ routing data structures, such as for acking?

See kvm_set_msi: The routing structure is only read in the context of that function, no reference is kept.

I was thinking more along the lines of inconsistent state due to races.

- set MSI for IOAPIC handled vector before kvm->irq_routing is assigned.
- IOAPIC EOI for that vector.
- EOI handler expects kvm->irq_routing present.

Quoted 9 lines

Quoted 7 lines

irqchip_in_kernel(kvm) returns true before kvm->irq_routing is actually in place. With kvm_set_irq there is no problem, but now there is another path into injection.

The real purpose of this is not entirely clear (and as Avi mentioned two interfaces should be avoided if possible).

See [1] for an implementation of one of Avi's proposals.

Just now that i start to appreciate KVM_SIGNAL_MSI.

"so we have a single ioctl for all interrupt handling. This allows eventual removal of the line-oriented ioctls."

So you move from one interface that handles both MSI/INTx, to _another_ interface that handles both. KVM_SIGNAL_MSI with address/data is clean and obvious.