Re: [RFC PATCH v2 1/2] KVM: kvm-vfio: User API for VT-d Posted-Interrupts

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

On 11/25/2014 05:10 PM, Alex Williamson wrote:
> On Tue, 2014-11-25 at 16:01 +0100, Eric Auger wrote:
>> On 11/25/2014 01:23 PM, Feng Wu wrote:
>>> This patch adds and documents a new attribute
>>> KVM_DEV_VFIO_DEVICE_POSTING_IRQ in KVM_DEV_VFIO_DEVICE group.
>>> This new attribute is used for VT-d Posted-Interrupts.
>>>
>>> When guest OS changes the interrupt configuration for an
>>> assigned device, such as, MSI/MSIx data/address fields,
>>> QEMU will use this IRQ attribute to tell KVM to update the
>>> related IRTE according the VT-d Posted-Interrrupts Specification,
>>> such as, the guest vector should be updated in the related IRTE.
>>>
>>> Signed-off-by: Feng Wu <feng.wu@intel.com>
>>> ---
>>>  Documentation/virtual/kvm/devices/vfio.txt |    9 +++++++++
>>>  include/uapi/linux/kvm.h                   |   10 ++++++++++
>>>  2 files changed, 19 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
>>> index f7aff29..39dee86 100644
>>> --- a/Documentation/virtual/kvm/devices/vfio.txt
>>> +++ b/Documentation/virtual/kvm/devices/vfio.txt
>>> @@ -42,3 +42,12 @@ activated before VFIO_DEVICE_SET_IRQS has been called to trigger the IRQ
>>>  or associate an eventfd to it. Unforwarding can only be called while the
>>>  signaling has been disabled with VFIO_DEVICE_SET_IRQS. If this condition is
>>>  not satisfied, the command returns an -EBUSY.
>>> +
>>> +  KVM_DEV_VFIO_DEVICE_POSTING_IRQ: Use posted interrtups mechanism to post
>>> +                                   the IRQ to guests.
>>> +For this attribute, kvm_device_attr.addr points to a kvm_posted_intr struct.
>>> +
>>> +When guest OS changes the interrupt configuration for an assigned device,
>>> +such as, MSI/MSIx data/address fields, QEMU will use this IRQ attribute
>>> +to tell KVM to update the related IRTE according the VT-d Posted-Interrrupts
>>> +Specification, such as, the guest vector should be updated in the related IRTE.
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index a269a42..e5f86ad 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -949,6 +949,7 @@ struct kvm_device_attr {
>>>  #define  KVM_DEV_VFIO_DEVICE			2
>>>  #define   KVM_DEV_VFIO_DEVICE_FORWARD_IRQ			1
>>>  #define   KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ			2
>>> +#define   KVM_DEV_VFIO_DEVICE_POSTING_IRQ			3
>>>  
>>>  enum kvm_device_type {
>>>  	KVM_DEV_TYPE_FSL_MPIC_20	= 1,
>>> @@ -973,6 +974,15 @@ struct kvm_arch_forwarded_irq {
>>>  	__u32 gsi; /* gsi, ie. virtual IRQ number */
>>>  };
>>>  
Hi Feng, Alex,
I am currently reworking my code to use something closer to this struct.
Would you agree with following changes?
>>> +struct kvm_posted_intr {
kvm_posted_irq
>>> +	__u32	argsz;
>>> +	__u32	fd;		/* file descriptor of the VFIO device */
>>> +	__u32	index;		/* VFIO device IRQ index */
>>> +	__u32	start;
>>> +	__u32	count;
>>> +	int	virq[0];	/* gsi, ie. virtual IRQ number */
__u32 gsi[];

>>> +};
>> Hi Feng,
>>
>> This struct could be used by arm code too. If Alex agrees I could use
>> that one instead. We just need to find a common sensible name
> 
> Yep, the interface might as well support batch setup.  The vfio code
> uses -1 for teardown if we want to avoid FORWARD vs UNFORWARD we could
> let the data in the structure define which operation to do. 

In case we remove the unforward and use fd=1 to tear down, the virq=gsi
must uniquely identify the struct. For ARM I think this is true, we
cannot have several physical IRQ forwarded to the same GSI. I don't know
about posted irqs or other archs.

Best Regards

Eric
 Ideally the
> code in virt/kvm/vfio.c would be almost entirely shared and just make
> different arch_foo() callouts.  The PCI smarts in 2/2 here should
> probably be moved out to that same arch_ code.  Thanks,
> 
> Alex
>