Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

On 29/08/17 12:58, Alexey Kardashevskiy wrote:
> On 21/08/17 12:47, Alexey Kardashevskiy wrote:
>> Folks,
>>
>> Ok, people did talk, exchanged ideas, lovely :) What happens now? Do I
>> repost this or go back to PCI bus flags or something else? Thanks.
> 
> 
> Anyone, any help? How do we proceed with this? Thanks.

Ping, anyone?

Or everybody is just waiting for the kvm forum to discuss it in person? Thanks.



> 
> 
> 
>>
>>
>>
>> On 14/08/17 19:45, Alexey Kardashevskiy wrote:
>>> Folks,
>>>
>>> Is there anything to change besides those compiler errors and David's
>>> comment in 5/5? Or the while patchset is too bad? Thanks.
>>>
>>>
>>>
>>> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
 This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
 mmapping MSI-X table"
 http://www.spinics.net/lists/kvm/msg152232.html

 This time it is using "caps" in IOMMU groups. The main question is if PCI
 bus flags or IOMMU domains are still better (and which one).
>>>



 Here is some background:

 Current vfio-pci implementation disallows to mmap the page
 containing MSI-X table in case that users can write directly
 to MSI-X table and generate an incorrect MSIs.

 However, this will cause some performance issue when there
 are some critical device registers in the same page as the
 MSI-X table. We have to handle the mmio access to these
 registers in QEMU emulation rather than in guest.

 To solve this issue, this series allows to expose MSI-X table
 to userspace when hardware enables the capability of interrupt
 remapping which can ensure that a given PCI device can only
 shoot the MSIs assigned for it. And we introduce a new bus_flags
 PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
 for different archs.


 This is based on sha1
 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
 git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"

 Please comment. Thanks.

 Changelog:

 v5:
 * redid the whole thing via so-called IOMMU group capabilities

 v4:
 * rebased on recent upstream
 * got all 6 patches from v2 (v3 was missing some)




 Alexey Kardashevskiy (5):
   iommu: Add capabilities to a group
   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
 remapping
   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
 enabled
   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
   vfio-pci: Allow to expose MSI-X table to userspace when safe

  include/linux/iommu.h| 20 
  include/linux/vfio.h |  1 +
  arch/powerpc/kernel/iommu.c  |  1 +
  drivers/iommu/amd_iommu.c|  3 +++
  drivers/iommu/intel-iommu.c  |  3 +++
  drivers/iommu/iommu.c| 35 +++
  drivers/vfio/pci/vfio_pci.c  | 20 +---
  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
  drivers/vfio/vfio.c  | 15 +++
  9 files changed, 99 insertions(+), 4 deletions(-)

>>>
>>>
>>
>>
> 
> 


-- 
Alexey

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

On 29/08/17 12:58, Alexey Kardashevskiy wrote:
> On 21/08/17 12:47, Alexey Kardashevskiy wrote:
>> Folks,
>>
>> Ok, people did talk, exchanged ideas, lovely :) What happens now? Do I
>> repost this or go back to PCI bus flags or something else? Thanks.
> 
> 
> Anyone, any help? How do we proceed with this? Thanks.

Ping, anyone?

Or everybody is just waiting for the kvm forum to discuss it in person? Thanks.



> 
> 
> 
>>
>>
>>
>> On 14/08/17 19:45, Alexey Kardashevskiy wrote:
>>> Folks,
>>>
>>> Is there anything to change besides those compiler errors and David's
>>> comment in 5/5? Or the while patchset is too bad? Thanks.
>>>
>>>
>>>
>>> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
 This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
 mmapping MSI-X table"
 http://www.spinics.net/lists/kvm/msg152232.html

 This time it is using "caps" in IOMMU groups. The main question is if PCI
 bus flags or IOMMU domains are still better (and which one).
>>>



 Here is some background:

 Current vfio-pci implementation disallows to mmap the page
 containing MSI-X table in case that users can write directly
 to MSI-X table and generate an incorrect MSIs.

 However, this will cause some performance issue when there
 are some critical device registers in the same page as the
 MSI-X table. We have to handle the mmio access to these
 registers in QEMU emulation rather than in guest.

 To solve this issue, this series allows to expose MSI-X table
 to userspace when hardware enables the capability of interrupt
 remapping which can ensure that a given PCI device can only
 shoot the MSIs assigned for it. And we introduce a new bus_flags
 PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
 for different archs.


 This is based on sha1
 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
 git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"

 Please comment. Thanks.

 Changelog:

 v5:
 * redid the whole thing via so-called IOMMU group capabilities

 v4:
 * rebased on recent upstream
 * got all 6 patches from v2 (v3 was missing some)




 Alexey Kardashevskiy (5):
   iommu: Add capabilities to a group
   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
 remapping
   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
 enabled
   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
   vfio-pci: Allow to expose MSI-X table to userspace when safe

  include/linux/iommu.h| 20 
  include/linux/vfio.h |  1 +
  arch/powerpc/kernel/iommu.c  |  1 +
  drivers/iommu/amd_iommu.c|  3 +++
  drivers/iommu/intel-iommu.c  |  3 +++
  drivers/iommu/iommu.c| 35 +++
  drivers/vfio/pci/vfio_pci.c  | 20 +---
  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
  drivers/vfio/vfio.c  | 15 +++
  9 files changed, 99 insertions(+), 4 deletions(-)

>>>
>>>
>>
>>
> 
> 


-- 
Alexey

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

On 21/08/17 12:47, Alexey Kardashevskiy wrote:
> Folks,
> 
> Ok, people did talk, exchanged ideas, lovely :) What happens now? Do I
> repost this or go back to PCI bus flags or something else? Thanks.


Anyone, any help? How do we proceed with this? Thanks.



> 
> 
> 
> On 14/08/17 19:45, Alexey Kardashevskiy wrote:
>> Folks,
>>
>> Is there anything to change besides those compiler errors and David's
>> comment in 5/5? Or the while patchset is too bad? Thanks.
>>
>>
>>
>> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
>>> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
>>> mmapping MSI-X table"
>>> http://www.spinics.net/lists/kvm/msg152232.html
>>>
>>> This time it is using "caps" in IOMMU groups. The main question is if PCI
>>> bus flags or IOMMU domains are still better (and which one).
>>
>>>
>>>
>>>
>>> Here is some background:
>>>
>>> Current vfio-pci implementation disallows to mmap the page
>>> containing MSI-X table in case that users can write directly
>>> to MSI-X table and generate an incorrect MSIs.
>>>
>>> However, this will cause some performance issue when there
>>> are some critical device registers in the same page as the
>>> MSI-X table. We have to handle the mmio access to these
>>> registers in QEMU emulation rather than in guest.
>>>
>>> To solve this issue, this series allows to expose MSI-X table
>>> to userspace when hardware enables the capability of interrupt
>>> remapping which can ensure that a given PCI device can only
>>> shoot the MSIs assigned for it. And we introduce a new bus_flags
>>> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
>>> for different archs.
>>>
>>>
>>> This is based on sha1
>>> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
>>> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
>>>
>>> Please comment. Thanks.
>>>
>>> Changelog:
>>>
>>> v5:
>>> * redid the whole thing via so-called IOMMU group capabilities
>>>
>>> v4:
>>> * rebased on recent upstream
>>> * got all 6 patches from v2 (v3 was missing some)
>>>
>>>
>>>
>>>
>>> Alexey Kardashevskiy (5):
>>>   iommu: Add capabilities to a group
>>>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
>>> remapping
>>>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
>>> enabled
>>>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>>>   vfio-pci: Allow to expose MSI-X table to userspace when safe
>>>
>>>  include/linux/iommu.h| 20 
>>>  include/linux/vfio.h |  1 +
>>>  arch/powerpc/kernel/iommu.c  |  1 +
>>>  drivers/iommu/amd_iommu.c|  3 +++
>>>  drivers/iommu/intel-iommu.c  |  3 +++
>>>  drivers/iommu/iommu.c| 35 +++
>>>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>>>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>>>  drivers/vfio/vfio.c  | 15 +++
>>>  9 files changed, 99 insertions(+), 4 deletions(-)
>>>
>>
>>
> 
> 


-- 
Alexey

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

On 21/08/17 12:47, Alexey Kardashevskiy wrote:
> Folks,
> 
> Ok, people did talk, exchanged ideas, lovely :) What happens now? Do I
> repost this or go back to PCI bus flags or something else? Thanks.


Anyone, any help? How do we proceed with this? Thanks.



> 
> 
> 
> On 14/08/17 19:45, Alexey Kardashevskiy wrote:
>> Folks,
>>
>> Is there anything to change besides those compiler errors and David's
>> comment in 5/5? Or the while patchset is too bad? Thanks.
>>
>>
>>
>> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
>>> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
>>> mmapping MSI-X table"
>>> http://www.spinics.net/lists/kvm/msg152232.html
>>>
>>> This time it is using "caps" in IOMMU groups. The main question is if PCI
>>> bus flags or IOMMU domains are still better (and which one).
>>
>>>
>>>
>>>
>>> Here is some background:
>>>
>>> Current vfio-pci implementation disallows to mmap the page
>>> containing MSI-X table in case that users can write directly
>>> to MSI-X table and generate an incorrect MSIs.
>>>
>>> However, this will cause some performance issue when there
>>> are some critical device registers in the same page as the
>>> MSI-X table. We have to handle the mmio access to these
>>> registers in QEMU emulation rather than in guest.
>>>
>>> To solve this issue, this series allows to expose MSI-X table
>>> to userspace when hardware enables the capability of interrupt
>>> remapping which can ensure that a given PCI device can only
>>> shoot the MSIs assigned for it. And we introduce a new bus_flags
>>> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
>>> for different archs.
>>>
>>>
>>> This is based on sha1
>>> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
>>> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
>>>
>>> Please comment. Thanks.
>>>
>>> Changelog:
>>>
>>> v5:
>>> * redid the whole thing via so-called IOMMU group capabilities
>>>
>>> v4:
>>> * rebased on recent upstream
>>> * got all 6 patches from v2 (v3 was missing some)
>>>
>>>
>>>
>>>
>>> Alexey Kardashevskiy (5):
>>>   iommu: Add capabilities to a group
>>>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
>>> remapping
>>>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
>>> enabled
>>>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>>>   vfio-pci: Allow to expose MSI-X table to userspace when safe
>>>
>>>  include/linux/iommu.h| 20 
>>>  include/linux/vfio.h |  1 +
>>>  arch/powerpc/kernel/iommu.c  |  1 +
>>>  drivers/iommu/amd_iommu.c|  3 +++
>>>  drivers/iommu/intel-iommu.c  |  3 +++
>>>  drivers/iommu/iommu.c| 35 +++
>>>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>>>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>>>  drivers/vfio/vfio.c  | 15 +++
>>>  9 files changed, 99 insertions(+), 4 deletions(-)
>>>
>>
>>
> 
> 


