Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support

[Vlad Zolotarov <vladz@cloudius-systems.com>]

On 10/05/15 10:56, Greg KH wrote:
> On Mon, Oct 05, 2015 at 10:41:39AM +0300, Vlad Zolotarov wrote:
>>>> +struct msix_info {
>>>> +	int num_irqs;
>>>> +	struct msix_entry *table;
>>>> +	struct uio_msix_irq_ctx {
>>>> +		struct eventfd_ctx *trigger;	/* MSI-x vector to eventfd */
>>> Why are you using eventfd for msi vectors?  What's the reason for
>>> needing this?
>> A small correction - for MSI-X vectors. There may be only one MSI vector per
>> PCI function and if it's used it would use the same interface as a legacy
>> INT#x interrupt uses at the moment.
>> So, for MSI-X case the reason is that there may be (in most cases there will
>> be) more than one interrupt vector. Thus, as I've explained in a PATCH1
>> thread we need a way to indicated each of them separately. eventfd seems
>> like a good way of doing so. If u have better ideas, pls., share.
> You need to document what you are doing here, I don't see any
> explaination for using eventfd at all.
>
> And no, I don't know of any other solution as I don't know what you are
> trying to do here (hint, the changelog didn't document it...)
>
>>> You haven't documented how this api works at all, you are going to have
>>> to a lot more work to justify this, as this greatly increases the
>>> complexity of the user/kernel api in unknown ways.
>> I actually do documented it a bit. Pls., check PATCH3 out.
> That provided no information at all about how to use the api.
>
> If it did, you would see that your api is broken for 32/64bit kernels
> and will fall over into nasty pieces the first time you try to use it
> there, which means it hasn't been tested at all :(

It has been tested of course ;)
I tested it only in 64 bit environment however where both kernel and 
user space applications were compiled on the same machine with the same 
compiler and it could be that "int" had the same number of bytes both in 
kernel and in user space application. Therefore it worked perfectly - I 
patched DPDK to use the new uio_pci_generic MSI-X API to test this and I 
have verified that all 3 interrupt modes work: MSI-X with SR-IOV VF 
device in Amazon EC2 guest and INT#x and MSI with a PF device on bare 
metal server.

However I agree using uint32_t for "vec" and "fd" would be much more 
correct.

>
> thanks,
>
> greg k-h