Re: [Qemu-devel] [PATCH 2/3] VFIO driver for mediated PCI device

[Kirti Wankhede]

On 6/29/2016 8:24 AM, Alex Williamson wrote:
> On Wed, 29 Jun 2016 00:15:23 +0530
> Kirti Wankhede <address@hidden> wrote:
> 
>> On 6/25/2016 1:15 AM, Alex Williamson wrote:
>>> On Sat, 25 Jun 2016 00:04:27 +0530
>>> Kirti Wankhede <address@hidden> wrote:
>>>   
>>
>>>>>> +
>>>>>> +static int mdev_get_irq_count(struct vfio_mdev *vmdev, int irq_type)
>>>>>> +{
>>>>>> +        /* Don't support MSIX for now */
>>>>>> +        if (irq_type == VFIO_PCI_MSIX_IRQ_INDEX)
>>>>>> +                return -1;
>>>>>> +
>>>>>> +        return 1;    
>>>>>
>>>>> Too much hard coding here, the mediated driver should define this.
>>>>>     
>>>>
>>>> I'm testing INTX and MSI, I don't have a way to test MSIX for now. So we
>>>> thought we can add supported for MSIX later. Till then hard code it to 1.  
>>>
>>> To me it screams that there needs to be an interface to the mediated
>>> device here.  How do you even know that the mediated device intends to
>>> support MSI?  What if it wants to emulated a VF and not support INTx?
>>> This is basically just a big "TODO" flag that needs to be addressed
>>> before a non-RFC.
>>>   
>>
>> VFIO user space app reads emulated PCI config space of mediated device.
>> In PCI capability list when MSI capability (PCI_CAP_ID_MSI) is present,
>> it calls VFIO_DEVICE_SET_IRQS ioctl with irq_set->index set to
>> VFIO_PCI_MSI_IRQ_INDEX.
>> Similarly, MSIX is identified from emulated config space of mediated
>> device that checks if MSI capability is present and number of vectors
>> extracted from PCI_MSI_FLAGS_QSIZE flag.
>> vfio_mpci modules don't need to query it from vendor driver of mediated
>> device. Depending on which interrupt to support, mediated driver should
>> emulate PCI config space.
> 
> Are you suggesting that if the user can determine which interrupts are
> supported and the various counts for each by querying the PCI config
> space of the mediated device then this interface should do the same,
> much like vfio_pci_get_irq_count(), such that it can provide results
> consistent with config space?  That I'm ok with.  Having the user find
> one IRQ count as they read PCI config space and another via the vfio
> API, I'm not ok with.  Thanks,
> 

Yes, it will be more like vfio_pci_get_irq_count(). I will have
mdev_get_irq_count() updated with such change in next version of patch.

Thanks,
Kirti.