From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753430AbbJETuh (ORCPT ); Mon, 5 Oct 2015 15:50:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56737 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752446AbbJETug (ORCPT ); Mon, 5 Oct 2015 15:50:36 -0400 Date: Mon, 5 Oct 2015 22:50:29 +0300 From: "Michael S. Tsirkin" To: Vlad Zolotarov Cc: linux-kernel@vger.kernel.org, hjk@hansjkoch.de, corbet@lwn.net, gregkh@linuxfoundation.org, bruce.richardson@intel.com, avi@cloudius-systems.com, gleb@cloudius-systems.com, stephen@networkplumber.org, alexander.duyck@gmail.com Subject: Re: [PATCH v3 0/3] uio: add MSI/MSI-X support to uio_pci_generic driver Message-ID: <20151005221651-mutt-send-email-mst@redhat.com> References: <1443991398-23761-1-git-send-email-vladz@cloudius-systems.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1443991398-23761-1-git-send-email-vladz@cloudius-systems.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Oct 04, 2015 at 11:43:15PM +0300, Vlad Zolotarov wrote: > This series add support for MSI and MSI-X interrupts to uio_pci_generic driver. > > Currently uio_pci_generic supports only legacy INT#x interrupts source. However > there are situations when this is not enough, for instance SR-IOV VF devices that > simply don't have INT#x capability. For such devices uio_pci_generic will simply > fail (more specifically probe() will fail). > > When IOMMU is either not available (e.g. Amazon EC2) or not acceptable due to performance > overhead and thus VFIO is not an option > users that develop user-space drivers are left > without any option but to develop some proprietary UIO drivers (e.g. igb_uio driver in Intel's > DPDK) just to be able to use UIO infrastructure. > > This series provides a generic solution for this problem while preserving the original behaviour > for devices for which the original uio_pci_generic had worked before (i.e. INT#x will be used by default). What is missing here is that drivers using uio_pci_generic generally poke at config and BAR sysfs files of the device. We can not stop them without breaking existing users, but this means that we can't enable bus mastering and MSI/MSI-X blindly: userspace bugs will corrupt the MSI-X table and/or MSi/MSI-X capability, and cause device to overwrite random addresses, corrupting kernel memory. Your solution seems to be a warning in dmesg and tainting the kernel, but that's not enough. You need to add infrastructure to prevent this. VFIO has some code to do this, but it's not bound by existing UIO API so it simply fails the mmap. We want I think existing applications to work, so I suspect we need to make a hole there (probably map a zero page in case apps want to read it, and maybe even set it up for COW in case they tweak the PBA which sometimes happens to be in the same page). Your patches also seem to add in eventfd and mmap capabilities which seems to be orthogonal. They are there in VFIO which I'm guessing is the real reason you do it. So, what you are trying to do might be closer to extending VFIO which already has a bunch of checks like that. Yes, it also wants to program the IOMMU. So maybe do it with a separate device that can be root-only, so unpriveledged users can't abuse it. You should Cc, and talk to the VFIO maintainer. > New in v3: > - Add __iomem qualifier to temp buffer receiving ioremap value. > > New in v2: > - Added #include to uio_pci_generic.c > > Vlad Zolotarov (3): > uio: add ioctl support > uio_pci_generic: add MSI/MSI-X support > Documentation: update uio-howto > > Documentation/DocBook/uio-howto.tmpl | 29 ++- > drivers/uio/uio.c | 15 ++ > drivers/uio/uio_pci_generic.c | 410 +++++++++++++++++++++++++++++++++-- > include/linux/uio_driver.h | 3 + > include/linux/uio_pci_generic.h | 36 +++ > 5 files changed, 467 insertions(+), 26 deletions(-) > create mode 100644 include/linux/uio_pci_generic.h > > -- > 2.1.0