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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 5 Oct 2015 13:48:39 +0300
From:	Vlad Zolotarov <vladz@...udius-systems.com>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	linux-kernel@...r.kernel.org, mst@...hat.com, hjk@...sjkoch.de,
	corbet@....net, bruce.richardson@...el.com,
	avi@...udius-systems.com, gleb@...udius-systems.com,
	stephen@...workplumber.org, alexander.duyck@...il.com
Subject: Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support



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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/