-- 
Alexey

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

Folks,

Ok, people did talk, exchanged ideas, lovely :) What happens now? Do I
repost this or go back to PCI bus flags or something else? Thanks.



On 14/08/17 19:45, Alexey Kardashevskiy wrote:
> Folks,
> 
> Is there anything to change besides those compiler errors and David's
> comment in 5/5? Or the while patchset is too bad? Thanks.
> 
> 
> 
> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
>> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
>> mmapping MSI-X table"
>> http://www.spinics.net/lists/kvm/msg152232.html
>>
>> This time it is using "caps" in IOMMU groups. The main question is if PCI
>> bus flags or IOMMU domains are still better (and which one).
> 
>>
>>
>>
>> Here is some background:
>>
>> Current vfio-pci implementation disallows to mmap the page
>> containing MSI-X table in case that users can write directly
>> to MSI-X table and generate an incorrect MSIs.
>>
>> However, this will cause some performance issue when there
>> are some critical device registers in the same page as the
>> MSI-X table. We have to handle the mmio access to these
>> registers in QEMU emulation rather than in guest.
>>
>> To solve this issue, this series allows to expose MSI-X table
>> to userspace when hardware enables the capability of interrupt
>> remapping which can ensure that a given PCI device can only
>> shoot the MSIs assigned for it. And we introduce a new bus_flags
>> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
>> for different archs.
>>
>>
>> This is based on sha1
>> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
>> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
>>
>> Please comment. Thanks.
>>
>> Changelog:
>>
>> v5:
>> * redid the whole thing via so-called IOMMU group capabilities
>>
>> v4:
>> * rebased on recent upstream
>> * got all 6 patches from v2 (v3 was missing some)
>>
>>
>>
>>
>> Alexey Kardashevskiy (5):
>>   iommu: Add capabilities to a group
>>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
>> remapping
>>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
>> enabled
>>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>>   vfio-pci: Allow to expose MSI-X table to userspace when safe
>>
>>  include/linux/iommu.h| 20 
>>  include/linux/vfio.h |  1 +
>>  arch/powerpc/kernel/iommu.c  |  1 +
>>  drivers/iommu/amd_iommu.c|  3 +++
>>  drivers/iommu/intel-iommu.c  |  3 +++
>>  drivers/iommu/iommu.c| 35 +++
>>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>>  drivers/vfio/vfio.c  | 15 +++
>>  9 files changed, 99 insertions(+), 4 deletions(-)
>>
> 
> 


-- 
Alexey

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

Folks,

Ok, people did talk, exchanged ideas, lovely :) What happens now? Do I
repost this or go back to PCI bus flags or something else? Thanks.



On 14/08/17 19:45, Alexey Kardashevskiy wrote:
> Folks,
> 
> Is there anything to change besides those compiler errors and David's
> comment in 5/5? Or the while patchset is too bad? Thanks.
> 
> 
> 
> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
>> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
>> mmapping MSI-X table"
>> http://www.spinics.net/lists/kvm/msg152232.html
>>
>> This time it is using "caps" in IOMMU groups. The main question is if PCI
>> bus flags or IOMMU domains are still better (and which one).
> 
>>
>>
>>
>> Here is some background:
>>
>> Current vfio-pci implementation disallows to mmap the page
>> containing MSI-X table in case that users can write directly
>> to MSI-X table and generate an incorrect MSIs.
>>
>> However, this will cause some performance issue when there
>> are some critical device registers in the same page as the
>> MSI-X table. We have to handle the mmio access to these
>> registers in QEMU emulation rather than in guest.
>>
>> To solve this issue, this series allows to expose MSI-X table
>> to userspace when hardware enables the capability of interrupt
>> remapping which can ensure that a given PCI device can only
>> shoot the MSIs assigned for it. And we introduce a new bus_flags
>> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
>> for different archs.
>>
>>
>> This is based on sha1
>> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
>> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
>>
>> Please comment. Thanks.
>>
>> Changelog:
>>
>> v5:
>> * redid the whole thing via so-called IOMMU group capabilities
>>
>> v4:
>> * rebased on recent upstream
>> * got all 6 patches from v2 (v3 was missing some)
>>
>>
>>
>>
>> Alexey Kardashevskiy (5):
>>   iommu: Add capabilities to a group
>>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
>> remapping
>>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
>> enabled
>>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>>   vfio-pci: Allow to expose MSI-X table to userspace when safe
>>
>>  include/linux/iommu.h| 20 
>>  include/linux/vfio.h |  1 +
>>  arch/powerpc/kernel/iommu.c  |  1 +
>>  drivers/iommu/amd_iommu.c|  3 +++
>>  drivers/iommu/intel-iommu.c  |  3 +++
>>  drivers/iommu/iommu.c| 35 +++
>>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>>  drivers/vfio/vfio.c  | 15 +++
>>  9 files changed, 99 insertions(+), 4 deletions(-)
>>
> 
> 


-- 
Alexey

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alex Williamson]

On Thu, 17 Aug 2017 10:56:35 +
David Laight  wrote:

> From: Alex Williamson
> > Sent: 16 August 2017 17:56  
> ...
> > Firmware pissing match...  Processors running with 8k or less page size
> > fall within the recommendations of the PCI spec for register alignment
> > of MMIO regions of the device and this whole problem becomes less of an
> > issue.  
> 
> Actually if qemu is causing the MSI-X table accesses to fault, why doesn't
> it just lie to the guest about the physical address of the MSI-X table?
> Then mmio access to anything in the same physical page will just work.

That's an interesting idea, but now you need to add a BAR for the
virtualized vector table, but you'll also need to support extending a
BAR because there won't necessarily be a BAR available to add.  Of
course PCI requires natural alignment of BARs, thus an extra few bytes
on the end doubles the BAR size.  So also hope that if we need to
extend a BAR that there's a relatively small one available.  In either
case you're changing the layout of the device from what the driver might
expect.  We try pretty hard with device assignment to leave things in
the same place as they appear on bare metal, perhaps removing things,
but not actually moving things.  It might work in the majority of
cases, but it seems a bit precarious overall.  Thanks,

Alex

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alex Williamson]

On Thu, 17 Aug 2017 10:56:35 +
David Laight  wrote:

> From: Alex Williamson
> > Sent: 16 August 2017 17:56  
> ...
> > Firmware pissing match...  Processors running with 8k or less page size
> > fall within the recommendations of the PCI spec for register alignment
> > of MMIO regions of the device and this whole problem becomes less of an
> > issue.  
> 
> Actually if qemu is causing the MSI-X table accesses to fault, why doesn't
> it just lie to the guest about the physical address of the MSI-X table?
> Then mmio access to anything in the same physical page will just work.

That's an interesting idea, but now you need to add a BAR for the
virtualized vector table, but you'll also need to support extending a
BAR because there won't necessarily be a BAR available to add.  Of
course PCI requires natural alignment of BARs, thus an extra few bytes
on the end doubles the BAR size.  So also hope that if we need to
extend a BAR that there's a relatively small one available.  In either
case you're changing the layout of the device from what the driver might
expect.  We try pretty hard with device assignment to leave things in
the same place as they appear on bare metal, perhaps removing things,
but not actually moving things.  It might work in the majority of
cases, but it seems a bit precarious overall.  Thanks,

Alex

RE: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[David Laight]

From: Alex Williamson
> Sent: 16 August 2017 17:56
...
> Firmware pissing match...  Processors running with 8k or less page size
> fall within the recommendations of the PCI spec for register alignment
> of MMIO regions of the device and this whole problem becomes less of an
> issue.

Actually if qemu is causing the MSI-X table accesses to fault, why doesn't
it just lie to the guest about the physical address of the MSI-X table?
Then mmio access to anything in the same physical page will just work.

It has already been pointed out that you can't actually police the
interrupts that are raised without host hardware support.

Actually, putting other vectors in the MSI-X table is boring, most
drivers will ignore unexpected interrupts.
Much more interesting are physical memory addresses and accessible IO
addresses.
Of course, a lot of boards have PCI master capability and can probably
be persuaded to do writes to specific location anyway.

David

RE: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[David Laight]

From: Alex Williamson
> Sent: 16 August 2017 17:56
...
> Firmware pissing match...  Processors running with 8k or less page size
> fall within the recommendations of the PCI spec for register alignment
> of MMIO regions of the device and this whole problem becomes less of an
> issue.

Actually if qemu is causing the MSI-X table accesses to fault, why doesn't
it just lie to the guest about the physical address of the MSI-X table?
Then mmio access to anything in the same physical page will just work.

It has already been pointed out that you can't actually police the
interrupts that are raised without host hardware support.

Actually, putting other vectors in the MSI-X table is boring, most
drivers will ignore unexpected interrupts.
Much more interesting are physical memory addresses and accessible IO
addresses.
Of course, a lot of boards have PCI master capability and can probably
be persuaded to do writes to specific location anyway.

David

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Wed, 2017-08-16 at 10:56 -0600, Alex Williamson wrote:
> 
> > WTF  Alex, can you stop once and for all with all that "POWER is
> > not standard" bullshit please ? It's completely wrong.
> 
> As you've stated, the MSI-X vector table on POWER is currently updated
> via a hypercall.  POWER is overall PCI compliant (I assume), but the
> guest does not directly modify the vector table in MMIO space of the
> device.  This is important...

Well no. On qemu the guest doesn't always (but it can save/restore it),
but on PowerVM this is done by the FW running inside the partition
itself. And that firmware just does normal stores to the device table.

IE. The problem here isn't so much who does the actual stores to the
device table but where they get the address and data values from, which
isn't covered by the spec.

