From: "Tian, Kevin" <kevin.tian@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Yongji Xie <xyjxie@linux.vnet.ibm.com>,
David Laight <David.Laight@ACULAB.COM>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
"iommu@lists.linux-foundation.org"
<iommu@lists.linux-foundation.org>,
"bhelgaas@google.com" <bhelgaas@google.com>,
"aik@ozlabs.ru" <aik@ozlabs.ru>,
"benh@kernel.crashing.org" <benh@kernel.crashing.org>,
"paulus@samba.org" <paulus@samba.org>,
"mpe@ellerman.id.au" <mpe@ellerman.id.au>,
"joro@8bytes.org" <joro@8bytes.org>,
"warrier@linux.vnet.ibm.com" <warrier@linux.vnet.ibm.com>,
"zhong@linux.vnet.ibm.com" <zhong@linux.vnet.ibm.com>,
"nikunj@linux.vnet.ibm.com" <nikunj@linux.vnet.ibm.com>,
"eric.auger@linaro.org" <eric.auger@linaro.org>,
"will.deacon@arm.com" <will.deacon@arm.com>,
"gwshan@linux.vnet.ibm.com" <gwshan@linux.vnet.ibm.com>,
"alistair@popple.id.au" <alistair@popple.id.au>,
"ruscur@russell.cc" <ruscur@russell.cc>
Subject: RE: [PATCH 5/5] vfio-pci: Allow to mmap MSI-X table if interrupt remapping is supported
Date: Thu, 12 May 2016 01:19:44 +0000 [thread overview]
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D15F850046@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20160511095331.18436241@t450s.home>
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Wednesday, May 11, 2016 11:54 PM
>
> On Wed, 11 May 2016 06:29:06 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
>
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Thursday, May 05, 2016 11:05 PM
> > >
> > > On Thu, 5 May 2016 12:15:46 +0000
> > > "Tian, Kevin" <kevin.tian@intel.com> wrote:
> > >
> > > > > From: Yongji Xie [mailto:xyjxie@linux.vnet.ibm.com]
> > > > > Sent: Thursday, May 05, 2016 7:43 PM
> > > > >
> > > > > Hi David and Kevin,
> > > > >
> > > > > On 2016/5/5 17:54, David Laight wrote:
> > > > >
> > > > > > From: Tian, Kevin
> > > > > >> Sent: 05 May 2016 10:37
> > > > > > ...
> > > > > >>> Acutually, we are not aimed at accessing MSI-X table from
> > > > > >>> guest. So I think it's safe to passthrough MSI-X table if we
> > > > > >>> can make sure guest kernel would not touch MSI-X table in
> > > > > >>> normal code path such as para-virtualized guest kernel on PPC64.
> > > > > >>>
> > > > > >> Then how do you prevent malicious guest kernel accessing it?
> > > > > > Or a malicious guest driver for an ethernet card setting up
> > > > > > the receive buffer ring to contain a single word entry that
> > > > > > contains the address associated with an MSI-X interrupt and
> > > > > > then using a loopback mode to cause a specific packet be
> > > > > > received that writes the required word through that address.
> > > > > >
> > > > > > Remember the PCIe cycle for an interrupt is a normal memory write
> > > > > > cycle.
> > > > > >
> > > > > > David
> > > > > >
> > > > >
> > > > > If we have enough permission to load a malicious driver or
> > > > > kernel, we can easily break the guest without exposed
> > > > > MSI-X table.
> > > > >
> > > > > I think it should be safe to expose MSI-X table if we can
> > > > > make sure that malicious guest driver/kernel can't use
> > > > > the MSI-X table to break other guest or host. The
> > > > > capability of IRQ remapping could provide this
> > > > > kind of protection.
> > > > >
> > > >
> > > > With IRQ remapping it doesn't mean you can pass through MSI-X
> > > > structure to guest. I know actual IRQ remapping might be platform
> > > > specific, but at least for Intel VT-d specification, MSI-X entry must
> > > > be configured with a remappable format by host kernel which
> > > > contains an index into IRQ remapping table. The index will find a
> > > > IRQ remapping entry which controls interrupt routing for a specific
> > > > device. If you allow a malicious program random index into MSI-X
> > > > entry of assigned device, the hole is obvious...
> > > >
> > > > Above might make sense only for a IRQ remapping implementation
> > > > which doesn't rely on extended MSI-X format (e.g. simply based on
> > > > BDF). If that's the case for PPC, then you should build MSI-X
> > > > passthrough based on this fact instead of general IRQ remapping
> > > > enabled or not.
> > >
> > > I don't think anyone is expecting that we can expose the MSI-X vector
> > > table to the guest and the guest can make direct use of it. The end
> > > goal here is that the guest on a power system is already
> > > paravirtualized to not program the device MSI-X by directly writing to
> > > the MSI-X vector table. They have hypercalls for this since they
> > > always run virtualized. Therefore a) they never intend to touch the
> > > MSI-X vector table and b) they have sufficient isolation that a guest
> > > can only hurt itself by doing so.
> > >
> > > On x86 we don't have a), our method of programming the MSI-X vector
> > > table is to directly write to it. Therefore we will always require QEMU
> > > to place a MemoryRegion over the vector table to intercept those
> > > accesses. However with interrupt remapping, we do have b) on x86, which
> > > means that we don't need to be so strict in disallowing user accesses
> > > to the MSI-X vector table. It's not useful for configuring MSI-X on
> > > the device, but the user should only be able to hurt themselves by
> > > writing it directly. x86 doesn't really get anything out of this
> > > change, but it helps this special case on power pretty significantly
> > > aiui. Thanks,
> > >
> >
> > Allowing guest direct write to MSI-x table has system-wide impact.
> > As I explained earlier, hypervisor needs to control "interrupt_index"
> > programmed in MSI-X entry, which is used to associate a specific
> > IRQ remapping entry. Now if you expose whole MSI-x table to guest,
> > it can program random index into MSI-X entry to associate with
> > any IRQ remapping entry and then there won't be any isolation per se.
> >
> > You can check "5.5.2 MSI and MSI-X Register Programming" in VT-d
> > spec.
>
> I think you're extrapolating beyond the vfio interface. The change
> here is to remove the vfio protection of the MSI-X vector table when
> the system provides interrupt isolation. The argument is that this is
> safe to do because the hardware protects the host from erroneous and
> malicious user programming, but it is not meant to provide a means to
> program MSI-X directly through the vector table. This is effectively
Sorry I didn't get this point. Once we allow userspace to mmap MSI-X
table, isn't it equivalent to allowing userspace directly program vector
table? Or is there other mechanism to prevent direct programming?
> the same as general DMA programming, if the vfio programming model is
> not followed the device generates iommu faults. I do have a concern
> that userspace driver writers are going to more often presume they can
> use the vector table directly because of this change, but I don't know
> that that is sufficient reason to prevent such a change. They'll
> quickly discover the device generates faults on interrupt rather than
> working as expected.
If userspace can actually program vector table directly, there is not
always fault triggered. As long as MSI-X table is fully under control
of userspace, any interrupt index can be used here which may link
to a IRQ remapping entry allocated for other devices.
>
> The question of how this affects the hypervisor is completely
> separate. Vfio in the kernel is a userspace driver interface, not a
> hypervisor. QEMU is the hypervisor. We have no plans to provide the VM
> with direct access to the MSI-X vector table for x86 guests on QEMU.
> There will still be a MemoryRegion emulating access to the vector table
> in order to translate writes into vfio interrupt ioctls. POWER would
> drop the MemoryRegion so that the full page is mapped to the guest,
> with the expectation that the guest never makes use of it since MSI-X
> is always configured via hypercalls on POWER systems. Likewise I
> expect ARM will still make use of the MemoryRegion emulating the vector
> table, which leaves them exposed to the performance issue POWER is
> trying to solve here since ARM also has 64k page support and has no
> paravirtualized MSI-X programming interface afaik. x86 is not
> impervious to this issue either, but a 4k page size falls within the
> PCI spec recommendations for MSI-X structure alignment, so it's much
> more rare to have issues. We have certainly seen hardware vendors that
> ignore the PCI spec alignment recommendations, but so far only for
> placing device registers within the same page as the PBA, which is an
> easier problem to deal with since the PBA is relatively unused by
> drivers. This may be an area where we need to develop a paravirt
> interface for MSI-X programming which disable the MemoryRegion
> emulating the vector table when used. Thanks,
>
I get this point. I incorrectly line allowing userspace mmap MSI-X
table to allowing guest direct access to MSI-X.
Thanks
Kevin
next prev parent reply other threads:[~2016-05-12 1:20 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-27 12:43 [PATCH 0/5] vfio-pci: Add support for mmapping MSI-X table Yongji Xie
2016-04-27 12:43 ` [PATCH 1/5] PCI: Add a new PCI_BUS_FLAGS_MSI_REMAP flag Yongji Xie
2016-05-24 20:55 ` Bjorn Helgaas
2016-05-25 5:46 ` Yongji Xie
2016-04-27 12:43 ` [PATCH 2/5] iommu: Set PCI_BUS_FLAGS_MSI_REMAP if IOMMU have capability of IRQ remapping Yongji Xie
2016-05-24 21:11 ` Bjorn Helgaas
2016-05-25 5:54 ` Yongji Xie
[not found] ` <201605250554.u4P5sRqv014439@mx0a-001b2d01.pphosted.com>
2016-05-26 3:48 ` Bjorn Helgaas
2016-04-27 12:43 ` [PATCH 3/5] PCI: Set PCI_BUS_FLAGS_MSI_REMAP if MSI controller supports " Yongji Xie
2016-05-24 21:04 ` Bjorn Helgaas
2016-05-25 5:48 ` Yongji Xie
2016-04-27 12:43 ` [PATCH 4/5] pci-ioda: Set PCI_BUS_FLAGS_MSI_REMAP for IODA host bridge Yongji Xie
2016-05-06 6:34 ` Alexey Kardashevskiy
2016-04-27 12:43 ` [PATCH 5/5] vfio-pci: Allow to mmap MSI-X table if interrupt remapping is supported Yongji Xie
2016-05-03 5:34 ` Tian, Kevin
2016-05-03 6:08 ` Yongji Xie
2016-05-03 6:22 ` Tian, Kevin
2016-05-03 7:34 ` Yongji Xie
2016-05-05 9:36 ` Tian, Kevin
2016-05-05 9:54 ` David Laight
2016-05-05 11:42 ` Yongji Xie
2016-05-05 12:15 ` Tian, Kevin
2016-05-05 13:28 ` Yongji Xie
2016-05-05 15:05 ` Alex Williamson
2016-05-06 6:35 ` Alexey Kardashevskiy
2016-05-06 16:54 ` Alex Williamson
2016-05-11 6:29 ` Tian, Kevin
2016-05-11 15:53 ` Alex Williamson
2016-05-12 1:19 ` Tian, Kevin [this message]
2016-05-12 2:20 ` Alex Williamson
2016-05-12 4:53 ` Tian, Kevin
2016-05-12 17:47 ` Alex Williamson
2016-05-13 2:33 ` Tian, Kevin
2016-05-13 5:32 ` Alex Williamson
2016-05-13 6:50 ` Tian, Kevin
2016-05-13 16:42 ` Alex Williamson
2016-05-13 9:16 ` David Laight
2016-05-13 2:36 ` Tian, Kevin
2016-05-05 11:44 ` Yongji Xie
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=AADFC41AFE54684AB9EE6CBC0274A5D15F850046@SHSMSX101.ccr.corp.intel.com \
--to=kevin.tian@intel.com \
--cc=David.Laight@ACULAB.COM \
--cc=aik@ozlabs.ru \
--cc=alex.williamson@redhat.com \
--cc=alistair@popple.id.au \
--cc=benh@kernel.crashing.org \
--cc=bhelgaas@google.com \
--cc=eric.auger@linaro.org \
--cc=gwshan@linux.vnet.ibm.com \
--cc=iommu@lists.linux-foundation.org \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=nikunj@linux.vnet.ibm.com \
--cc=paulus@samba.org \
--cc=ruscur@russell.cc \
--cc=warrier@linux.vnet.ibm.com \
--cc=will.deacon@arm.com \
--cc=xyjxie@linux.vnet.ibm.com \
--cc=zhong@linux.vnet.ibm.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).