This series introduces a way for PCI resource allocator to force
MMIO BARs not to share PAGE_SIZE. This would make sense to VFIO
driver. Because current VFIO implementation disallows to mmap
sub-page(size < PAGE_SIZE) MMIO BARs which may share the same page
with other BARs for security reasons. Thus, we have to handle mmio
access to these BARs in QEMU emulation rather than in guest which
will cause some performance loss.
In our solution, we try to make use of the existing code path of
resource_alignment kernel parameter and add a macro to set default
alignment for it. Thus we can define this macro by default on some
archs which may easily hit the performance issue because of their
64K page.
In this series, patch 1,2,3 fixed bugs of using resource_alignment;
patch 4 tried to add a new option for resource_alignment to use
IORESOURCE_STARTALIGN to specify the alignment of PCI BARs; patch 5
adds a macro to set the default alignment of all MMIO BARs.
Changelog v4:
- Rebased against v4.8-rc1
- Drop one irrelevant patch
- Drop the patch that adding wildcard to resource_alignment to enforce
the alignment of all MMIO BARs to be at least PAGE_SIZE
- Change the format of option "noresize" of resource_alignment
- Code style improvements
Changelog v3:
- Ignore enforced alignment to fixed BARs
- Fix issue that disabling memory decoding when reassigning the alignment
- Only enable default alignment on PowerNV platform
Changelog v2:
- Ignore enforced alignment to VF BARs on pci_reassigndev_resource_alignment()
Yongji Xie (5):
PCI: Ignore enforced alignment when kernel uses existing firmware setup
PCI: Ignore enforced alignment to VF BARs
PCI: Do not disable memory decoding in pci_reassigndev_resource_alignment()
PCI: Add a new option for resource_alignment to reassign alignment
PCI: Add a macro to set default alignment for all PCI devices
Documentation/kernel-parameters.txt | 9 +++--
arch/powerpc/include/asm/pci.h | 4 ++
drivers/pci/pci.c | 71 ++++++++++++++++++++++++++---------
3 files changed, 64 insertions(+), 20 deletions(-)
--
1.7.9.5
PCI resources allocator will use firmware setup and not try to
reassign resource when PCI_PROBE_ONLY or IORESOURCE_PCI_FIXED
is set.
The enforced alignment in pci_reassigndev_resource_alignment()
should be ignored in this case. Otherwise, some PCI devices'
resources would be released here and not re-allocated.
Signed-off-by: Yongji Xie <redacted>
---
drivers/pci/pci.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
VF BARs are read-only zeroes according to SRIOV spec,
the normal way(writing BARs) of allocating resources wouldn't
be applied to VFs. The VFs' resources would be allocated
when we enable SR-IOV capability. So we should not try to
reassign alignment after we enable VFs. It's meaningless
and will release the allocated resources which leads to a bug.
Signed-off-by: Yongji Xie <redacted>
---
drivers/pci/pci.c | 9 +++++++++
1 file changed, 9 insertions(+)
We should not disable memory decoding when we reassign alignment
in pci_reassigndev_resource_alignment(). It's meaningless and
have some side effects. For example, we found it would break
this kind of P2P bridge:
0001:02:02.0 PCI bridge: PLX Technology, Inc. PEX 8718 16-Lane,
5-Port PCI Express Gen 3 (8.0 GT/s) Switch (rev aa)
And it may also potentially break the PCI devices with mmio_always_on
bit set.
Besides, disabling memory decoding is not expected in some fixup
function such as fixup_vga(). The fixup_vga() read PCI_COMMAND_MEMORY
to know whether the devices has been initialized by the firmware or
not. Disabling memory decoding would cause the one initialized by
firmware may not be set as the default VGA device when more than one
graphics adapter is present.
Signed-off-by: Yongji Xie <redacted>
---
drivers/pci/pci.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
When vfio passthroughs a PCI device of which MMIO BARs are
smaller than PAGE_SIZE, guest will not handle the mmio
accesses to the BARs which leads to mmio emulations in host.
This is because vfio will not allow to passthrough one BAR's
mmio page which may be shared with other BARs. Otherwise,
there will be a backdoor that guest can use to access BARs
of other guest.
This patch adds a macro to set default alignment for all
PCI devices. Then we could solve this issue on some platforms
which would easily hit this issue because of their 64K page
such as PowerNV platform by defining this macro as PAGE_SIZE.
Signed-off-by: Yongji Xie <redacted>
---
arch/powerpc/include/asm/pci.h | 4 ++++
drivers/pci/pci.c | 4 ++++
2 files changed, 8 insertions(+)
When using resource_alignment kernel parameter, the current
implement reassigns the alignment by changing resources' size
which can potentially break some drivers. For example, the driver
uses the size to locate some register whose length is related
to the size.
This patch adds a new option "noresize" for the parameter to
solve this problem.
Signed-off-by: Yongji Xie <redacted>
---
Documentation/kernel-parameters.txt | 9 ++++++---
drivers/pci/pci.c | 37 +++++++++++++++++++++++++----------
2 files changed, 33 insertions(+), 13 deletions(-)
@@ -3023,15 +3023,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted. window. The default value is 64 megabytes. resource_alignment= Format:- [<order of align>@][<domain>:]<bus>:<slot>.<func>[; ...]- [<order of align>@]pci:<vendor>:<device>\- [:<subvendor>:<subdevice>][; ...]+ [<order of align>@][noresize@][<domain>:]+ <bus>:<slot>.<func>[; ...]+ [<order of align>@][noresize@]pci:<vendor>:<device>+ [:<subvendor>:<subdevice>][; ...] Specifies alignment and device to reassign aligned memory resources. If <order of align> is not specified, PAGE_SIZE is used as alignment. PCI-PCI bridge can be specified, if resource windows need to be expanded.+ noresize: Don't change the resources' sizes when+ reassigning alignment. ecrc= Enable/disable PCIe ECRC (transaction layer end-to-end CRC checking). bios: Use BIOS/firmware settings. This is the
@@ -5057,7 +5067,7 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)return;/* check if specified PCI is target device to reassign */-align=pci_specified_resource_alignment(dev);+align=pci_specified_resource_alignment(dev,&resize);if(!align)return;
@@ -5080,15 +5090,22 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)}size=resource_size(r);-if(size<align){-size=align;-dev_info(&dev->dev,-"Rounding up size of resource #%d to %#llx.\n",-i,(unsignedlonglong)size);+if(resize){+if(size<align){+size=align;+dev_info(&dev->dev,+"Rounding up size of resource #%d to %#llx.\n",+i,(unsignedlonglong)size);+}+r->flags|=IORESOURCE_UNSET;+r->end=size-1;+r->start=0;+}else{+r->flags&=~IORESOURCE_SIZEALIGN;+r->flags|=IORESOURCE_STARTALIGN|IORESOURCE_UNSET;+r->start=max(align,size);+r->end=r->start+size-1;}-r->flags|=IORESOURCE_UNSET;-r->end=size-1;-r->start=0;}/* Need to disable bridge's resource window,*toenablethekerneltoreassignnewresource
This series introduces a way for PCI resource allocator to force
MMIO BARs not to share PAGE_SIZE. This would make sense to VFIO
driver. Because current VFIO implementation disallows to mmap
sub-page(size < PAGE_SIZE) MMIO BARs which may share the same page
with other BARs for security reasons. Thus, we have to handle mmio
access to these BARs in QEMU emulation rather than in guest which
will cause some performance loss.
In our solution, we try to make use of the existing code path of
resource_alignment kernel parameter and add a macro to set default
alignment for it. Thus we can define this macro by default on some
archs which may easily hit the performance issue because of their
64K page.
In this series, patch 1,2,3 fixed bugs of using resource_alignment;
patch 4 tried to add a new option for resource_alignment to use
IORESOURCE_STARTALIGN to specify the alignment of PCI BARs; patch 5
adds a macro to set the default alignment of all MMIO BARs.
Changelog v4:
- Rebased against v4.8-rc1
- Drop one irrelevant patch
- Drop the patch that adding wildcard to resource_alignment to enforce
the alignment of all MMIO BARs to be at least PAGE_SIZE
- Change the format of option "noresize" of resource_alignment
- Code style improvements
Changelog v3:
- Ignore enforced alignment to fixed BARs
- Fix issue that disabling memory decoding when reassigning the alignment
- Only enable default alignment on PowerNV platform
Changelog v2:
- Ignore enforced alignment to VF BARs on pci_reassigndev_resource_alignment()
Yongji Xie (5):
PCI: Ignore enforced alignment when kernel uses existing firmware setup
PCI: Ignore enforced alignment to VF BARs
PCI: Do not disable memory decoding in pci_reassigndev_resource_alignment()
PCI: Add a new option for resource_alignment to reassign alignment
PCI: Add a macro to set default alignment for all PCI devices
Documentation/kernel-parameters.txt | 9 +++--
arch/powerpc/include/asm/pci.h | 4 ++
drivers/pci/pci.c | 71 ++++++++++++++++++++++++++---------
3 files changed, 64 insertions(+), 20 deletions(-)
On Fri, Aug 12, 2016 at 01:42:24PM +0800, Yongji Xie wrote:
We should not disable memory decoding when we reassign alignment
in pci_reassigndev_resource_alignment(). It's meaningless and
have some side effects. For example, we found it would break
this kind of P2P bridge:
0001:02:02.0 PCI bridge: PLX Technology, Inc. PEX 8718 16-Lane,
5-Port PCI Express Gen 3 (8.0 GT/s) Switch (rev aa)
I doubt that turning memory decode off breaks this bridge. I can
believe that it could cause a problem, but I doubt it would be
specific to this bridge.
I also don't think it's meaningless. After your patch, we throw away
our knowledge of what the BAR contains when we set "r->start = 0".
But if you leave memory decoding enabled, the device will still
respond at whatever address the BAR contains. That seems like a
problem.
quoted hunk
And it may also potentially break the PCI devices with mmio_always_on
bit set.
Besides, disabling memory decoding is not expected in some fixup
function such as fixup_vga(). The fixup_vga() read PCI_COMMAND_MEMORY
to know whether the devices has been initialized by the firmware or
not. Disabling memory decoding would cause the one initialized by
firmware may not be set as the default VGA device when more than one
graphics adapter is present.
Signed-off-by: Yongji Xie <redacted>
---
drivers/pci/pci.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 12, 2016 at 01:42:26PM +0800, Yongji Xie wrote:
When vfio passthroughs a PCI device of which MMIO BARs are
smaller than PAGE_SIZE, guest will not handle the mmio
accesses to the BARs which leads to mmio emulations in host.
This is because vfio will not allow to passthrough one BAR's
mmio page which may be shared with other BARs. Otherwise,
there will be a backdoor that guest can use to access BARs
of other guest.
This patch adds a macro to set default alignment for all
PCI devices. Then we could solve this issue on some platforms
which would easily hit this issue because of their 64K page
such as PowerNV platform by defining this macro as PAGE_SIZE.
Just to clarify, I think the issue happens on any arch, whenever
device BARs are smaller than PAGE_SIZE. This is obviously more
*likely* when PAGE_SIZE is large, but could still happen even with
4K pages.
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 12, 2016 at 01:42:26PM +0800, Yongji Xie wrote:
quoted
When vfio passthroughs a PCI device of which MMIO BARs are
smaller than PAGE_SIZE, guest will not handle the mmio
accesses to the BARs which leads to mmio emulations in host.
This is because vfio will not allow to passthrough one BAR's
mmio page which may be shared with other BARs. Otherwise,
there will be a backdoor that guest can use to access BARs
of other guest.
This patch adds a macro to set default alignment for all
PCI devices. Then we could solve this issue on some platforms
which would easily hit this issue because of their 64K page
such as PowerNV platform by defining this macro as PAGE_SIZE.
Just to clarify, I think the issue happens on any arch, whenever
device BARs are smaller than PAGE_SIZE. This is obviously more
*likely* when PAGE_SIZE is large, but could still happen even with
4K pages.
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 12, 2016 at 01:42:24PM +0800, Yongji Xie wrote:
quoted
We should not disable memory decoding when we reassign alignment
in pci_reassigndev_resource_alignment(). It's meaningless and
have some side effects. For example, we found it would break
this kind of P2P bridge:
0001:02:02.0 PCI bridge: PLX Technology, Inc. PEX 8718 16-Lane,
5-Port PCI Express Gen 3 (8.0 GT/s) Switch (rev aa)
I doubt that turning memory decode off breaks this bridge. I can
believe that it could cause a problem, but I doubt it would be
specific to this bridge.
I found that disabling memory decoding would not break
this kind of bridge.
However, it would cause some problems if we have a
VGA device using "vgaarb" driver behind the bridge.
The driver didn't call something like pci_enable_device()
when it is loaded or initialized. So when the driver issued
memory access to the VGA device, the access cannot be
supported by the bridge because its memory decoding is
still disabled.
Maybe we should drop this patch and try to fix the driver.
Thanks,
Yongji
I also don't think it's meaningless. After your patch, we throw away
our knowledge of what the BAR contains when we set "r->start = 0".
But if you leave memory decoding enabled, the device will still
respond at whatever address the BAR contains. That seems like a
problem.
quoted
And it may also potentially break the PCI devices with mmio_always_on
bit set.
Besides, disabling memory decoding is not expected in some fixup
function such as fixup_vga(). The fixup_vga() read PCI_COMMAND_MEMORY
to know whether the devices has been initialized by the firmware or
not. Disabling memory decoding would cause the one initialized by
firmware may not be set as the default VGA device when more than one
graphics adapter is present.
Signed-off-by: Yongji Xie <redacted>
---
drivers/pci/pci.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html