The added fact that qemu hijacks the stores not just to "remap" them
but also do the whole reuqesting of the interrupt etc... in the host
system is a qemu design choice which also hasn't any relation to the
spec (and arguably isnt' a great choice for our systems).

For example, on PowerVM, the HV assigns a pile of MSIs to the guest to
assign to its devices. The FW inside the guest does a default
assignment but that can be changed.

Thus the interrupts are effectively "hooked up" at the HV level at the
point where the PCI bridge is mapped into the guest.

> > This has nothing to do with PCIe standard !
> 
> Yes, it actually does, because if the guest relies on the vector table
> to be virtualized then it doesn't particularly matter whether the
> vfio-pci kernel driver allows that portion of device MMIO space to be
> directly accessed or mapped because QEMU needs for it to be trapped in
> order to provide that virtualization.

And this has nothing to do with the PCIe standard... this has
everything to do with a combination of qemu design choices and
defficient FW interfaces on x86 platforms.

> I'm not knocking POWER, it's a smart thing for virtualization to have
> defined this hypercall which negates the need for vector table
> virtualization and allows efficient mapping of the device.  On other
> platform, it's not necessarily practical given the broad base of legacy
> guests supported where we'd never get agreement to implement this as
> part of the platform spec... if there even was such a thing.  Maybe we
> could provide the hypercall and dynamically enable direct vector table
> mapping (disabling vector table virtualization) only if the hypercall
> is used.

No I think a better approach would be to provide the guest with a pile
of MSIs to use with devices and have FW (such as ACPI) tell the guest
about them.

> > The PCIe standard says strictly *nothing* whatsoever about how an OS
> > obtains the magic address/values to put in the device and how the PCIe
> > host bridge may do appropriate fitering.
> 
> And now we've jumped the tracks...  The only way the platform specific
> address/data values become important is if we allow direct access to
> the vector table AND now we're formulating how the user/guest might
> write to it directly.  Otherwise the virtualization of the vector
> table, or paravirtualization via hypercall provides the translation
> where the host and guest address/data pairs can operate in completely
> different address spaces.

They can regardless if things are done properly :-)

> > There is nothing on POWER that prevents the guest from writing the MSI-
> > X address/data by hand. The problem isn't who writes the values or even
> > how. The problem breaks down into these two things that are NOT covered
> > by any aspect of the PCIe standard:
> 
> You've moved on to a different problem, I think everyone aside from
> POWER is still back at the problem where who writes the vector table
> values is a forefront problem.
>  
> >   1- The OS needs to obtain address/data values for an MSI that will
> > "work" for the device.
> > 
> >   2- The HW+HV needs to prevent collateral damage caused by a device
> > issuing stores to incorrect address or with incorrect data. Now *this*
> > is necessary for *ANY* kind of DMA whether it's an MSI or something
> > else anyway.
> > 
> > Now, the filtering done by qemu is NOT a reasonable way to handle 2)
> > and whatever excluse about "making it harder" doesn't fly a meter when
> > it comes to security. Making it "harder to break accidentally" I also
> > don't buy, people don't just randomly put things in their MSI-X tables
> > "accidentally", that stuff works or doesn't.
> 
> As I said before, I'm not willing to preserve the weak attributes that
> blocking direct vector table access provides over pursuing a more
> performant interface, but I also don't think their value is absolute
> zero either.
> 
> > That leaves us with 1). Now this is purely a platform specific matters,
> > not a spec matter. Once the HW has a way to enforce you can only
> > generate "allowed" MSIs it becomes a matter of having some FW mechanism
> > that can be used to 

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Wed, 2017-08-16 at 10:56 -0600, Alex Williamson wrote:
> 
> > WTF  Alex, can you stop once and for all with all that "POWER is
> > not standard" bullshit please ? It's completely wrong.
> 
> As you've stated, the MSI-X vector table on POWER is currently updated
> via a hypercall.  POWER is overall PCI compliant (I assume), but the
> guest does not directly modify the vector table in MMIO space of the
> device.  This is important...

Well no. On qemu the guest doesn't always (but it can save/restore it),
but on PowerVM this is done by the FW running inside the partition
itself. And that firmware just does normal stores to the device table.

IE. The problem here isn't so much who does the actual stores to the
device table but where they get the address and data values from, which
isn't covered by the spec.

The added fact that qemu hijacks the stores not just to "remap" them
but also do the whole reuqesting of the interrupt etc... in the host
system is a qemu design choice which also hasn't any relation to the
spec (and arguably isnt' a great choice for our systems).

For example, on PowerVM, the HV assigns a pile of MSIs to the guest to
assign to its devices. The FW inside the guest does a default
assignment but that can be changed.

Thus the interrupts are effectively "hooked up" at the HV level at the
point where the PCI bridge is mapped into the guest.

> > This has nothing to do with PCIe standard !
> 
> Yes, it actually does, because if the guest relies on the vector table
> to be virtualized then it doesn't particularly matter whether the
> vfio-pci kernel driver allows that portion of device MMIO space to be
> directly accessed or mapped because QEMU needs for it to be trapped in
> order to provide that virtualization.

And this has nothing to do with the PCIe standard... this has
everything to do with a combination of qemu design choices and
defficient FW interfaces on x86 platforms.

> I'm not knocking POWER, it's a smart thing for virtualization to have
> defined this hypercall which negates the need for vector table
> virtualization and allows efficient mapping of the device.  On other
> platform, it's not necessarily practical given the broad base of legacy
> guests supported where we'd never get agreement to implement this as
> part of the platform spec... if there even was such a thing.  Maybe we
> could provide the hypercall and dynamically enable direct vector table
> mapping (disabling vector table virtualization) only if the hypercall
> is used.

No I think a better approach would be to provide the guest with a pile
of MSIs to use with devices and have FW (such as ACPI) tell the guest
about them.

> > The PCIe standard says strictly *nothing* whatsoever about how an OS
> > obtains the magic address/values to put in the device and how the PCIe
> > host bridge may do appropriate fitering.
> 
> And now we've jumped the tracks...  The only way the platform specific
> address/data values become important is if we allow direct access to
> the vector table AND now we're formulating how the user/guest might
> write to it directly.  Otherwise the virtualization of the vector
> table, or paravirtualization via hypercall provides the translation
> where the host and guest address/data pairs can operate in completely
> different address spaces.

They can regardless if things are done properly :-)

> > There is nothing on POWER that prevents the guest from writing the MSI-
> > X address/data by hand. The problem isn't who writes the values or even
> > how. The problem breaks down into these two things that are NOT covered
> > by any aspect of the PCIe standard:
> 
> You've moved on to a different problem, I think everyone aside from
> POWER is still back at the problem where who writes the vector table
> values is a forefront problem.
>  
> >   1- The OS needs to obtain address/data values for an MSI that will
> > "work" for the device.
> > 
> >   2- The HW+HV needs to prevent collateral damage caused by a device
> > issuing stores to incorrect address or with incorrect data. Now *this*
> > is necessary for *ANY* kind of DMA whether it's an MSI or something
> > else anyway.
> > 
> > Now, the filtering done by qemu is NOT a reasonable way to handle 2)
> > and whatever excluse about "making it harder" doesn't fly a meter when
> > it comes to security. Making it "harder to break accidentally" I also
> > don't buy, people don't just randomly put things in their MSI-X tables
> > "accidentally", that stuff works or doesn't.
> 
> As I said before, I'm not willing to preserve the weak attributes that
> blocking direct vector table access provides over pursuing a more
> performant interface, but I also don't think their value is absolute
> zero either.
> 
> > That leaves us with 1). Now this is purely a platform specific matters,
> > not a spec matter. Once the HW has a way to enforce you can only
> > generate "allowed" MSIs it becomes a matter of having some FW mechanism
> > that can be used to 

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alex Williamson]

On Wed, 16 Aug 2017 10:35:49 +1000
Benjamin Herrenschmidt  wrote:

> On Tue, 2017-08-15 at 10:37 -0600, Alex Williamson wrote:
> > Of course I don't think either of those are worth imposing a
> > performance penalty where we don't otherwise need one.  However, if we
> > look at a VM scenario where the guest is following the PCI standard for
> > programming MSI-X interrupts (ie. not POWER), we need some mechanism to
> > intercept those MMIO writes to the vector table and configure the host
> > interrupt domain of the device rather than allowing the guest direct
> > access.  This is simply part of virtualizing the device to the guest.
> > So even if the kernel allows mmap'ing the vector table, the hypervisor
> > needs to trap it, so the mmap isn't required or used anyway.  It's only
> > when you define a non-PCI standard for your guest to program
> > interrupts, as POWER has done, and can therefore trust that the
> > hypervisor does not need to trap on the vector table that having that
> > mmap'able vector table becomes fully useful.  AIUI, ARM supports 64k
> > pages too... does ARM have any strategy that would actually make it
> > possible to make use of an mmap covering the vector table?  Thanks,  
> 
> WTF  Alex, can you stop once and for all with all that "POWER is
> not standard" bullshit please ? It's completely wrong.

As you've stated, the MSI-X vector table on POWER is currently updated
via a hypercall.  POWER is overall PCI compliant (I assume), but the
guest does not directly modify the vector table in MMIO space of the
device.  This is important...

> This has nothing to do with PCIe standard !

Yes, it actually does, because if the guest relies on the vector table
to be virtualized then it doesn't particularly matter whether the
vfio-pci kernel driver allows that portion of device MMIO space to be
directly accessed or mapped because QEMU needs for it to be trapped in
order to provide that virtualization.

I'm not knocking POWER, it's a smart thing for virtualization to have
defined this hypercall which negates the need for vector table
virtualization and allows efficient mapping of the device.  On other
platform, it's not necessarily practical given the broad base of legacy
guests supported where we'd never get agreement to implement this as
part of the platform spec... if there even was such a thing.  Maybe we
could provide the hypercall and dynamically enable direct vector table
mapping (disabling vector table virtualization) only if the hypercall
is used.

> The PCIe standard says strictly *nothing* whatsoever about how an OS
> obtains the magic address/values to put in the device and how the PCIe
> host bridge may do appropriate fitering.

And now we've jumped the tracks...  The only way the platform specific
address/data values become important is if we allow direct access to
the vector table AND now we're formulating how the user/guest might
write to it directly.  Otherwise the virtualization of the vector
table, or paravirtualization via hypercall provides the translation
where the host and guest address/data pairs can operate in completely
different address spaces.

> There is nothing on POWER that prevents the guest from writing the MSI-
> X address/data by hand. The problem isn't who writes the values or even
> how. The problem breaks down into these two things that are NOT covered
> by any aspect of the PCIe standard:

You've moved on to a different problem, I think everyone aside from
POWER is still back at the problem where who writes the vector table
values is a forefront problem.
 
>   1- The OS needs to obtain address/data values for an MSI that will
> "work" for the device.
> 
>   2- The HW+HV needs to prevent collateral damage caused by a device
> issuing stores to incorrect address or with incorrect data. Now *this*
> is necessary for *ANY* kind of DMA whether it's an MSI or something
> else anyway.
> 
> Now, the filtering done by qemu is NOT a reasonable way to handle 2)
> and whatever excluse about "making it harder" doesn't fly a meter when
> it comes to security. Making it "harder to break accidentally" I also
> don't buy, people don't just randomly put things in their MSI-X tables
> "accidentally", that stuff works or doesn't.

As I said before, I'm not willing to preserve the weak attributes that
blocking direct vector table access provides over pursuing a more
performant interface, but I also don't think their value is absolute
zero either.

> That leaves us with 1). Now this is purely a platform specific matters,
> not a spec matter. Once the HW has a way to enforce you can only
> generate "allowed" MSIs it becomes a matter of having some FW mechanism
> that can be used to informed the OS what address/values to use for a
> given interrupts.
> 
> This is provided on POWER by a combination of device-tree and RTAS. It
> could be that x86/ARM64 doesn't provide good enough mechanisms via ACPI
> but this is no way a problem of standard 

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alex Williamson]

