Re: [RFC PATCH v1 1/2] vfio: Add new interrupt group for VFIO

[Eric Auger <eric.auger@linaro.org>]

On 11/20/2014 04:53 PM, Alex Williamson wrote:
> On Thu, 2014-11-20 at 17:05 +0800, Feng Wu wrote:
>> Add new group KVM_DEV_VFIO_INTERRUPT and command
>> KVM_DEV_VFIO_DEVIE_POSTING_IRQ related to it.
>>
>> This is used for VT-d Posted-Interrupts setup.
> 
> Eric proposed an interface for ARM forwarded interrupts[1] using group
> KVM_DEV_VFIO_DEVICE with attributes KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ and
> KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ.  Why are we proposing yet another
> group and attributes here?  Why can't we re-use the ones Eric proposes?

Hi Alex, Feng

I share your point of view about the KVM_DEV_VFIO_DEVICE group. For the
attribute (renamed KVM_DEV_VFIO_DEVICE_FORWARD_IRQ in RFC v2) the issue
is I specify the kvm_device_attr.addr points to a kvm_arch_forwarded_irq
struct. Feng needs another struct - kvm_posted_intr -. An alternative is
to merge both structs if it makes sense.

Best Regards

Eric
> 
> [1] https://lkml.org/lkml/2014/8/25/258
> 
>> Signed-off-by: Feng Wu <feng.wu@intel.com>
>> ---
>>  Documentation/virtual/kvm/devices/vfio.txt |    8 ++++++++
>>  include/uapi/linux/kvm.h                   |   14 ++++++++++++++
>>  2 files changed, 22 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
>> index ef51740..bd99176 100644
>> --- a/Documentation/virtual/kvm/devices/vfio.txt
>> +++ b/Documentation/virtual/kvm/devices/vfio.txt
>> @@ -13,6 +13,7 @@ VFIO-group is held by KVM.
>>  
>>  Groups:
>>    KVM_DEV_VFIO_GROUP
>> +  KVM_DEV_VFIO_INTERRUPT
>>  
>>  KVM_DEV_VFIO_GROUP attributes:
>>    KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
>> @@ -20,3 +21,10 @@ KVM_DEV_VFIO_GROUP attributes:
>>  
>>  For each, kvm_device_attr.addr points to an int32_t file descriptor
>>  for the VFIO group.
>> +
>> +KVM_DEV_VFIO_INTERRUPT attributes:
>> +  KVM_DEV_VFIO_INTERRUPT_POSTING_IRQ: Set up the interrupt configuration for
>> +VT-d Posted-Interrrupts
>> +
>> +For each, kvm_device_attr.addr points to struct kvm_posted_intr, which
>> +include the needed information for VT-d Posted-Interrupts setup.
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 6076882..5544fcc 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -943,9 +943,23 @@ struct kvm_device_attr {
>>  	__u64	addr;		/* userspace address of attr data */
>>  };
>>  
>> +struct virq_info {
>> +	__u32	index;		/* index of the msi/msix entry */
>> +	int	virq;		/* virq of the interrupt */
>> +};
>> +
>> +struct kvm_posted_intr {
>> +	__u32	fd;		/* file descriptor of the VFIO device */
>> +	__u32	count;
>> +	bool	msix;
> 
> Note that MSI-X (as opposed to MSI) is a PCI concept.  Being a VFIO
> interface this should operate at VFIO IRQ index and sub-index.
> 
>> +	struct virq_info virq_info[0];
>> +};
>> +
>>  #define  KVM_DEV_VFIO_GROUP			1
>>  #define   KVM_DEV_VFIO_GROUP_ADD			1
>>  #define   KVM_DEV_VFIO_GROUP_DEL			2
>> +#define  KVM_DEV_VFIO_INTERRUPT			2
>> +#define   KVM_DEV_VFIO_INTERRUPT_POSTING_IRQ		1
>>  
>>  enum kvm_device_type {
>>  	KVM_DEV_TYPE_FSL_MPIC_20	= 1,
> 
> 
>