From: Alex Williamson <alex.williamson@redhat.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Haggai Eran <haggaie@mellanox.com>,
Ilya Lesokhin <ilyal@mellanox.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"bhelgaas@google.com" <bhelgaas@google.com>,
"Noa Osherovich" <noaos@mellanox.com>,
Or Gerlitz <ogerlitz@mellanox.com>,
"Liran Liss" <liranl@mellanox.com>
Subject: Re: [PATCH v2 0/2] VFIO SRIOV support
Date: Tue, 19 Jul 2016 13:43:36 -0600 [thread overview]
Message-ID: <20160719134336.79d24af9@t450s.home> (raw)
In-Reply-To: <20160719091017.4c23244f@t450s.home>
On Tue, 19 Jul 2016 09:10:17 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:
> On Tue, 19 Jul 2016 07:06:34 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
>
> > > From: Alex Williamson
> > > Sent: Tuesday, July 19, 2016 5:34 AM
> > >
> > > On Sun, 17 Jul 2016 13:05:21 +0300
> > > Haggai Eran <haggaie@mellanox.com> wrote:
> > >
> > > > On 7/14/2016 8:03 PM, Alex Williamson wrote:
> > > > >> 2. Add an owner_pid to struct vfio_group and make sure in
> > > vfio_group_get_device_fd that
> > > > >> > the PFs vfio_group is owned by the same process as the one that is trying to get
> > > a fd for a VF.
> > > > > This only solves a very specific use case, it doesn't address any of
> > > > > the issues where the VF struct device in the host kernel might get
> > > > > bound to another driver.
> > > > The current patch uses driver_override to make the kernel use VFIO for
> > > > all the new VFs. It still allows the host kernel to bind them to another
> > > > driver, but that would require an explicit action on the part of the
> > > > administrator. Don't you think that is enough?
> > >
> > > Binding the VFs to vfio-pci with driver_override just prevents any sort
> > > of initial use by native host drivers, it doesn't in any way tie them to
> > > the user that created them or prevent any normal operations on the
> > > device. The entire concept of a user-created device is new and
> > > entirely separate from a user-owned device as typically used with
> > > vfio. We currently have an assumption with VF assignment that the PF
> > > is trusted in the host, that's broken here and I have a hard time
> > > blaming it on the admin or management tool for allowing such a thing
> > > when it previously hasn't been a possibility. If nothing else, it
> > > seems like we're opening the system for phishing attempts where a user
> > > of a PF creates VFs hoping they might be assigned to a victim VM, or
> > > worse the host.
> > >
> >
> > What about fully virtualizing the SR-IOV capability? The VM is not allowed
> > to touch physical SR-IOV capability directly so there would not be a problem
> > of user-created devices. Physical SR-IOV is always enabled by admin at
> > the host side. Admin can combine any number of VFs (even cross multiple
> > compatible devices) in the virtual SR-IOV capability on any passthrough
> > device...
> >
> > The limitation is that the VM can initially access only PF resource which is
> > usually less than what the entire device provides, so not that efficient
> > when the VM doesn't want to enable SR-IOV at all.
>
> Are you suggesting a scenario where we have one PF with SR-IOV disabled
> assigned to the user and another PF owned by the host with SR-IOV
> enable, we virtualize SR-IOV to the user and use the VFs from the other
> PF to act as a "pool" of VFs to be exposed to the user depending on
> SR-IOV manipulation? Something like that could work with existing
> vfio, just requiring the QEMU bits to virtualize SR-IOV and mange the
> VFs, but I expect it's not a useful model for Mellanox. I believe it
> was Ilya that stated the purpose in exposing SR-IOV was for
> development, so I'm assuming they actually want to do development of
> the PF SR-IOV enabling in a VM, not just give the illusion of SR-IOV to
> the VM. Thanks,
Thinking about this further, it seems that trying to create this IOV
enablement interface through a channel which is explicitly designed to
interact with an untrusted and potentially malicious user is the wrong
approach. We already have an interface for a trusted entity to enable
VFs, it's through pci-sysfs. Therefore if we were to use something like
libvirt to orchestrate the lifecycle of the VFs, I think we remove a
lot of the problems. In this case QEMU would virtualize the SR-IOV
capability (maybe this is along the lines of what Kevin was thinking),
but that virtualization would take a path out through the QEMU QMP
interface to execute the SR-IOV change on the device rather than going
through the vfio kernel interface. A management tool like libvirt
would then need to translate that into sysfs operations to create the
VFs and do whatever we're going to do with them (device_add them back
to the VM, make them available to a peer VM, make them available to
the host *gasp*). VFIO in the kernel would need to add SR-IOV
support, but the only automatic SR-IOV path would be to disable IOV
when the PF is released, enabling would only occur through sysfs. We
would probably need a new pci-sysfs interface to manage the driver for
newly created VFs though to avoid default host drivers
(sriov_driver_override?). In this model QEMU is essentially just
making requests to other userspace entities to perform actions and how
those actions are performed can be left to userspace policy, not kernel
policy. I think this would still satisfy the development use case, the
enabling path just takes a different route where privileged userspace
is more intimately involved in the process. Thoughts? Thanks,
Alex
next prev parent reply other threads:[~2016-07-19 19:43 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-19 12:16 [PATCH v2 0/2] VFIO SRIOV support Ilya Lesokhin
2016-06-19 12:16 ` [PATCH v2 1/2] PCI: Extend PCI IOV API Ilya Lesokhin
2016-06-19 14:10 ` kbuild test robot
2016-06-19 12:16 ` [PATCH v2 2/2] VFIO: Add support for SR-IOV extended capablity Ilya Lesokhin
2016-06-19 23:07 ` kbuild test robot
2016-06-20 17:37 ` [PATCH v2 0/2] VFIO SRIOV support Alex Williamson
2016-06-21 7:19 ` Ilya Lesokhin
2016-06-21 15:45 ` Alex Williamson
2016-07-14 14:53 ` Ilya Lesokhin
2016-07-14 17:03 ` Alex Williamson
2016-07-17 10:05 ` Haggai Eran
2016-07-18 21:34 ` Alex Williamson
2016-07-19 7:06 ` Tian, Kevin
2016-07-19 15:10 ` Alex Williamson
2016-07-19 19:43 ` Alex Williamson [this message]
2016-07-21 5:51 ` Tian, Kevin
2016-07-25 7:53 ` Haggai Eran
2016-07-25 15:07 ` Alex Williamson
2016-07-25 15:34 ` Ilya Lesokhin
2016-07-25 15:58 ` Alex Williamson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160719134336.79d24af9@t450s.home \
--to=alex.williamson@redhat.com \
--cc=bhelgaas@google.com \
--cc=haggaie@mellanox.com \
--cc=ilyal@mellanox.com \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=liranl@mellanox.com \
--cc=noaos@mellanox.com \
--cc=ogerlitz@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).