On Wed, 16 Aug 2017 10:35:49 +1000
Benjamin Herrenschmidt  wrote:

> On Tue, 2017-08-15 at 10:37 -0600, Alex Williamson wrote:
> > Of course I don't think either of those are worth imposing a
> > performance penalty where we don't otherwise need one.  However, if we
> > look at a VM scenario where the guest is following the PCI standard for
> > programming MSI-X interrupts (ie. not POWER), we need some mechanism to
> > intercept those MMIO writes to the vector table and configure the host
> > interrupt domain of the device rather than allowing the guest direct
> > access.  This is simply part of virtualizing the device to the guest.
> > So even if the kernel allows mmap'ing the vector table, the hypervisor
> > needs to trap it, so the mmap isn't required or used anyway.  It's only
> > when you define a non-PCI standard for your guest to program
> > interrupts, as POWER has done, and can therefore trust that the
> > hypervisor does not need to trap on the vector table that having that
> > mmap'able vector table becomes fully useful.  AIUI, ARM supports 64k
> > pages too... does ARM have any strategy that would actually make it
> > possible to make use of an mmap covering the vector table?  Thanks,  
> 
> WTF  Alex, can you stop once and for all with all that "POWER is
> not standard" bullshit please ? It's completely wrong.

As you've stated, the MSI-X vector table on POWER is currently updated
via a hypercall.  POWER is overall PCI compliant (I assume), but the
guest does not directly modify the vector table in MMIO space of the
device.  This is important...

> This has nothing to do with PCIe standard !

Yes, it actually does, because if the guest relies on the vector table
to be virtualized then it doesn't particularly matter whether the
vfio-pci kernel driver allows that portion of device MMIO space to be
directly accessed or mapped because QEMU needs for it to be trapped in
order to provide that virtualization.

I'm not knocking POWER, it's a smart thing for virtualization to have
defined this hypercall which negates the need for vector table
virtualization and allows efficient mapping of the device.  On other
platform, it's not necessarily practical given the broad base of legacy
guests supported where we'd never get agreement to implement this as
part of the platform spec... if there even was such a thing.  Maybe we
could provide the hypercall and dynamically enable direct vector table
mapping (disabling vector table virtualization) only if the hypercall
is used.

> The PCIe standard says strictly *nothing* whatsoever about how an OS
> obtains the magic address/values to put in the device and how the PCIe
> host bridge may do appropriate fitering.

And now we've jumped the tracks...  The only way the platform specific
address/data values become important is if we allow direct access to
the vector table AND now we're formulating how the user/guest might
write to it directly.  Otherwise the virtualization of the vector
table, or paravirtualization via hypercall provides the translation
where the host and guest address/data pairs can operate in completely
different address spaces.

> There is nothing on POWER that prevents the guest from writing the MSI-
> X address/data by hand. The problem isn't who writes the values or even
> how. The problem breaks down into these two things that are NOT covered
> by any aspect of the PCIe standard:

You've moved on to a different problem, I think everyone aside from
POWER is still back at the problem where who writes the vector table
values is a forefront problem.
 
>   1- The OS needs to obtain address/data values for an MSI that will
> "work" for the device.
> 
>   2- The HW+HV needs to prevent collateral damage caused by a device
> issuing stores to incorrect address or with incorrect data. Now *this*
> is necessary for *ANY* kind of DMA whether it's an MSI or something
> else anyway.
> 
> Now, the filtering done by qemu is NOT a reasonable way to handle 2)
> and whatever excluse about "making it harder" doesn't fly a meter when
> it comes to security. Making it "harder to break accidentally" I also
> don't buy, people don't just randomly put things in their MSI-X tables
> "accidentally", that stuff works or doesn't.

As I said before, I'm not willing to preserve the weak attributes that
blocking direct vector table access provides over pursuing a more
performant interface, but I also don't think their value is absolute
zero either.

> That leaves us with 1). Now this is purely a platform specific matters,
> not a spec matter. Once the HW has a way to enforce you can only
> generate "allowed" MSIs it becomes a matter of having some FW mechanism
> that can be used to informed the OS what address/values to use for a
> given interrupts.
> 
> This is provided on POWER by a combination of device-tree and RTAS. It
> could be that x86/ARM64 doesn't provide good enough mechanisms via ACPI
> but this is no way a problem of standard compliance, just inferior
> 

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Tue, 2017-08-15 at 10:37 -0600, Alex Williamson wrote:
> Of course I don't think either of those are worth imposing a
> performance penalty where we don't otherwise need one.  However, if we
> look at a VM scenario where the guest is following the PCI standard for
> programming MSI-X interrupts (ie. not POWER), we need some mechanism to
> intercept those MMIO writes to the vector table and configure the host
> interrupt domain of the device rather than allowing the guest direct
> access.  This is simply part of virtualizing the device to the guest.
> So even if the kernel allows mmap'ing the vector table, the hypervisor
> needs to trap it, so the mmap isn't required or used anyway.  It's only
> when you define a non-PCI standard for your guest to program
> interrupts, as POWER has done, and can therefore trust that the
> hypervisor does not need to trap on the vector table that having that
> mmap'able vector table becomes fully useful.  AIUI, ARM supports 64k
> pages too... does ARM have any strategy that would actually make it
> possible to make use of an mmap covering the vector table?  Thanks,

WTF  Alex, can you stop once and for all with all that "POWER is
not standard" bullshit please ? It's completely wrong.

This has nothing to do with PCIe standard !

The PCIe standard says strictly *nothing* whatsoever about how an OS
obtains the magic address/values to put in the device and how the PCIe
host bridge may do appropriate fitering.

There is nothing on POWER that prevents the guest from writing the MSI-
X address/data by hand. The problem isn't who writes the values or even
how. The problem breaks down into these two things that are NOT covered
by any aspect of the PCIe standard:

  1- The OS needs to obtain address/data values for an MSI that will
"work" for the device.

  2- The HW+HV needs to prevent collateral damage caused by a device
issuing stores to incorrect address or with incorrect data. Now *this*
is necessary for *ANY* kind of DMA whether it's an MSI or something
else anyway.

Now, the filtering done by qemu is NOT a reasonable way to handle 2)
and whatever excluse about "making it harder" doesn't fly a meter when
it comes to security. Making it "harder to break accidentally" I also
don't buy, people don't just randomly put things in their MSI-X tables
"accidentally", that stuff works or doesn't.

That leaves us with 1). Now this is purely a platform specific matters,
not a spec matter. Once the HW has a way to enforce you can only
generate "allowed" MSIs it becomes a matter of having some FW mechanism
that can be used to informed the OS what address/values to use for a
given interrupts.

This is provided on POWER by a combination of device-tree and RTAS. It
could be that x86/ARM64 doesn't provide good enough mechanisms via ACPI
but this is no way a problem of standard compliance, just inferior
firmware interfaces.

So again, for the 234789246th time in years, can we get that 1-bit-of-
information sorted one way or another so we can fix our massive
performance issue instead of adding yet another dozen layers of paint
on that shed ?

Ben.

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Tue, 2017-08-15 at 10:37 -0600, Alex Williamson wrote:
> Of course I don't think either of those are worth imposing a
> performance penalty where we don't otherwise need one.  However, if we
> look at a VM scenario where the guest is following the PCI standard for
> programming MSI-X interrupts (ie. not POWER), we need some mechanism to
> intercept those MMIO writes to the vector table and configure the host
> interrupt domain of the device rather than allowing the guest direct
> access.  This is simply part of virtualizing the device to the guest.
> So even if the kernel allows mmap'ing the vector table, the hypervisor
> needs to trap it, so the mmap isn't required or used anyway.  It's only
> when you define a non-PCI standard for your guest to program
> interrupts, as POWER has done, and can therefore trust that the
> hypervisor does not need to trap on the vector table that having that
> mmap'able vector table becomes fully useful.  AIUI, ARM supports 64k
> pages too... does ARM have any strategy that would actually make it
> possible to make use of an mmap covering the vector table?  Thanks,

WTF  Alex, can you stop once and for all with all that "POWER is
not standard" bullshit please ? It's completely wrong.

This has nothing to do with PCIe standard !

The PCIe standard says strictly *nothing* whatsoever about how an OS
obtains the magic address/values to put in the device and how the PCIe
host bridge may do appropriate fitering.

There is nothing on POWER that prevents the guest from writing the MSI-
X address/data by hand. The problem isn't who writes the values or even
how. The problem breaks down into these two things that are NOT covered
by any aspect of the PCIe standard:

  1- The OS needs to obtain address/data values for an MSI that will
"work" for the device.

  2- The HW+HV needs to prevent collateral damage caused by a device
issuing stores to incorrect address or with incorrect data. Now *this*
is necessary for *ANY* kind of DMA whether it's an MSI or something
else anyway.

Now, the filtering done by qemu is NOT a reasonable way to handle 2)
and whatever excluse about "making it harder" doesn't fly a meter when
it comes to security. Making it "harder to break accidentally" I also
don't buy, people don't just randomly put things in their MSI-X tables
"accidentally", that stuff works or doesn't.

That leaves us with 1). Now this is purely a platform specific matters,
not a spec matter. Once the HW has a way to enforce you can only
generate "allowed" MSIs it becomes a matter of having some FW mechanism
that can be used to informed the OS what address/values to use for a
given interrupts.

This is provided on POWER by a combination of device-tree and RTAS. It
could be that x86/ARM64 doesn't provide good enough mechanisms via ACPI
but this is no way a problem of standard compliance, just inferior
firmware interfaces.

So again, for the 234789246th time in years, can we get that 1-bit-of-
information sorted one way or another so we can fix our massive
performance issue instead of adding yet another dozen layers of paint
on that shed ?

Ben.

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alex Williamson]

On Mon, 14 Aug 2017 14:12:33 +0100
Robin Murphy  wrote:

