Re: [PATCH v2] KVM: Introduce direct MSI message injection for in-kernel irqchips

2012-03-29 Thread Michael S. Tsirkin 
On Wed, Mar 28, 2012 at 07:47:31PM +0200, Jan Kiszka wrote:
 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 jan.kis...@siemens.com

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.


 ---
 
 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(-)
 
 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 81ff39f..dd6377e 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -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
 diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
 index 1a7fe86..a28f338 100644
 --- a/arch/x86/kvm/Kconfig
 +++ b/arch/x86/kvm/Kconfig
 @@ -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
 diff --git a/include/linux/kvm.h b/include/linux/kvm.h
 index 7a9dd4b..2dc24ac 100644
 --- a/include/linux/kvm.h
 +++ b/include/linux/kvm.h
 @@ -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
 diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
 index f63ccb0..28694f4 100644
 --- a/virt/kvm/Kconfig
 +++ b/virt/kvm/Kconfig
 @@ -18,3 +18,6 @@ config KVM_MMIO
  
  config KVM_ASYNC_PF
 bool
 +
 +config HAVE_KVM_MSI
 +   bool
 diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
 index a612bc8..ea9887e 100644
 --- a/virt/kvm/kvm_main.c
 +++ b/virt/kvm/kvm_main.c
 @@ -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)

Re: [PATCH v2] KVM: Introduce direct MSI message injection for in-kernel irqchips

2012-03-29 Thread Jan Kiszka 
On 2012-03-29 17:39, Michael S. Tsirkin wrote:
 On Wed, Mar 28, 2012 at 07:47:31PM +0200, Jan Kiszka wrote:
 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 jan.kis...@siemens.com
 
 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.

KVM_SET_MSI dates back to the idea to revoke an unfinished injection.
But I can also call it SIGNAL_MSI. Update will follow.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] KVM: Introduce direct MSI message injection for in-kernel irqchips

2012-03-28 Thread 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 jan.kis...@siemens.com
---

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(-)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 81ff39f..dd6377e 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -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
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 1a7fe86..a28f338 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -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
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 7a9dd4b..2dc24ac 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -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
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index f63ccb0..28694f4 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -18,3 +18,6 @@ config KVM_MMIO
 
 config KVM_ASYNC_PF
bool
+
+config HAVE_KVM_MSI
+   bool
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index a612bc8..ea9887e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -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:
-- 
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at

Re: [PATCH v2] KVM: Introduce direct MSI message injection for in-kernel irqchips

2012-03-28 Thread Jan Kiszka 
On 2012-03-28 19:47, Jan Kiszka wrote:
 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

By providing a direct injection path, [...]. Will fix if another round
is required, please add manually otherwise.

Jan

 resources and simplify the necessary steps for user land.
 
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 ---
 
 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(-)
 
 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 81ff39f..dd6377e 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -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
 diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
 index 1a7fe86..a28f338 100644
 --- a/arch/x86/kvm/Kconfig
 +++ b/arch/x86/kvm/Kconfig
 @@ -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
 diff --git a/include/linux/kvm.h b/include/linux/kvm.h
 index 7a9dd4b..2dc24ac 100644
 --- a/include/linux/kvm.h
 +++ b/include/linux/kvm.h
 @@ -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
 diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
 index f63ccb0..28694f4 100644
 --- a/virt/kvm/Kconfig
 +++ b/virt/kvm/Kconfig
 @@ -18,3 +18,6 @@ config KVM_MMIO
  
  config KVM_ASYNC_PF
 bool
 +
 +config HAVE_KVM_MSI
 +   bool
 diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
 index a612bc8..ea9887e 100644
 --- a/virt/kvm/kvm_main.c
 +++ b/virt/kvm/kvm_main.c
 @@ -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

Re: [PATCH v2] KVM: Introduce direct MSI message injection for in-kernel irqchips

2012-03-28 Thread Eric Northup 
On Wed, Mar 28, 2012 at 10:47 AM, Jan Kiszka jan.kis...@siemens.com wrote:
[...]
 +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

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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] KVM: Introduce direct MSI message injection for in-kernel irqchips

2012-03-28 Thread Jan Kiszka 
On 2012-03-28 21:58, Eric Northup wrote:
 On Wed, Mar 28, 2012 at 10:47 AM, Jan Kiszka jan.kis...@siemens.com wrote:
 [...]
 +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
 
 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.

True. And I think we want this information, e.g. to implement IRQ
decoalescing for periodic HPET timers that use MSIs.

Will adjust the documentations.

Thanks for reviewing,
Jan




signature.asc
Description: OpenPGP digital signature