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

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-03-29 21:41, Michael S. Tsirkin wrote:
> On Thu, Mar 29, 2012 at 09:14:12PM +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 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 <jan.kiszka@siemens.com>
>> ---
>>
>> 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(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 81ff39f..ed27d1b 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_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
>> 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..225b452 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_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
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 6cf158c..35c69d6 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -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) {}
>> 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/irq_comm.c b/virt/kvm/irq_comm.c
>> index 9f614b4..a6a0365 100644
>> --- a/virt/kvm/irq_comm.c
>> +++ b/virt/kvm/irq_comm.c
>> @@ -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)
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index a612bc8..5addfa8 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -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: {
>> +		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

> 
>> +		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:
>> -- 
>> 1.7.3.4

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