> On 14/08/17 10:45, Alexey Kardashevskiy wrote:
> > Folks,
> > 
> > Is there anything to change besides those compiler errors and David's
> > comment in 5/5? Or the while patchset is too bad? Thanks.  
> 
> While I now understand it's not the low-level thing I first thought it
> was, so my reasoning has changed, personally I don't like this approach
> any more than the previous one - it still smells of abusing external
> APIs to pass information from one part of VFIO to another (and it has
> the same conceptual problem of attributing something to interrupt
> sources that is actually a property of the interrupt target).
> 
> Taking a step back, though, why does vfio-pci perform this check in the
> first place? If a malicious guest already has control of a device, any
> kind of interrupt spoofing it could do by fiddling with the MSI-X
> message address/data it could simply do with a DMA write anyway, so the
> security argument doesn't stand up in general (sure, not all PCIe
> devices may be capable of arbitrary DMA, but that seems like more of a
> tenuous security-by-obscurity angle to me). Besides, with Type1 IOMMU
> the fact that we've let a device be assigned at all means that this is
> already a non-issue (because either the hardware provides isolation or
> the user has explicitly accepted the consequences of an unsafe
> configuration) - from patch #4 that's apparently the same for SPAPR TCE,
> in which case it seems this flag doesn't even need to be propagated and
> could simply be assumed always.
> 
> On the other hand, if the check is not so much to mitigate malicious
> guests attacking the system as to prevent dumb guests breaking
> themselves (e.g. if some or all of the MSI-X capability is actually
> emulated), then allowing things to sometimes go wrong on the grounds of
> an irrelevant hardware feature doesn't seem correct :/

While the theoretical security provided by preventing direct access to
the MSI-X vector table may be mostly a matter of obfuscation, in
practice, I think it changes the problem of creating arbitrary DMA
writes from a generic, trivial, PCI spec based exercise to a more device
specific challenge.  I do however have evidence that there are
consumers of the vfio API who would have attempted to program device
interrupts by directly manipulating the vector table had they not been
prevented from doing so and contacting me to learn about the SET_IRQ
ioctl.  Therefore I think the behavior also contributes to making the
overall API more difficult to use incorrectly.

Of course I don't think either of those are worth imposing a
performance penalty where we don't otherwise need one.  However, if we
look at a VM scenario where the guest is following the PCI standard for
programming MSI-X interrupts (ie. not POWER), we need some mechanism to
intercept those MMIO writes to the vector table and configure the host
interrupt domain of the device rather than allowing the guest direct
access.  This is simply part of virtualizing the device to the guest.
So even if the kernel allows mmap'ing the vector table, the hypervisor
needs to trap it, so the mmap isn't required or used anyway.  It's only
when you define a non-PCI standard for your guest to program
interrupts, as POWER has done, and can therefore trust that the
hypervisor does not need to trap on the vector table that having that
mmap'able vector table becomes fully useful.  AIUI, ARM supports 64k
pages too... does ARM have any strategy that would actually make it
possible to make use of an mmap covering the vector table?  Thanks,

Alex

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alex Williamson]

On Mon, 14 Aug 2017 14:12:33 +0100
Robin Murphy  wrote:

> On 14/08/17 10:45, Alexey Kardashevskiy wrote:
> > Folks,
> > 
> > Is there anything to change besides those compiler errors and David's
> > comment in 5/5? Or the while patchset is too bad? Thanks.  
> 
> While I now understand it's not the low-level thing I first thought it
> was, so my reasoning has changed, personally I don't like this approach
> any more than the previous one - it still smells of abusing external
> APIs to pass information from one part of VFIO to another (and it has
> the same conceptual problem of attributing something to interrupt
> sources that is actually a property of the interrupt target).
> 
> Taking a step back, though, why does vfio-pci perform this check in the
> first place? If a malicious guest already has control of a device, any
> kind of interrupt spoofing it could do by fiddling with the MSI-X
> message address/data it could simply do with a DMA write anyway, so the
> security argument doesn't stand up in general (sure, not all PCIe
> devices may be capable of arbitrary DMA, but that seems like more of a
> tenuous security-by-obscurity angle to me). Besides, with Type1 IOMMU
> the fact that we've let a device be assigned at all means that this is
> already a non-issue (because either the hardware provides isolation or
> the user has explicitly accepted the consequences of an unsafe
> configuration) - from patch #4 that's apparently the same for SPAPR TCE,
> in which case it seems this flag doesn't even need to be propagated and
> could simply be assumed always.
> 
> On the other hand, if the check is not so much to mitigate malicious
> guests attacking the system as to prevent dumb guests breaking
> themselves (e.g. if some or all of the MSI-X capability is actually
> emulated), then allowing things to sometimes go wrong on the grounds of
> an irrelevant hardware feature doesn't seem correct :/

While the theoretical security provided by preventing direct access to
the MSI-X vector table may be mostly a matter of obfuscation, in
practice, I think it changes the problem of creating arbitrary DMA
writes from a generic, trivial, PCI spec based exercise to a more device
specific challenge.  I do however have evidence that there are
consumers of the vfio API who would have attempted to program device
interrupts by directly manipulating the vector table had they not been
prevented from doing so and contacting me to learn about the SET_IRQ
ioctl.  Therefore I think the behavior also contributes to making the
overall API more difficult to use incorrectly.

Of course I don't think either of those are worth imposing a
performance penalty where we don't otherwise need one.  However, if we
look at a VM scenario where the guest is following the PCI standard for
programming MSI-X interrupts (ie. not POWER), we need some mechanism to
intercept those MMIO writes to the vector table and configure the host
interrupt domain of the device rather than allowing the guest direct
access.  This is simply part of virtualizing the device to the guest.
So even if the kernel allows mmap'ing the vector table, the hypervisor
needs to trap it, so the mmap isn't required or used anyway.  It's only
when you define a non-PCI standard for your guest to program
interrupts, as POWER has done, and can therefore trust that the
hypervisor does not need to trap on the vector table that having that
mmap'able vector table becomes fully useful.  AIUI, ARM supports 64k
pages too... does ARM have any strategy that would actually make it
possible to make use of an mmap covering the vector table?  Thanks,

Alex

RE: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[David Laight]

From: Benjamin Herrenschmidt
> Sent: 15 August 2017 02:34
> On Tue, 2017-08-15 at 09:16 +0800, Jike Song wrote:
> > > Taking a step back, though, why does vfio-pci perform this check in the
> > > first place? If a malicious guest already has control of a device, any
> > > kind of interrupt spoofing it could do by fiddling with the MSI-X
> > > message address/data it could simply do with a DMA write anyway, so the
> > > security argument doesn't stand up in general (sure, not all PCIe
> > > devices may be capable of arbitrary DMA, but that seems like more of a
> > > tenuous security-by-obscurity angle to me).
> 
> I tried to make that point for years, thanks for re-iterating it :-)

Indeed, we have an FPGA based PCIe card where the MSI-X table is just a
piece of PCIe accessible memory.
The device driver has to read the MSI-X table and write the address+data
values to other registers which are then used to raise the interrupt.
(Ok, I've written a better interrupt generator so we don't do that
any more.)

David

RE: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[David Laight]

From: Benjamin Herrenschmidt
> Sent: 15 August 2017 02:34
> On Tue, 2017-08-15 at 09:16 +0800, Jike Song wrote:
> > > Taking a step back, though, why does vfio-pci perform this check in the
> > > first place? If a malicious guest already has control of a device, any
> > > kind of interrupt spoofing it could do by fiddling with the MSI-X
> > > message address/data it could simply do with a DMA write anyway, so the
> > > security argument doesn't stand up in general (sure, not all PCIe
> > > devices may be capable of arbitrary DMA, but that seems like more of a
> > > tenuous security-by-obscurity angle to me).
> 
> I tried to make that point for years, thanks for re-iterating it :-)

Indeed, we have an FPGA based PCIe card where the MSI-X table is just a
piece of PCIe accessible memory.
The device driver has to read the MSI-X table and write the address+data
values to other registers which are then used to raise the interrupt.
(Ok, I've written a better interrupt generator so we don't do that
any more.)

David

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Mon, 2017-08-14 at 14:12 +0100, Robin Murphy wrote:
> On the other hand, if the check is not so much to mitigate malicious
> guests attacking the system as to prevent dumb guests breaking
> themselves (e.g. if some or all of the MSI-X capability is actually
> emulated), then allowing things to sometimes go wrong on the grounds of
> an irrelevant hardware feature doesn't seem correct :/

There is 0 value in trying to prevent the guest kernel from shooting
itself in the foot. There are so many other ways it can do it that I
fail the point of even attempting it here.

In addition, this actually harms performance on some devices. There
are cases where the MSI-X table shares a page with other registrers
that are used during normal device operation. This is especially
problematic on architectures such as powerpc that use 64K pages.

Those devices thus suffer a massive performance loss, for the sake of
something that never happens in practice (especially on pseries where
the MSI configuration is done by paravirt calls, thus by qemu itself).

Cheers,
Ben.

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Mon, 2017-08-14 at 14:12 +0100, Robin Murphy wrote:
> On the other hand, if the check is not so much to mitigate malicious
> guests attacking the system as to prevent dumb guests breaking
> themselves (e.g. if some or all of the MSI-X capability is actually
> emulated), then allowing things to sometimes go wrong on the grounds of
> an irrelevant hardware feature doesn't seem correct :/

There is 0 value in trying to prevent the guest kernel from shooting
itself in the foot. There are so many other ways it can do it that I
fail the point of even attempting it here.

In addition, this actually harms performance on some devices. There
are cases where the MSI-X table shares a page with other registrers
that are used during normal device operation. This is especially
problematic on architectures such as powerpc that use 64K pages.

Those devices thus suffer a massive performance loss, for the sake of
something that never happens in practice (especially on pseries where
the MSI configuration is done by paravirt calls, thus by qemu itself).

Cheers,
Ben.

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Tue, 2017-08-15 at 09:47 +0800, Jike Song wrote:
> On 08/15/2017 09:33 AM, Benjamin Herrenschmidt wrote:
> > On Tue, 2017-08-15 at 09:16 +0800, Jike Song wrote:
> > > > Taking a step back, though, why does vfio-pci perform this check in the
> > > > first place? If a malicious guest already has control of a device, any
> > > > kind of interrupt spoofing it could do by fiddling with the MSI-X
> > > > message address/data it could simply do with a DMA write anyway, so the
> > > > security argument doesn't stand up in general (sure, not all PCIe
> > > > devices may be capable of arbitrary DMA, but that seems like more of a
> > > > tenuous security-by-obscurity angle to me).
> > 
> > I tried to make that point for years, thanks for re-iterating it :-)
> > 
> > > Hi Robin,
> > > 
> > > DMA writes will be translated (thereby censored) by DMA Remapping 
> > > hardware,
> > > while MSI/MSI-X will not. Is this different for non-x86?
> > 
> > There is no way your DMA remapping HW can differenciate. The only
> > difference between a DMA write and an MSI is ... the address. So if I
> > can make my device DMA to the MSI address range, I've defeated your
> > security.
> 
> I don't think with IRQ remapping enabled, you can make your device DMA to
> MSI address, without being treated as an IRQ and remapped. If so, the IRQ
> remapping hardware is simply broken :)

You are mixing things here.

Robin's point is that there is no security provided by the obfuscating
of the MSI-X table by qemu because whatever qemu does to "filter" the
MSI-X targer addresses can be worked around by making the device DMA
wherever you want.

None of what you say invalidates that basic fact.

Now, as far as your remapping HW goes, either it filters interrupts or
it doesn't. If it does then yes, it can't be spoofed, and thus you
don't need the filtering of the table in qemu.

If it doesn't, then the guest can spoof any interrupt using DMAs and
whatever qemu does to filter the table is not going to fix it.

Thus the point remains that the only value in qemu filtering the table
is to enable already insecure use cases to work, without actually
making them any more secure.

Ben.

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Tue, 2017-08-15 at 09:47 +0800, Jike Song wrote:
> On 08/15/2017 09:33 AM, Benjamin Herrenschmidt wrote:
> > On Tue, 2017-08-15 at 09:16 +0800, Jike Song wrote:
> > > > Taking a step back, though, why does vfio-pci perform this check in the
> > > > first place? If a malicious guest already has control of a device, any
> > > > kind of interrupt spoofing it could do by fiddling with the MSI-X
> > > > message address/data it could simply do with a DMA write anyway, so the
> > > > security argument doesn't stand up in general (sure, not all PCIe
> > > > devices may be capable of arbitrary DMA, but that seems like more of a
> > > > tenuous security-by-obscurity angle to me).
> > 
> > I tried to make that point for years, thanks for re-iterating it :-)
> > 
> > > Hi Robin,
> > > 
> > > DMA writes will be translated (thereby censored) by DMA Remapping 
> > > hardware,
> > > while MSI/MSI-X will not. Is this different for non-x86?
> > 
> > There is no way your DMA remapping HW can differenciate. The only
> > difference between a DMA write and an MSI is ... the address. So if I
> > can make my device DMA to the MSI address range, I've defeated your
> > security.
> 
> I don't think with IRQ remapping enabled, you can make your device DMA to
> MSI address, without being treated as an IRQ and remapped. If so, the IRQ
> remapping hardware is simply broken :)

You are mixing things here.

Robin's point is that there is no security provided by the obfuscating
of the MSI-X table by qemu because whatever qemu does to "filter" the
MSI-X targer addresses can be worked around by making the device DMA
wherever you want.

None of what you say invalidates that basic fact.

Now, as far as your remapping HW goes, either it filters interrupts or
it doesn't. If it does then yes, it can't be spoofed, and thus you
don't need the filtering of the table in qemu.

If it doesn't, then the guest can spoof any interrupt using DMAs and
whatever qemu does to filter the table is not going to fix it.

Thus the point remains that the only value in qemu filtering the table
is to enable already insecure use cases to work, without actually
making them any more secure.

Ben.

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Tue, 2017-08-15 at 09:16 +0800, Jike Song wrote:
> > Taking a step back, though, why does vfio-pci perform this check in the
> > first place? If a malicious guest already has control of a device, any
> > kind of interrupt spoofing it could do by fiddling with the MSI-X
> > message address/data it could simply do with a DMA write anyway, so the
> > security argument doesn't stand up in general (sure, not all PCIe
> > devices may be capable of arbitrary DMA, but that seems like more of a
> > tenuous security-by-obscurity angle to me).

I tried to make that point for years, thanks for re-iterating it :-)

> Hi Robin,
> 
> DMA writes will be translated (thereby censored) by DMA Remapping hardware,
> while MSI/MSI-X will not. Is this different for non-x86?

There is no way your DMA remapping HW can differenciate. The only
difference between a DMA write and an MSI is ... the address. So if I
can make my device DMA to the MSI address range, I've defeated your
security.

The table obfuscating in qemu is only useful as an insecure way of
"making things sort-of-work" for HW that doesnt have proper remapping
or filtering.

On pseries we don't have that problem because:

 1) Our hypervisor (which is qemu) provide the DMA address for MSIs/X
so there is no need for "magic remapping" to give the guest a value
that works.

 2) Our HW (configured by VFIO/KVM) filters which device can DMA to
what address (including which MSIs/X) thus even if the guest doesn't
use the address passed and messes around, it can only shoot itself in
the foot.

So all we need is a way to tell qemu to stop doing that filtering on
our platform. This is *one bit* of information, it's taken 3 years of
arguments and we still don't have a solution. In the meantime,
workloads *are* being hurt by significant performance degradation due
to the MSI-X table sharing a 64K page (our page size) with other MMIOs.

Yay !

Ben.

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Benjamin Herrenschmidt]

On Tue, 2017-08-15 at 09:16 +0800, Jike Song wrote:
> > Taking a step back, though, why does vfio-pci perform this check in the
> > first place? If a malicious guest already has control of a device, any
> > kind of interrupt spoofing it could do by fiddling with the MSI-X
> > message address/data it could simply do with a DMA write anyway, so the
> > security argument doesn't stand up in general (sure, not all PCIe
> > devices may be capable of arbitrary DMA, but that seems like more of a
> > tenuous security-by-obscurity angle to me).

I tried to make that point for years, thanks for re-iterating it :-)

> Hi Robin,
> 
> DMA writes will be translated (thereby censored) by DMA Remapping hardware,
> while MSI/MSI-X will not. Is this different for non-x86?

There is no way your DMA remapping HW can differenciate. The only
difference between a DMA write and an MSI is ... the address. So if I
can make my device DMA to the MSI address range, I've defeated your
security.

The table obfuscating in qemu is only useful as an insecure way of
"making things sort-of-work" for HW that doesnt have proper remapping
or filtering.

On pseries we don't have that problem because:

 1) Our hypervisor (which is qemu) provide the DMA address for MSIs/X
so there is no need for "magic remapping" to give the guest a value
that works.

 2) Our HW (configured by VFIO/KVM) filters which device can DMA to
what address (including which MSIs/X) thus even if the guest doesn't
use the address passed and messes around, it can only shoot itself in
the foot.

So all we need is a way to tell qemu to stop doing that filtering on
our platform. This is *one bit* of information, it's taken 3 years of
arguments and we still don't have a solution. In the meantime,
workloads *are* being hurt by significant performance degradation due
to the MSI-X table sharing a 64K page (our page size) with other MMIOs.

Yay !

Ben.

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Jike Song]

On 08/14/2017 09:12 PM, Robin Murphy wrote:
> On 14/08/17 10:45, Alexey Kardashevskiy wrote:
>> Folks,
>>
>> Is there anything to change besides those compiler errors and David's
>> comment in 5/5? Or the while patchset is too bad? Thanks.
> 
> While I now understand it's not the low-level thing I first thought it
> was, so my reasoning has changed, personally I don't like this approach
> any more than the previous one - it still smells of abusing external
> APIs to pass information from one part of VFIO to another (and it has
> the same conceptual problem of attributing something to interrupt
> sources that is actually a property of the interrupt target).
> 
> Taking a step back, though, why does vfio-pci perform this check in the
> first place? If a malicious guest already has control of a device, any
> kind of interrupt spoofing it could do by fiddling with the MSI-X
> message address/data it could simply do with a DMA write anyway, so the
> security argument doesn't stand up in general (sure, not all PCIe
> devices may be capable of arbitrary DMA, but that seems like more of a
> tenuous security-by-obscurity angle to me).

Hi Robin,

DMA writes will be translated (thereby censored) by DMA Remapping hardware,
while MSI/MSI-X will not. Is this different for non-x86?

--
Thanks,
Jike

> Besides, with Type1 IOMMU
> the fact that we've let a device be assigned at all means that this is
> already a non-issue (because either the hardware provides isolation or
> the user has explicitly accepted the consequences of an unsafe
> configuration) - from patch #4 that's apparently the same for SPAPR TCE,
> in which case it seems this flag doesn't even need to be propagated and
> could simply be assumed always.
> 
> On the other hand, if the check is not so much to mitigate malicious
> guests attacking the system as to prevent dumb guests breaking
> themselves (e.g. if some or all of the MSI-X capability is actually
> emulated), then allowing things to sometimes go wrong on the grounds of
> an irrelevant hardware feature doesn't seem correct :/
> 
> Robin.
> 
>> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
>>> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
>>> mmapping MSI-X table"
>>> http://www.spinics.net/lists/kvm/msg152232.html
>>>
>>> This time it is using "caps" in IOMMU groups. The main question is if PCI
>>> bus flags or IOMMU domains are still better (and which one).
>>
>>>
>>>
>>>
>>> Here is some background:
>>>
>>> Current vfio-pci implementation disallows to mmap the page
>>> containing MSI-X table in case that users can write directly
>>> to MSI-X table and generate an incorrect MSIs.
>>>
>>> However, this will cause some performance issue when there
>>> are some critical device registers in the same page as the
>>> MSI-X table. We have to handle the mmio access to these
>>> registers in QEMU emulation rather than in guest.
>>>
>>> To solve this issue, this series allows to expose MSI-X table
>>> to userspace when hardware enables the capability of interrupt
>>> remapping which can ensure that a given PCI device can only
>>> shoot the MSIs assigned for it. And we introduce a new bus_flags
>>> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
>>> for different archs.
>>>
>>>
>>> This is based on sha1
>>> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
>>> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
>>>
>>> Please comment. Thanks.
>>>
>>> Changelog:
>>>
>>> v5:
>>> * redid the whole thing via so-called IOMMU group capabilities
>>>
>>> v4:
>>> * rebased on recent upstream
>>> * got all 6 patches from v2 (v3 was missing some)
>>>
>>>
>>>
>>>
>>> Alexey Kardashevskiy (5):
>>>   iommu: Add capabilities to a group
>>>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
>>> remapping
>>>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
>>> enabled
>>>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>>>   vfio-pci: Allow to expose MSI-X table to userspace when safe
>>>
>>>  include/linux/iommu.h| 20 
>>>  include/linux/vfio.h |  1 +
>>>  arch/powerpc/kernel/iommu.c  |  1 +
>>>  drivers/iommu/amd_iommu.c|  3 +++
>>>  drivers/iommu/intel-iommu.c  |  3 +++
>>>  drivers/iommu/iommu.c| 35 +++
>>>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>>>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>>>  drivers/vfio/vfio.c  | 15 +++
>>>  9 files changed, 99 insertions(+), 4 deletions(-)
>>>
>>
>>
> 

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Jike Song]

On 08/14/2017 09:12 PM, Robin Murphy wrote:
> On 14/08/17 10:45, Alexey Kardashevskiy wrote:
>> Folks,
>>
>> Is there anything to change besides those compiler errors and David's
>> comment in 5/5? Or the while patchset is too bad? Thanks.
> 
> While I now understand it's not the low-level thing I first thought it
> was, so my reasoning has changed, personally I don't like this approach
> any more than the previous one - it still smells of abusing external
> APIs to pass information from one part of VFIO to another (and it has
> the same conceptual problem of attributing something to interrupt
> sources that is actually a property of the interrupt target).
> 
> Taking a step back, though, why does vfio-pci perform this check in the
> first place? If a malicious guest already has control of a device, any
> kind of interrupt spoofing it could do by fiddling with the MSI-X
> message address/data it could simply do with a DMA write anyway, so the
> security argument doesn't stand up in general (sure, not all PCIe
> devices may be capable of arbitrary DMA, but that seems like more of a
> tenuous security-by-obscurity angle to me).

Hi Robin,

DMA writes will be translated (thereby censored) by DMA Remapping hardware,
while MSI/MSI-X will not. Is this different for non-x86?

--
Thanks,
Jike

> Besides, with Type1 IOMMU
> the fact that we've let a device be assigned at all means that this is
> already a non-issue (because either the hardware provides isolation or
> the user has explicitly accepted the consequences of an unsafe
> configuration) - from patch #4 that's apparently the same for SPAPR TCE,
> in which case it seems this flag doesn't even need to be propagated and
> could simply be assumed always.
> 
> On the other hand, if the check is not so much to mitigate malicious
> guests attacking the system as to prevent dumb guests breaking
> themselves (e.g. if some or all of the MSI-X capability is actually
> emulated), then allowing things to sometimes go wrong on the grounds of
> an irrelevant hardware feature doesn't seem correct :/
> 
> Robin.
> 
>> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
>>> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
>>> mmapping MSI-X table"
>>> http://www.spinics.net/lists/kvm/msg152232.html
>>>
>>> This time it is using "caps" in IOMMU groups. The main question is if PCI
>>> bus flags or IOMMU domains are still better (and which one).
>>
>>>
>>>
>>>
>>> Here is some background:
>>>
>>> Current vfio-pci implementation disallows to mmap the page
>>> containing MSI-X table in case that users can write directly
>>> to MSI-X table and generate an incorrect MSIs.
>>>
>>> However, this will cause some performance issue when there
>>> are some critical device registers in the same page as the
>>> MSI-X table. We have to handle the mmio access to these
>>> registers in QEMU emulation rather than in guest.
>>>
>>> To solve this issue, this series allows to expose MSI-X table
>>> to userspace when hardware enables the capability of interrupt
>>> remapping which can ensure that a given PCI device can only
>>> shoot the MSIs assigned for it. And we introduce a new bus_flags
>>> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
>>> for different archs.
>>>
>>>
>>> This is based on sha1
>>> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
>>> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
>>>
>>> Please comment. Thanks.
>>>
>>> Changelog:
>>>
>>> v5:
>>> * redid the whole thing via so-called IOMMU group capabilities
>>>
>>> v4:
>>> * rebased on recent upstream
>>> * got all 6 patches from v2 (v3 was missing some)
>>>
>>>
>>>
>>>
>>> Alexey Kardashevskiy (5):
>>>   iommu: Add capabilities to a group
>>>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
>>> remapping
>>>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
>>> enabled
>>>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>>>   vfio-pci: Allow to expose MSI-X table to userspace when safe
>>>
>>>  include/linux/iommu.h| 20 
>>>  include/linux/vfio.h |  1 +
>>>  arch/powerpc/kernel/iommu.c  |  1 +
>>>  drivers/iommu/amd_iommu.c|  3 +++
>>>  drivers/iommu/intel-iommu.c  |  3 +++
>>>  drivers/iommu/iommu.c| 35 +++
>>>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>>>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>>>  drivers/vfio/vfio.c  | 15 +++
>>>  9 files changed, 99 insertions(+), 4 deletions(-)
>>>
>>
>>
> 

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Jike Song]

On 08/15/2017 09:33 AM, Benjamin Herrenschmidt wrote:
> On Tue, 2017-08-15 at 09:16 +0800, Jike Song wrote:
>>> Taking a step back, though, why does vfio-pci perform this check in the
>>> first place? If a malicious guest already has control of a device, any
>>> kind of interrupt spoofing it could do by fiddling with the MSI-X
>>> message address/data it could simply do with a DMA write anyway, so the
>>> security argument doesn't stand up in general (sure, not all PCIe
>>> devices may be capable of arbitrary DMA, but that seems like more of a
>>> tenuous security-by-obscurity angle to me).
> 
> I tried to make that point for years, thanks for re-iterating it :-)
> 
>> Hi Robin,
>>
>> DMA writes will be translated (thereby censored) by DMA Remapping hardware,
>> while MSI/MSI-X will not. Is this different for non-x86?
> 
> There is no way your DMA remapping HW can differenciate. The only
> difference between a DMA write and an MSI is ... the address. So if I
> can make my device DMA to the MSI address range, I've defeated your
> security.

I don't think with IRQ remapping enabled, you can make your device DMA to
MSI address, without being treated as an IRQ and remapped. If so, the IRQ
remapping hardware is simply broken :)

--
Thanks,
Jike

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Jike Song]

On 08/15/2017 09:33 AM, Benjamin Herrenschmidt wrote:
> On Tue, 2017-08-15 at 09:16 +0800, Jike Song wrote:
>>> Taking a step back, though, why does vfio-pci perform this check in the
>>> first place? If a malicious guest already has control of a device, any
>>> kind of interrupt spoofing it could do by fiddling with the MSI-X
>>> message address/data it could simply do with a DMA write anyway, so the
>>> security argument doesn't stand up in general (sure, not all PCIe
>>> devices may be capable of arbitrary DMA, but that seems like more of a
>>> tenuous security-by-obscurity angle to me).
> 
> I tried to make that point for years, thanks for re-iterating it :-)
> 
>> Hi Robin,
>>
>> DMA writes will be translated (thereby censored) by DMA Remapping hardware,
>> while MSI/MSI-X will not. Is this different for non-x86?
> 
> There is no way your DMA remapping HW can differenciate. The only
> difference between a DMA write and an MSI is ... the address. So if I
> can make my device DMA to the MSI address range, I've defeated your
> security.

I don't think with IRQ remapping enabled, you can make your device DMA to
MSI address, without being treated as an IRQ and remapped. If so, the IRQ
remapping hardware is simply broken :)

--
Thanks,
Jike

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Robin Murphy]

On 14/08/17 10:45, Alexey Kardashevskiy wrote:
> Folks,
> 
> Is there anything to change besides those compiler errors and David's
> comment in 5/5? Or the while patchset is too bad? Thanks.

While I now understand it's not the low-level thing I first thought it
was, so my reasoning has changed, personally I don't like this approach
any more than the previous one - it still smells of abusing external
APIs to pass information from one part of VFIO to another (and it has
the same conceptual problem of attributing something to interrupt
sources that is actually a property of the interrupt target).

Taking a step back, though, why does vfio-pci perform this check in the
first place? If a malicious guest already has control of a device, any
kind of interrupt spoofing it could do by fiddling with the MSI-X
message address/data it could simply do with a DMA write anyway, so the
security argument doesn't stand up in general (sure, not all PCIe
devices may be capable of arbitrary DMA, but that seems like more of a
tenuous security-by-obscurity angle to me). Besides, with Type1 IOMMU
the fact that we've let a device be assigned at all means that this is
already a non-issue (because either the hardware provides isolation or
the user has explicitly accepted the consequences of an unsafe
configuration) - from patch #4 that's apparently the same for SPAPR TCE,
in which case it seems this flag doesn't even need to be propagated and
could simply be assumed always.

On the other hand, if the check is not so much to mitigate malicious
guests attacking the system as to prevent dumb guests breaking
themselves (e.g. if some or all of the MSI-X capability is actually
emulated), then allowing things to sometimes go wrong on the grounds of
an irrelevant hardware feature doesn't seem correct :/

Robin.

> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
>> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
>> mmapping MSI-X table"
>> http://www.spinics.net/lists/kvm/msg152232.html
>>
>> This time it is using "caps" in IOMMU groups. The main question is if PCI
>> bus flags or IOMMU domains are still better (and which one).
> 
>>
>>
>>
>> Here is some background:
>>
>> Current vfio-pci implementation disallows to mmap the page
>> containing MSI-X table in case that users can write directly
>> to MSI-X table and generate an incorrect MSIs.
>>
>> However, this will cause some performance issue when there
>> are some critical device registers in the same page as the
>> MSI-X table. We have to handle the mmio access to these
>> registers in QEMU emulation rather than in guest.
>>
>> To solve this issue, this series allows to expose MSI-X table
>> to userspace when hardware enables the capability of interrupt
>> remapping which can ensure that a given PCI device can only
>> shoot the MSIs assigned for it. And we introduce a new bus_flags
>> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
>> for different archs.
>>
>>
>> This is based on sha1
>> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
>> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
>>
>> Please comment. Thanks.
>>
>> Changelog:
>>
>> v5:
>> * redid the whole thing via so-called IOMMU group capabilities
>>
>> v4:
>> * rebased on recent upstream
>> * got all 6 patches from v2 (v3 was missing some)
>>
>>
>>
>>
>> Alexey Kardashevskiy (5):
>>   iommu: Add capabilities to a group
>>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
>> remapping
>>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
>> enabled
>>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>>   vfio-pci: Allow to expose MSI-X table to userspace when safe
>>
>>  include/linux/iommu.h| 20 
>>  include/linux/vfio.h |  1 +
>>  arch/powerpc/kernel/iommu.c  |  1 +
>>  drivers/iommu/amd_iommu.c|  3 +++
>>  drivers/iommu/intel-iommu.c  |  3 +++
>>  drivers/iommu/iommu.c| 35 +++
>>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>>  drivers/vfio/vfio.c  | 15 +++
>>  9 files changed, 99 insertions(+), 4 deletions(-)
>>
> 
> 

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Robin Murphy]

On 14/08/17 10:45, Alexey Kardashevskiy wrote:
> Folks,
> 
> Is there anything to change besides those compiler errors and David's
> comment in 5/5? Or the while patchset is too bad? Thanks.

While I now understand it's not the low-level thing I first thought it
was, so my reasoning has changed, personally I don't like this approach
any more than the previous one - it still smells of abusing external
APIs to pass information from one part of VFIO to another (and it has
the same conceptual problem of attributing something to interrupt
sources that is actually a property of the interrupt target).

Taking a step back, though, why does vfio-pci perform this check in the
first place? If a malicious guest already has control of a device, any
kind of interrupt spoofing it could do by fiddling with the MSI-X
message address/data it could simply do with a DMA write anyway, so the
security argument doesn't stand up in general (sure, not all PCIe
devices may be capable of arbitrary DMA, but that seems like more of a
tenuous security-by-obscurity angle to me). Besides, with Type1 IOMMU
the fact that we've let a device be assigned at all means that this is
already a non-issue (because either the hardware provides isolation or
the user has explicitly accepted the consequences of an unsafe
configuration) - from patch #4 that's apparently the same for SPAPR TCE,
in which case it seems this flag doesn't even need to be propagated and
could simply be assumed always.

On the other hand, if the check is not so much to mitigate malicious
guests attacking the system as to prevent dumb guests breaking
themselves (e.g. if some or all of the MSI-X capability is actually
emulated), then allowing things to sometimes go wrong on the grounds of
an irrelevant hardware feature doesn't seem correct :/

Robin.

> On 07/08/17 17:25, Alexey Kardashevskiy wrote:
>> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
>> mmapping MSI-X table"
>> http://www.spinics.net/lists/kvm/msg152232.html
>>
>> This time it is using "caps" in IOMMU groups. The main question is if PCI
>> bus flags or IOMMU domains are still better (and which one).
> 
>>
>>
>>
>> Here is some background:
>>
>> Current vfio-pci implementation disallows to mmap the page
>> containing MSI-X table in case that users can write directly
>> to MSI-X table and generate an incorrect MSIs.
>>
>> However, this will cause some performance issue when there
>> are some critical device registers in the same page as the
>> MSI-X table. We have to handle the mmio access to these
>> registers in QEMU emulation rather than in guest.
>>
>> To solve this issue, this series allows to expose MSI-X table
>> to userspace when hardware enables the capability of interrupt
>> remapping which can ensure that a given PCI device can only
>> shoot the MSIs assigned for it. And we introduce a new bus_flags
>> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
>> for different archs.
>>
>>
>> This is based on sha1
>> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
>> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
>>
>> Please comment. Thanks.
>>
>> Changelog:
>>
>> v5:
>> * redid the whole thing via so-called IOMMU group capabilities
>>
>> v4:
>> * rebased on recent upstream
>> * got all 6 patches from v2 (v3 was missing some)
>>
>>
>>
>>
>> Alexey Kardashevskiy (5):
>>   iommu: Add capabilities to a group
>>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
>> remapping
>>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
>> enabled
>>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>>   vfio-pci: Allow to expose MSI-X table to userspace when safe
>>
>>  include/linux/iommu.h| 20 
>>  include/linux/vfio.h |  1 +
>>  arch/powerpc/kernel/iommu.c  |  1 +
>>  drivers/iommu/amd_iommu.c|  3 +++
>>  drivers/iommu/intel-iommu.c  |  3 +++
>>  drivers/iommu/iommu.c| 35 +++
>>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>>  drivers/vfio/vfio.c  | 15 +++
>>  9 files changed, 99 insertions(+), 4 deletions(-)
>>
> 
> 

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

Folks,

Is there anything to change besides those compiler errors and David's
comment in 5/5? Or the while patchset is too bad? Thanks.



On 07/08/17 17:25, Alexey Kardashevskiy wrote:
> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
> mmapping MSI-X table"
> http://www.spinics.net/lists/kvm/msg152232.html
> 
> This time it is using "caps" in IOMMU groups. The main question is if PCI
> bus flags or IOMMU domains are still better (and which one).

> 
> 
> 
> Here is some background:
> 
> Current vfio-pci implementation disallows to mmap the page
> containing MSI-X table in case that users can write directly
> to MSI-X table and generate an incorrect MSIs.
> 
> However, this will cause some performance issue when there
> are some critical device registers in the same page as the
> MSI-X table. We have to handle the mmio access to these
> registers in QEMU emulation rather than in guest.
> 
> To solve this issue, this series allows to expose MSI-X table
> to userspace when hardware enables the capability of interrupt
> remapping which can ensure that a given PCI device can only
> shoot the MSIs assigned for it. And we introduce a new bus_flags
> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
> for different archs.
> 
> 
> This is based on sha1
> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
> 
> Please comment. Thanks.
> 
> Changelog:
> 
> v5:
> * redid the whole thing via so-called IOMMU group capabilities
> 
> v4:
> * rebased on recent upstream
> * got all 6 patches from v2 (v3 was missing some)
> 
> 
> 
> 
> Alexey Kardashevskiy (5):
>   iommu: Add capabilities to a group
>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
> remapping
>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
> enabled
>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>   vfio-pci: Allow to expose MSI-X table to userspace when safe
> 
>  include/linux/iommu.h| 20 
>  include/linux/vfio.h |  1 +
>  arch/powerpc/kernel/iommu.c  |  1 +
>  drivers/iommu/amd_iommu.c|  3 +++
>  drivers/iommu/intel-iommu.c  |  3 +++
>  drivers/iommu/iommu.c| 35 +++
>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>  drivers/vfio/vfio.c  | 15 +++
>  9 files changed, 99 insertions(+), 4 deletions(-)
> 


-- 
Alexey

Re: [RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

Folks,

Is there anything to change besides those compiler errors and David's
comment in 5/5? Or the while patchset is too bad? Thanks.



On 07/08/17 17:25, Alexey Kardashevskiy wrote:
> This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
> mmapping MSI-X table"
> http://www.spinics.net/lists/kvm/msg152232.html
> 
> This time it is using "caps" in IOMMU groups. The main question is if PCI
> bus flags or IOMMU domains are still better (and which one).

> 
> 
> 
> Here is some background:
> 
> Current vfio-pci implementation disallows to mmap the page
> containing MSI-X table in case that users can write directly
> to MSI-X table and generate an incorrect MSIs.
> 
> However, this will cause some performance issue when there
> are some critical device registers in the same page as the
> MSI-X table. We have to handle the mmio access to these
> registers in QEMU emulation rather than in guest.
> 
> To solve this issue, this series allows to expose MSI-X table
> to userspace when hardware enables the capability of interrupt
> remapping which can ensure that a given PCI device can only
> shoot the MSIs assigned for it. And we introduce a new bus_flags
> PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
> for different archs.
> 
> 
> This is based on sha1
> 26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"
> 
> Please comment. Thanks.
> 
> Changelog:
> 
> v5:
> * redid the whole thing via so-called IOMMU group capabilities
> 
> v4:
> * rebased on recent upstream
> * got all 6 patches from v2 (v3 was missing some)
> 
> 
> 
> 
> Alexey Kardashevskiy (5):
>   iommu: Add capabilities to a group
>   iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
> remapping
>   iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
> enabled
>   powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
>   vfio-pci: Allow to expose MSI-X table to userspace when safe
> 
>  include/linux/iommu.h| 20 
>  include/linux/vfio.h |  1 +
>  arch/powerpc/kernel/iommu.c  |  1 +
>  drivers/iommu/amd_iommu.c|  3 +++
>  drivers/iommu/intel-iommu.c  |  3 +++
>  drivers/iommu/iommu.c| 35 +++
>  drivers/vfio/pci/vfio_pci.c  | 20 +---
>  drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
>  drivers/vfio/vfio.c  | 15 +++
>  9 files changed, 99 insertions(+), 4 deletions(-)
> 


-- 
Alexey

[RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
mmapping MSI-X table"
http://www.spinics.net/lists/kvm/msg152232.html

This time it is using "caps" in IOMMU groups. The main question is if PCI
bus flags or IOMMU domains are still better (and which one).



Here is some background:

Current vfio-pci implementation disallows to mmap the page
containing MSI-X table in case that users can write directly
to MSI-X table and generate an incorrect MSIs.

However, this will cause some performance issue when there
are some critical device registers in the same page as the
MSI-X table. We have to handle the mmio access to these
registers in QEMU emulation rather than in guest.

To solve this issue, this series allows to expose MSI-X table
to userspace when hardware enables the capability of interrupt
remapping which can ensure that a given PCI device can only
shoot the MSIs assigned for it. And we introduce a new bus_flags
PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
for different archs.


This is based on sha1
26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"

Please comment. Thanks.

Changelog:

v5:
* redid the whole thing via so-called IOMMU group capabilities

v4:
* rebased on recent upstream
* got all 6 patches from v2 (v3 was missing some)




Alexey Kardashevskiy (5):
  iommu: Add capabilities to a group
  iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
remapping
  iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
enabled
  powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
  vfio-pci: Allow to expose MSI-X table to userspace when safe

 include/linux/iommu.h| 20 
 include/linux/vfio.h |  1 +
 arch/powerpc/kernel/iommu.c  |  1 +
 drivers/iommu/amd_iommu.c|  3 +++
 drivers/iommu/intel-iommu.c  |  3 +++
 drivers/iommu/iommu.c| 35 +++
 drivers/vfio/pci/vfio_pci.c  | 20 +---
 drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
 drivers/vfio/vfio.c  | 15 +++
 9 files changed, 99 insertions(+), 4 deletions(-)

-- 
2.11.0

[RFC PATCH v5 0/5] vfio-pci: Add support for mmapping MSI-X table

[Alexey Kardashevskiy]

This is a followup for "[PATCH kernel v4 0/6] vfio-pci: Add support for 
mmapping MSI-X table"
http://www.spinics.net/lists/kvm/msg152232.html

This time it is using "caps" in IOMMU groups. The main question is if PCI
bus flags or IOMMU domains are still better (and which one).



Here is some background:

Current vfio-pci implementation disallows to mmap the page
containing MSI-X table in case that users can write directly
to MSI-X table and generate an incorrect MSIs.

However, this will cause some performance issue when there
are some critical device registers in the same page as the
MSI-X table. We have to handle the mmio access to these
registers in QEMU emulation rather than in guest.

To solve this issue, this series allows to expose MSI-X table
to userspace when hardware enables the capability of interrupt
remapping which can ensure that a given PCI device can only
shoot the MSIs assigned for it. And we introduce a new bus_flags
PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
for different archs.


This is based on sha1
26c5cebfdb6c "Merge branch 'parisc-4.13-4' of 
git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux"

Please comment. Thanks.

Changelog:

v5:
* redid the whole thing via so-called IOMMU group capabilities

v4:
* rebased on recent upstream
* got all 6 patches from v2 (v3 was missing some)




Alexey Kardashevskiy (5):
  iommu: Add capabilities to a group
  iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if MSI controller enables IRQ
remapping
  iommu/intel/amd: Set IOMMU_GROUP_CAP_ISOLATE_MSIX if IRQ remapping is
enabled
  powerpc/iommu: Set IOMMU_GROUP_CAP_ISOLATE_MSIX
  vfio-pci: Allow to expose MSI-X table to userspace when safe

 include/linux/iommu.h| 20 
 include/linux/vfio.h |  1 +
 arch/powerpc/kernel/iommu.c  |  1 +
 drivers/iommu/amd_iommu.c|  3 +++
 drivers/iommu/intel-iommu.c  |  3 +++
 drivers/iommu/iommu.c| 35 +++
 drivers/vfio/pci/vfio_pci.c  | 20 +---
 drivers/vfio/pci/vfio_pci_rdwr.c |  5 -
 drivers/vfio/vfio.c  | 15 +++
 9 files changed, 99 insertions(+), 4 deletions(-)

-- 
2.11.0