Honor PCI prefetchable attributes for a virtual machine on ARM64
Subsystems (4)
cover Honor PCI prefetchable attributes for a virtual machine on ARM64
Problem statement: Virtual machine crashes when NVIDIA GPU driver access a prefetchable BAR space due to the unaligned reads/writes for pass-through devices. The same binary works fine as expected in the host kernel. Only one BAR has control & status registers (CSR) and other PCI BARs are marked as prefetchable. NVIDIA GPU driver uses the write-combine feature for mapping the prefetchable BARs to improve performance. This problem applies to all other drivers which want to enable WC.
Solution: Honor PCI prefetchable attributes for the guest operating systems.
Proposal: ARM64-KVM uses VMA struct for the needed information e.g. region physical address, size, and memory-type (struct page backed mapping or anonymous memory) for setting up a stage-2 page table. Right now memory region either can be mapped as DEVICE (strongly ordered) or NORMAL (write-back cache) depends on the flag VM_PFNMAP in VMA. VFIO-PCI will keep the prefetchable (write-combine) information in vma->vm_page_prot similar to other fields, and KVM will prepare stage-2 entries based on the memory-type attribute that was set in VMA.
Shanker Donthineni (2):
vfio/pci: keep the prefetchable attribute of a BAR region in VMA
KVM: arm64: Add write-combine support for stage-2 entries
arch/arm64/include/asm/kvm_mmu.h | 3 ++- arch/arm64/include/asm/kvm_pgtable.h | 2 ++ arch/arm64/include/asm/memory.h | 4 +++- arch/arm64/kvm/hyp/pgtable.c | 9 +++++++-- arch/arm64/kvm/mmu.c | 22 +++++++++++++++++++--- arch/arm64/kvm/vgic/vgic-v2.c | 2 +- drivers/vfio/pci/vfio_pci.c | 6 +++++- 7 files changed, 39 insertions(+), 9 deletions(-)
Signature · 1 line
2.17.1
RFC 1/2 vfio/pci: keep the prefetchable attribute of a BAR region in VMA
For pass-through device assignment, the ARM64 KVM hypervisor retrieves the memory region properties physical address, size, and whether a region backed with struct page or not from VMA. The prefetchable attribute of a BAR region isn't visible to KVM to make an optimal decision for stage2 attributes.
This patch updates vma->vm_page_prot and maps with write-combine attribute if the associated BAR is prefetchable. For ARM64 pgprot_writecombine() is mapped to memory-type MT_NORMAL_NC which has no side effects on reads and multiple writes can be combined.
--- drivers/vfio/pci/vfio_pci.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/vfio/pci/vfio_pci.c +5 -1
@@ -1703,7 +1703,11 @@ static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma) } vma->vm_private_data = vdev; - vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); +if (IS_ENABLED (CONFIG_ARM64 ) && + (pci_resource_flags (pdev, index) &IORESOURCE_PREFETCH )) + vma->vm_page_prot =pgprot_writecombine (vma->vm_page_prot); +else + vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); vma->vm_pgoff = (pci_resource_start (pdev, index) >>PAGE_SHIFT ) + pgoff;/*
Signature · 1 line
2.17.1
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 21 lines · On Thu, 29 Apr 2021 11:29:05 -0500 Shanker Donthineni <[email protected]>…
For pass-through device assignment, the ARM64 KVM hypervisor retrieves the memory region properties physical address, size, and whether a region backed with struct page or not from VMA. The prefetchable attribute of a BAR region isn't visible to KVM to make an optimal decision for stage2 attributes.
This patch updates
vma->vm_page_protand maps with write-combine attribute if the associated BAR is prefetchable. For ARM64pgprot_writecombine()is mapped to memory-typeMT_NORMAL_NCwhich has no side effects on reads and multiple writes can be combined.Signed-off-by Shanker Donthineni <[email protected]>--- drivers/vfio/pci/vfio_pci.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/vfio/pci/vfio_pci.c +0
-0
drivers/vfio/pci/vfio_pci.c static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma) · line 1710
Quoted 9 lines
drivers/vfio/pci/vfio_pci.c +5
-1@@ -1703,7 +1703,11 @@ static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma) } vma->vm_private_data = vdev; - vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); +if (IS_ENABLED (CONFIG_ARM64 ) && + (pci_resource_flags (pdev, index) &IORESOURCE_PREFETCH )) + vma->vm_page_prot =pgprot_writecombine (vma->vm_page_prot); +else + vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot);
If this were a valid thing to do, it should be done for all architectures, not just ARM64. However, a prefetchable range only necessarily allows merged writes, which seems like a subset of the semantics implied by a WC attribute, therefore this doesn't seem universally valid.
I'm also a bit confused by your problem statement that indicates that without WC you're seeing unaligned accesses, does this suggest that your driver is actually relying on WC semantics to perform merging to achieve alignment? That seems rather like a driver bug, I'd expect UC vs WC is largely a difference in performance, not a means to enforce proper driver access patterns. Per the PCI spec, the bridge itself can merge writes to prefetchable areas, presumably regardless of this processor attribute, perhaps that's the feature your driver is relying on that might be missing here. Thanks,
Alex
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Thanks Alex for quick reply.
Quoted 6 lines · Alex Williamson
If this were a valid thing to do, it should be done for all architectures, not just ARM64. However, a prefetchable range only necessarily allows merged writes, which seems like a subset of the semantics implied by a WC attribute, therefore this doesn't seem universally valid.
Quoted 9 lines
I'm also a bit confused by your problem statement that indicates that without WC you're seeing unaligned accesses, does this suggest that your driver is actually relying on WC semantics to perform merging to achieve alignment? That seems rather like a driver bug, I'd expect UC vs WC is largely a difference in performance, not a means to enforce proper driver access patterns. Per the PCI spec, the bridge itself can merge writes to prefetchable areas, presumably regardless of this processor attribute, perhaps that's the feature your driver is relying on that might be missing here. Thanks,
The driver uses WC semantics, It's mapping PCI prefetchable BARS using ioremap_wc(). We don't see any issue for x86 architecture, driver works fine in the host and guest kernel. The same driver works on ARM64 kernel but crashes inside VM. GPU driver uses the architecture agnostic function ioremap_wc() like other drivers. This limitation applies to all the drivers if they use WC memory and follow ARM64 NORMAL-NC access rules.
On ARM64, ioremap_wc() is mapped to non-cacheable memory-type, no side effects on reads and unaligned accesses are allowed as per ARM-ARM architecture. The driver behavior is different in host vs guest on ARM64.
ARM CPU generating alignment faults before transaction reaches the PCI-RC/switch/end-point-device.
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition of ioremap_wc() function doesn't match the host kernel on ARM64
Quoted 2 lines
Alex
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 18 lines · On Thu, 29 Apr 2021 14:14:50 -0500 Shanker R Donthineni…
Thanks Alex for quick reply.
Quoted 6 lines · Alex Williamson
If this were a valid thing to do, it should be done for all architectures, not just ARM64. However, a prefetchable range only necessarily allows merged writes, which seems like a subset of the semantics implied by a WC attribute, therefore this doesn't seem universally valid.
Quoted 9 lines
I'm also a bit confused by your problem statement that indicates that without WC you're seeing unaligned accesses, does this suggest that your driver is actually relying on WC semantics to perform merging to achieve alignment? That seems rather like a driver bug, I'd expect UC vs WC is largely a difference in performance, not a means to enforce proper driver access patterns. Per the PCI spec, the bridge itself can merge writes to prefetchable areas, presumably regardless of this processor attribute, perhaps that's the feature your driver is relying on that might be missing here. Thanks,
Quoted 7 lines
The driver uses WC semantics, It's mapping PCI prefetchable BARS using ioremap_wc(). We don't see any issue for x86 architecture, driver works fine in the host and guest kernel. The same driver works on ARM64 kernel but crashes inside VM. GPU driver uses the architecture agnostic function
ioremap_wc()like other drivers. This limitation applies to all the drivers if they use WC memory and follow ARM64 NORMAL-NC access rules.
x86 KVM works for other reasons, KVM will trust the vCPU attributes for the memory range rather than relying only on the host mapping.
Quoted 4 lines
On ARM64,
ioremap_wc()is mapped to non-cacheable memory-type, no side effects on reads and unaligned accesses are allowed as per ARM-ARM architecture. The driver behavior is different in host vs guest on ARM64.
Per the PCI spec, prefetchable memory only necessarily allows the bridge to merge writes. I believe this is only a subset of what WC mappings allow, therefore I expect this is incompatible with drivers that do not use WC mappings.
Quoted 2 lines
ARM CPU generating alignment faults before transaction reaches the PCI-RC/switch/end-point-device.
If an alignment fault is fixed by configuring a WC mapping, doesn't that suggest that the driver performed an unaligned access itself and is relying on write combining by the processor to correct that error? That's wrong. Fix the driver or please offer another explanation of how the WC mapping resolves this. I suspect you could enable tracing in QEMU, disable MMIO mmaps on the vfio-pci device and find the invalid access.
Quoted 4 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the host kernel on ARM64
Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thanks,
Alex
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
RE: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Alex,
Quoted 15 lines
From: Alex Williamson <[email protected]>
Subject: Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMAQuoted 9 lines · On Thu, 29 Apr 2021 14:14:50 -0500 Shanker R Donthineni…
Thanks Alex for quick reply.
Quoted 6 lines · Alex Williamson
If this were a valid thing to do, it should be done for all architectures, not just ARM64. However, a prefetchable range only necessarily allows merged writes, which seems like a subset of the semantics implied by a WC attribute, therefore this doesn't seem universally valid.
I didn't get your exact concern. If we removed the check for ARM arch and simply stored that this is a prefetchable region in VMA, then each arch KVM port could decide which PTE mappings are OK for prefetchable BAR. KVM doesn't want to go through PCIe enumeration, and would rather have the properties stored in VMA.
Beyond that, on arm64 specifically there is no WC Memtype, but we use Normal Non Cacheable mapping for ioremap_wc which can be prefetched and can be write combined. What semantics break for a device if its prefetchable BAR is marked as Normal Noncacheable on arm64?
We need a way for write combining to work in a KVM-ARM guest, as it is an important usecase for GPUs and NICs and also NVMe CMB IIRC. So *some* way is needed of letting KVM know to map as write combine (Normal NC) at stage2. Do you have a better solution in mind?
Quoted 50 lines
Quoted 9 lines
Quoted 9 lines
I'm also a bit confused by your problem statement that indicates that without WC you're seeing unaligned accesses, does this suggest that your driver is actually relying on WC semantics to perform merging to achieve alignment? That seems rather like a driver bug, I'd expect UC vs WC is largely a difference in performance, not a means to enforce proper driver access patterns. Per the PCI spec, the bridge itself can merge writes to prefetchable areas, presumably regardless of this processor attribute, perhaps that's the feature your driver is relying on that might be missing here. Thanks,
Quoted 7 lines
The driver uses WC semantics, It's mapping PCI prefetchable BARS using
ioremap_wc(). We don't see any issue for x86 architecture, driver works fine in the host and guest kernel. The same driver works on ARM64 kernel but crashes inside VM. GPU driver uses the architecture agnostic functionioremap_wc()like other drivers. This limitation applies to all the drivers if they use WC memory and follow ARM64 NORMAL-NC access rules.x86 KVM works for other reasons, KVM will trust the vCPU attributes for the memory range rather than relying only on the host mapping.
Quoted 1 line
On ARM64,
ioremap_wc()is mapped to non-cacheable memory-type, noside
Quoted 3 lines
effects on reads and unaligned accesses are allowed as per ARM-ARM architecture. The driver behavior is different in host vs guest on ARM64.
Per the PCI spec, prefetchable memory only necessarily allows the bridge to merge writes. I believe this is only a subset of what WC mappings allow, therefore I expect this is incompatible with drivers that do not use WC mappings.
Quoted 2 lines
ARM CPU generating alignment faults before transaction reaches the PCI-RC/switch/end-point-device.
If an alignment fault is fixed by configuring a WC mapping, doesn't that suggest that the driver performed an unaligned access itself and is relying on write combining by the processor to correct that error?
That's wrong. Fix the driver or please offer another explanation of how the WC mapping resolves this. I suspect you could enable tracing in QEMU, disable MMIO mmaps on the vfio-pci device and find the invalid access.Quoted 4 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the host
kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thanks,
Alex
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
In reply to Alex WilliamsonHi Alex
Quoted 8 lines · Alex Williamson
If an alignment fault is fixed by configuring a WC mapping, doesn't that suggest that the driver performed an unaligned access itself and is relying on write combining by the processor to correct that error? That's wrong. Fix the driver or please offer another explanation of how the WC mapping resolves this. I suspect you could enable tracing in QEMU, disable MMIO mmaps on the vfio-pci device and find the invalid access.
Quoted 6 lines
Quoted 4 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the host
kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thank
We’ve already instrumented the driver code and found the code path for the unaligned accesses. We’ll fix this issue if it’s not following WC semantics.
Fixing the performance concern will be under KVM stage-2 page-table control. We're looking for a guidance/solution for updating stage-2 PTE based on PCI-BAR attribute.
Marc Zyngier
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Shanker,
Quoted 19 lines · Shanker R Donthineni
Hi Alex
Quoted 8 lines · Alex Williamson
If an alignment fault is fixed by configuring a WC mapping, doesn't that suggest that the driver performed an unaligned access itself and is relying on write combining by the processor to correct that error? That's wrong. Fix the driver or please offer another explanation of how the WC mapping resolves this. I suspect you could enable tracing in QEMU, disable MMIO mmaps on the vfio-pci device and find the invalid access.
Quoted 6 lines
Quoted 4 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the host
kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thank
Quoted 7 lines
We’ve already instrumented the driver code and found the code path for the unaligned accesses. We’ll fix this issue if it’s not following WC semantics.
Fixing the performance concern will be under KVM stage-2 page-table control. We're looking for a guidance/solution for updating stage-2 PTE based on PCI-BAR attribute.
Before we start discussing the *how*, I'd like to clearly understand what *arm64* memory attributes you are relying on. We already have established that the unaligned access was a bug, which was the biggest argument in favour of NORMAL_NC. What are the other requirements?
Thanks,
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Marc,
On 4/30/21 6:47 AM, Marc Zyngi
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the host
kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thank
We’ve already instrumented the driver code and found the code path for the unaligned accesses. We’ll fix this issue if it’s not following WC semantics.
Quoted 8 lines
Quoted 3 lines
Fixing the performance concern will be under KVM stage-2 page-table control. We're looking for a guidance/solution for updating stage-2 PTE based on PCI-BAR attribute.
Before we start discussing the *how*, I'd like to clearly understand what *arm64* memory attributes you are relying on. We already have established that the unaligned access was a bug, which was the biggest argument in favour of
NORMAL_NC. What are the other requirements?
We are relying on NORMAL_NC mapping for PCI prefetchable BARs instead of DEVICE_nGnRE in baremetal and VM. No other requirement other than this.
-Shanker
Marc Zyngier
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 18 lines · Shanker R Donthineni
Hi Marc,
On 4/30/21 6:47 AM, Marc Zyngi
Quoted 13 lines
Quoted 13 lines
Quoted 6 lines
Quoted 4 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the host
kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thank
We’ve already instrumented the driver code and found the code path for the unaligned accesses. We’ll fix this issue if it’s not following WC semantics.
Fixing the performance concern will be under KVM stage-2 page-table control. We're looking for a guidance/solution for updating stage-2 PTE based on PCI-BAR attribute.
Quoted 8 lines
Quoted 5 lines
Before we start discussing the *how*, I'd like to clearly understand what *arm64* memory attributes you are relying on. We already have established that the unaligned access was a bug, which was the biggest argument in favour of
NORMAL_NC. What are the other requirements?We are relying on
NORMAL_NCmapping for PCI prefetchable BARs instead ofDEVICE_nGnREin baremetal and VM. No other requirement other than this.
That wasn't really my question. What properties do you require for this assigned MMIO window outside of unaligned accesses? Or to put it another way, what goes wrong with any of the other device types?
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
In reply to Marc ZyngierHi Marc,
Quoted 11 lines · Marc Zyngier
Quoted 11 lines
Quoted 7 lines
Quoted 5 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the host
kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thank
We’ve already instrumented the driver code and found the code path for the unaligned accesses. We’ll fix this issue if it’s not following WC semantics.
Quoted 7 lines
Quoted 3 lines
Fixing the performance concern will be under KVM stage-2 page-table control. We're looking for a guidance/solution for updating stage-2 PTE based on PCI-BAR attribute.
Before we start discussing the *how*, I'd like to clearly understand what *arm64* memory attributes you are relying on. We already have established that the unaligned access was a bug, which was the biggest argument in favour of
NORMAL_NC. What are the other requirements?
Sorry, my earlier response was not complete...
ARMv8 architecture has two features Gathering and Reorder transactions, very important from a performance point of view. Small inline packets for NIC cards and accesses to GPU's frame buffer are CPU-bound operations. We want to take advantages of GRE features to achieve higher performance.
Both these features are disabled for prefetchable BARs in VM because memory-type MT_DEVICE_nGnRE enforced in stage-2.
Quoted 6 lines
Thanks,
M.--
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 24 lines · Shanker R Donthineni
Hi Marc,
Quoted 11 lines · Marc Zyngier
Quoted 11 lines
Quoted 7 lines
Quoted 5 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the host
kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thank
We’ve already instrumented the driver code and found the code path for the unaligned accesses. We’ll fix this issue if it’s not following WC semantics.
Quoted 7 lines
Quoted 3 lines
Fixing the performance concern will be under KVM stage-2 page-table control. We're looking for a guidance/solution for updating stage-2 PTE based on PCI-BAR attribute.
Before we start discussing the *how*, I'd like to clearly understand what *arm64* memory attributes you are relying on. We already have established that the unaligned access was a bug, which was the biggest argument in favour of
NORMAL_NC. What are the other requirements?Sorry, my earlier response was not complete...
Quoted 8 lines
ARMv8 architecture has two features Gathering and Reorder transactions, very important from a performance point of view. Small inline packets for NIC cards and accesses to GPU's frame buffer are CPU-bound operations. We want to take advantages of GRE features to achieve higher performance.
Both these features are disabled for prefetchable BARs in VM because memory-type
MT_DEVICE_nGnREenforced in stage-2.
Right, so Normal_NC is a red herring, and it is Device_GRE that you really are after, right?
Now, I'm not convinced that we can do that directly from vfio in a device-agnostic manner. It is userspace that places the device in the guest's memory, and I have the ugly feeling that userspace needs to be in control of memory attributes.
Otherwise, we change the behaviour for all existing devices that have prefetchable BARs, and I don't think that's an acceptable move (userspace ABI change).
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
RE: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Marc,
Quoted 36 lines
-----Original Message-----
From: Marc Zyngier <[email protected]>
Sent: Friday, April 30, 2021 10:31 AMQuoted 22 lines · Shanker R Donthineni
Hi Marc,
Quoted 18 lines · Marc Zyngier
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the
host kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thank
Quoted 7 lines
We’ve already instrumented the driver code and found the code path for the unaligned accesses. We’ll fix this issue if it’s not following WC semantics.
Fixing the performance concern will be under KVM stage-2 page-table control. We're looking for a guidance/solution for updating stage-2 PTE based on PCI-BAR attribute.
Before we start discussing the *how*, I'd like to clearly understand what *arm64* memory attributes you are relying on. We already have established that the unaligned access was a bug, which was the biggest argument in favour of
NORMAL_NC. What are the otherrequirements?
Quoted 8 lines
Sorry, my earlier response was not complete...
ARMv8 architecture has two features Gathering and Reorder transactions, very important from a performance point of view. Small inline packets for NIC cards and accesses to GPU's frame buffer are CPU-bound operations. We want to take advantages of GRE features to achieve higher performance.
Quoted 6 lines
Quoted 2 lines
Both these features are disabled for prefetchable BARs in VM because memory-type
MT_DEVICE_nGnREenforced in stage-2.Right, so
Normal_NCis a red herring, and it isDevice_GREthat you really are after, right?
I think Device GRE has some practical problems.
1. A lot of userspace code which is used to getting write combined mappings to GPU memory from kernel drivers does memcpy/memset on it which
can insert ldp/stp which can crash on Device Memory Type. From a quick search I didn't find a memcpy_io or memset_io in glibc. Perhaps there are some other functions available, but a lot of userspace applications that work on x86 and ARM baremetal won't work on ARM VMs without such changes. Changes to all of userspace may not always be practical, specially if linking to binaries
2. Sometimes even if application is not using memset/memcpy directly, gcc may insert a builtin memcpy/memset.
3. Recompiling all applications with gcc -m strict-align has performance issues. In our experiments that resulted in an increase in code size, and also 3-5% performance decrease reliably.
Also, it is not always practical to recompile all of userspace, depending on who owns the code/linked binaries etc.
From KVM-ARM point of view, what is it about Normal NC at stage 2 for Prefetchable BAR (however KVM gets the hint, whether from userspace or VMA) that is undesirable vs Device GRE? I couldn't think of a difference to devices whether the combining or prefetching or reordering happened because of one or the other.
Quoted 13 lines
Now, I'm not convinced that we can do that directly from vfio in a device- agnostic manner. It is userspace that places the device in the guest's memory, and I have the ugly feeling that userspace needs to be in control of memory attributes.
Otherwise, we change the behaviour for all existing devices that have prefetchable BARs, and I don't think that's an acceptable move (userspace ABI change).
M.--
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Vikram,
Quoted 45 lines · Vikram Sethi
Hi Marc,
Quoted 36 lines
-----Original Message-----
From: Marc Zyngier <[email protected]>
Sent: Friday, April 30, 2021 10:31 AMQuoted 22 lines · Shanker R Donthineni
Hi Marc,
Quoted 18 lines · Marc Zyngier
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
We've two concerns here:
- Performance impacts for pass-through devices.
- The definition ofioremap_wc()function doesn't match the
host kernel on ARM64Performance I can understand, but I think you're also using it to mask a driver bug which should be resolved first. Thank
Quoted 7 lines
We’ve already instrumented the driver code and found the code path for the unaligned accesses. We’ll fix this issue if it’s not following WC semantics.
Fixing the performance concern will be under KVM stage-2 page-table control. We're looking for a guidance/solution for updating stage-2 PTE based on PCI-BAR attribute.
Before we start discussing the *how*, I'd like to clearly understand what *arm64* memory attributes you are relying on. We already have established that the unaligned access was a bug, which was the biggest argument in favour of
NORMAL_NC. What are the otherrequirements?
Quoted 8 lines
Sorry, my earlier response was not complete...
ARMv8 architecture has two features Gathering and Reorder transactions, very important from a performance point of view. Small inline packets for NIC cards and accesses to GPU's frame buffer are CPU-bound operations. We want to take advantages of GRE features to achieve higher performance.
Quoted 6 lines
Quoted 2 lines
Both these features are disabled for prefetchable BARs in VM because memory-type
MT_DEVICE_nGnREenforced in stage-2.Right, so
Normal_NCis a red herring, and it isDevice_GREthat you really are after, right?
Quoted 9 lines
I think Device GRE has some practical problems.
1. A lot of userspace code which is used to getting write combined mappings to GPU memory from kernel drivers does memcpy/memset on it which can insert ldp/stp which can crash on Device Memory Type. From a quick search I didn't find amemcpy_ioormemset_ioin glibc. Perhaps there are some other functions available, but a lot of userspace applications that work on x86 and ARM baremetal won't work on ARM VMs without such changes. Changes to all of userspace may not always be practical, specially if linking to binaries
This seems to go against what Alex was hinting at earlier, which is that unaligned accesses were not expected on prefetchable regions, and Shanker latter confirming that it was an actual bug. Where do we stand here?
Quoted 10 lines
2. Sometimes even if application is not using memset/memcpy directly, gcc may insert a builtin memcpy/memset.
3. Recompiling all applications with gcc -m strict-align has performance issues. In our experiments that resulted in an increase in code size, and also 3-5% performance decrease reliably. Also, it is not always practical to recompile all of userspace, depending on who owns the code/linked binaries etc.
Quoted 5 lines
From KVM-ARM point of view, what is it about Normal NC at stage 2 for Prefetchable BAR (however KVM gets the hint, whether from userspace or VMA) that is undesirable vs Device GRE? I couldn't think of a difference to devices whether the combining or prefetching or reordering happened because of one or the other.
The problem I see is that we have VM and userspace being written in terms of Write-Combine, which is:
- loosely defined even on x86
- subject to interpretations in the way it maps to PCI
- has no direct equivalent in the ARMv8 collection of memory attributes (and Normal_NC comes with speculation capabilities which strikes me as extremely undesirable on arbitrary devices)
How do we translate this into something consistent? I'd like to see an actual description of what we *really* expect from WC on prefetchable PCI regions, turn that into a documented definition agreed across architectures, and then we can look at implementing it with one memory type or another on arm64.
Because once we expose that memory type at S2 for KVM guests, it becomes ABI and there is no turning back. So I want to get it right once and for all.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Marc,
Quoted 9 lines · Marc Zyngier
Quoted 9 lines
I think Device GRE has some practical problems.
1. A lot of userspace code which is used to getting write combined mappings to GPU memory from kernel drivers does memcpy/memset on it which can insert ldp/stp which can crash on Device Memory Type. From a quick search I didn't find amemcpy_ioormemset_ioin glibc. Perhaps there are some other functions available, but a lot of userspace applications that work on x86 and ARM baremetal won't work on ARM VMs without such changes. Changes to all of userspace may not always be practical, specially if linking to binaries
Quoted 5 lines
This seems to go against what Alex was hinting at earlier, which is that unaligned accesses were not expected on prefetchable regions, and Shanker latter confirming that it was an actual bug. Where do we stand here?
We agreed to call it a driver bug if it's not following Linux write-combining API ioremap_wc() semantics. So far I didn't find whether unaligned accesses allowed or not for WC regions explicitly in Linux documentation. Page faults due to driver unaligned accesses in kernel space will be under driver control, we'll fix it.
Driver uses the architecture agnostic functions that are available in the Linux kernel and expecting the same behavior in VM vs Baremetal. We would like to keep the driver implementation is architecture-independent as much as possible and support VM unaware. For ARM64, VM's ioremap_wc() definition doesn't match baremetal.
We don't have any control over the userspace applications/drivers/libraries as Vikram saying. Another example GCC memset() function uses 'DC ZVA' which triggers an alignment fault if the actual memory type is device_xxx.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Marc Zyngier
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 18 lines · Shanker R Donthineni
Hi Marc,
Quoted 9 lines · Marc Zyngier
Quoted 9 lines
I think Device GRE has some practical problems.
1. A lot of userspace code which is used to getting write combined mappings to GPU memory from kernel drivers does memcpy/memset on it which can insert ldp/stp which can crash on Device Memory Type. From a quick search I didn't find amemcpy_ioormemset_ioin glibc. Perhaps there are some other functions available, but a lot of userspace applications that work on x86 and ARM baremetal won't work on ARM VMs without such changes. Changes to all of userspace may not always be practical, specially if linking to binariesQuoted 5 lines
This seems to go against what Alex was hinting at earlier, which is that unaligned accesses were not expected on prefetchable regions, and Shanker latter confirming that it was an actual bug. Where do we stand here?
Quoted 4 lines
We agreed to call it a driver bug if it's not following Linux write-combining API
ioremap_wc()semantics. So far I didn't find whether unaligned accesses allowed or not for WC regions explicitly in Linux documentation.
And that's exactly the kind of problem I want clarification on before we add *anything* to KVM. Proper, unambiguous definition of what WC is on the CPU side, and how it maps onto PCI. Without such a definition, we're just driving blind.
Quoted 9 lines
Page faults due to driver unaligned accesses
in kernel space will be under driver control, we'll fix it.Driver uses the architecture agnostic functions that are available in the Linux kernel and expecting the same behavior in VM vs Baremetal. We would like to keep the driver implementation is architecture-independent as much as possible and support VM unaware. For ARM64, VM's
ioremap_wc()definition doesn't match baremetal.
You are mixing two things: what Linux/arm64 gives to kernel drivers, and what KVM, as an implementation of the ARMv8 architecture, gives to virtual machines. There is zero reason for the two to match if there is no definition of what we need to provide.
Quoted 4 lines
We don't have any control over the userspace applications/drivers/libraries as Vikram saying. Another example GCC
memset()function uses 'DC ZVA' which triggers an alignment fault if the actual memory type isdevice_xxx.
Again, you're talking about an application, and I'm talking about how to map a nebulous concept that originated on a foreign architecture onto something that is entirely different. So please drop the "that's how my SW works", and instead give me a good definition of what WC means in architectural terms.
Thanks
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Marc,
Quoted 4 lines · Marc Zyngier
You are mixing two things: what Linux/arm64 gives to kernel drivers, and what KVM, as an implementation of the ARMv8 architecture, gives to virtual machines. There is zero reason for the two to match if there is no definition of what we need to provide.
Suggestion here is memory-types PROT_NORMAL_NC and PROT_DEVICE_nGnRE are applicable to braemetal only and don't use for device memory inside VM.
RE: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
In reply to Marc ZyngierHi Marc,
Quoted 7 lines
From: Marc Zyngier <[email protected]>
Hi Vikram,The problem I see is that we have VM and userspace being written in terms of Write-Combine, which is:
Quoted 7 lines
- loosely defined even on x86
- subject to interpretations in the way it maps to PCI
- has no direct equivalent in the ARMv8 collection of memory attributes (and
Normal_NCcomes with speculation capabilities which strikes me as extremely undesirable on arbitrary devices)
If speculation with Normal NC to prefetchable BARs in devices was a problem, those devices would already be broken in baremetal with ioremap_wc on arm64, and we would need quirks there to not do Normal NC for them but Device GRE, and if such a quirk was needed on baremetal, it could be picked up by vfio/KVM as well. But we haven't seen any broken devices doing wc on baremetal on ARM64, have we? I know we have tested NICs write combining on arm64 in baremetal, as well as GPU and NVMe CMB without issues.
Further, I don't see why speculation to non cacheble would be an issue if prefetch without side effects is allowed by the device, which is what a prefetchable BAR is. If it is an issue for a device I would consider that a bug already needing a quirk in Baremetal/host kernel already.
From PCI spec " A prefetchable address range may have write side effects, but it may not have read side effects."
Quoted 10 lines
How do we translate this into something consistent? I'd like to see an actual description of what we *really* expect from WC on prefetchable PCI regions, turn that into a documented definition agreed across architectures, and then we can look at implementing it with one memory type or another on arm64.
Because once we expose that memory type at S2 for KVM guests, it becomes ABI and there is no turning back. So I want to get it right once and for all.
I agree that we need a precise definition for the Linux ioremap_wc API wrt what drivers (kernel and userspace) can expect and whether memset/memcpy is expected to work or not and whether aligned accesses are a requirement.
To the extent ABI is set, I would think that the ABI is also already set in the host kernel for arm64 WC = Normal NC, so why should that not also be the ABI for same driver in VMs.
Quoted 6 lines
Thanks,
M.--
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Vikram,
Quoted 18 lines · Vikram Sethi
Hi Marc,
Quoted 7 lines
From: Marc Zyngier <[email protected]>
Hi Vikram,The problem I see is that we have VM and userspace being written in terms of Write-Combine, which is:
Quoted 7 lines
- loosely defined even on x86
- subject to interpretations in the way it maps to PCI
- has no direct equivalent in the ARMv8 collection of memory attributes (and
Normal_NCcomes with speculation capabilities which strikes me as extremely undesirable on arbitrary devices)
Quoted 6 lines
If speculation with Normal NC to prefetchable BARs in devices was a problem, those devices would already be broken in baremetal with
ioremap_wcon arm64, and we would need quirks there to not do Normal NC for them but Device GRE, and if such a quirk was needed on baremetal, it could be picked up by vfio/KVM as well. But we haven't seen any broken devices doing wc on baremetal on ARM64, have we?
The lack of evidence does not equate to a proof, and your devices not misbehaving doesn't mean it is the right thing, specially when we have such a wide range of CPU and interconnect implementation. Which is why I really want an answer at the architecture level. Not a "it works for me" type of answer.
Furthermore, as I replied to Shanker in a separate email, what Linux/arm64 does is pretty much irrelevant. KVM/arm64 implements the ARMv8 architecture, and it is at that level that we need to solve the problem.
If, by enumerating the properties of Prefetchable, you can show that they are a strict superset of Normal_NC, I'm on board. I haven't seen such an enumeration so far.
Quoted 10 lines
I know we have tested NICs write combining on arm64 in baremetal, as well as GPU and NVMe CMB without issues.
Further, I don't see why speculation to non cacheble would be an
issue if prefetch without side effects is allowed by the device,
which is what a prefetchable BAR is.
If it is an issue for a device I would consider that a bug already needing a quirk in Baremetal/host kernel already.
From PCI spec " A prefetchable address range may have write side effects, but it may not have read side effects."
Right, so we have made a small step in the direction of mapping "prefetchable" onto "Normal_NC", thanks for that. What about all the other properties (unaligned accesses, ordering, gathering)?
Quoted 9 lines
Quoted 9 lines
How do we translate this into something consistent? I'd like to see an actual description of what we *really* expect from WC on prefetchable PCI regions, turn that into a documented definition agreed across architectures, and then we can look at implementing it with one memory type or another on arm64.
Because once we expose that memory type at S2 for KVM guests, it becomes ABI and there is no turning back. So I want to get it right once and for all.
Quoted 7 lines
I agree that we need a precise definition for the Linux
ioremap_wcAPI wrt what drivers (kernel and userspace) can expect and whether memset/memcpy is expected to work or not and whether aligned accesses are a requirement.
To the extent ABI is set, I would think that the ABI is also already set in the host kernel for arm64 WC = Normal NC, so why should that not also be the ABI for same driver in VMs.
KVM is an implementation of the ARM architecture, and doesn't really care about what WC is. If we come to the conclusion that Normal_NC is the natural match for Prefetchable attributes, than we're good and we can have Normal_NC being set by userspace, or even VFIO. But I don't want to set it only because "it works when bare-metal Linux uses it". Remember KVM doesn't only run Linux as guests.
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 31 lines
Date: Mon, 03 May 2021 11:17:23 +0100
From: Marc Zyngier <[email protected]>Hi Vikram,
Quoted 24 lines · Vikram Sethi
Hi Marc,
Quoted 7 lines
From: Marc Zyngier <[email protected]>
Hi Vikram,The problem I see is that we have VM and userspace being written in terms of Write-Combine, which is:
Quoted 7 lines
- loosely defined even on x86
- subject to interpretations in the way it maps to PCI
- has no direct equivalent in the ARMv8 collection of memory attributes (and
Normal_NCcomes with speculation capabilities which strikes me as extremely undesirable on arbitrary devices)If speculation with Normal NC to prefetchable BARs in devices was a problem, those devices would already be broken in baremetal with
ioremap_wcon arm64, and we would need quirks there to not do Normal NC for them but Device GRE, and if such a quirk was needed on baremetal, it could be picked up by vfio/KVM as well. But we haven't seen any broken devices doing wc on baremetal on ARM64, have we?
I think the SC2A11 SoC used in the Socionext developerbox counts as "broken":
https://www.96boards.org/documentation/enterprise/developerbox/support/known-issues.html
I'm not sure my understanding of the issue is 100% correct, but I believe the firmware workaround described there uses the stage 2 translation tables to map "Normal NC" onto "Device nGRE" or something even more restricted. Now this hardware may be classified as simply broken. However...
On hardware based on the NXP LX2160A SoC we're seeing some weird behaviour when using "Normal NC" mappings with an AMD GPU that disappear by using "Device nGnRnE" mappings on OpenBSD. No such issue was observed with hardware based on an Ampere eMAG SoC. I don't fully understand this issue yet, and it may very well be a bug in OpenBSD code, but it does show there are potential pitfalls with using "Normal NC" for mapping prefetchable BARs of PCIe devices.
Quoted 26 lines
The lack of evidence does not equate to a proof, and your devices not misbehaving doesn't mean it is the right thing, specially when we have such a wide range of CPU and interconnect implementation. Which is why I really want an answer at the architecture level. Not a "it works for me" type of answer.
Furthermore, as I replied to Shanker in a separate email, what Linux/arm64 does is pretty much irrelevant. KVM/arm64 implements the ARMv8 architecture, and it is at that level that we need to solve the problem.
If, by enumerating the properties of Prefetchable, you can show that they are a strict superset of
Normal_NC, I'm on board. I haven't seen such an enumeration so far.Quoted 10 lines
I know we have tested NICs write combining on arm64 in baremetal, as well as GPU and NVMe CMB without issues.
Further, I don't see why speculation to non cacheble would be an
issue if prefetch without side effects is allowed by the device,
which is what a prefetchable BAR is.
If it is an issue for a device I would consider that a bug already needing a quirk in Baremetal/host kernel already.
From PCI spec " A prefetchable address range may have write side effects, but it may not have read side effects."
Quoted 3 lines
Right, so we have made a small step in the direction of mapping "prefetchable" onto "
Normal_NC", thanks for that. What about all the other properties (unaligned accesses, ordering, gathering)?
On x86 WC:
1. Is not cached (but stores are buffered).
2. Allows unaligned access just like normal memory.
3. Allows speculative reads.
4. Has weaker ordering than normal memory; [lsm]fence instructions are needed to guarantee a particular ordering of writes with respect to other writes and reads.
5. Stores are buffered. This buffer isn't snooped so it has to be flushed before changes are globally visible. The [sm]fence instructions flush the store buffer.
6. The store buffer may combine multiple writes into a single write.
Now whether the fact the unaligned access is allowed is really part of the semantics of WC mappings is debatable as x86 always allows unaligned access, even for areas mapped with ioremap().
However, this is where userland comes in. The userland graphics stack does assume that graphics memory mapped throug a prefetchable PCIe BAR allows unaligned access if the architecture allows unaligned access for cacheable memory. On arm64 this means that such memory needs to be "Normal NC". And since kernel drivers tend to map such memory using ioremap_wc() that pretty much implies ioremap_wc() shoul use "Normal NC" as well isn't it?
Quoted 35 lines
Quoted 10 lines
Quoted 10 lines
How do we translate this into something consistent? I'd like to see an actual description of what we *really* expect from WC on prefetchable PCI regions, turn that into a documented definition agreed across architectures, and then we can look at implementing it with one memory type or another on arm64.
Because once we expose that memory type at S2 for KVM guests, it becomes ABI and there is no turning back. So I want to get it right once and for all.
Quoted 7 lines
I agree that we need a precise definition for the Linux
ioremap_wcAPI wrt what drivers (kernel and userspace) can expect and whether memset/memcpy is expected to work or not and whether aligned accesses are a requirement.
To the extent ABI is set, I would think that the ABI is also already set in the host kernel for arm64 WC = Normal NC, so why should that not also be the ABI for same driver in VMs.KVM is an implementation of the ARM architecture, and doesn't really care about what WC is. If we come to the conclusion that
Normal_NCis the natural match for Prefetchable attributes, than we're good and we can haveNormal_NCbeing set by userspace, or even VFIO. But I don't want to set it only because "it works when bare-metal Linux uses it". Remember KVM doesn't only run Linux as guests.M.Signature · 6 lines
Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
RE: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 4 lines
From: Mark Kettenis <[email protected]>
Quoted 1 line
From: Marc Zyngier <[email protected]>
snip
Quoted 4 lines
Quoted 4 lines
If, by enumerating the properties of Prefetchable, you can show that they are a strict superset of
Normal_NC, I'm on board. I haven't seen such an enumeration so far.
snip
Quoted 4 lines
Quoted 3 lines
Right, so we have made a small step in the direction of mapping "prefetchable" onto "
Normal_NC", thanks for that. What about all the other properties (unaligned accesses, ordering, gathering)?
Regarding gathering/write combining, that is also allowed to prefetchable per PCI spec
From 1.3.2.2 of 5/0 base spec:
A PCI Express Endpoint requesting memory resources through a BAR must set the BAR's Prefetchable bit unless the range contains locations with read side-effects or locations in which the Function does not tolerate write merging.
Further 7.5.1.2.1 says " A Function is permitted
to mark a range as prefetchable if there are no side effects on reads, the Function returns all bytes on reads regardless of the byte enables, and host bridges can merge processor writes into this range139 without causing errors"
The "regardless of byte enables" suggests to me that unaligned is OK, as only certain byte enables may be set, what do you think?
So to me prefetchable in PCIe spec allows for write combining, read without sideeffect (prefetch/speculative as long as uncached), and unaligned. Regarding ordering I didn't find a statement one way or other in PCIe prefetchable definition, but I think that goes beyond what PCIe says or doesn't say anyway since reordering can also happen in the CPU, and since driver must be aware of correctness issues in its producer/consumer models it will need the right barriers where they are required for correctness anyway (required for the driver/userspace to work on host w/ ioremap_wc).
But perhaps the bigger question is since WC doesn't exist as a Memory type on armv8, yet we are trying to fit something onto ioremap_wc which came from x86 world, shouldn't the arm64 MT we use for WC match the semantics of whatever drivers and userspace expected from ioremap_wc as defined on x86, which as Mark notes below includes unaligned? If we agree to that, we can codify it in the documentation of ioremap_wc and allow for Normal NC on arm64 for ioremap_wc in host or guest.
Beyond that, if we don't want to do it automatically based on prefetchable but from explicit call from userspace is fine too.
Quoted 65 lines
On x86 WC:
1. Is not cached (but stores are buffered).
2. Allows unaligned access just like normal memory.
3. Allows speculative reads.
4. Has weaker ordering than normal memory; [lsm]fence instructions are needed to guarantee a particular ordering of writes with respect to other writes and reads.
5. Stores are buffered. This buffer isn't snooped so it has to be flushed before changes are globally visible. The [sm]fence instructions flush the store buffer.
6. The store buffer may combine multiple writes into a single write.
Now whether the fact the unaligned access is allowed is really part of the semantics of WC mappings is debatable as x86 always allows unaligned access, even for areas mapped with
ioremap().However, this is where userland comes in. The userland graphics stack does assume that graphics memory mapped throug a prefetchable PCIe BAR allows unaligned access if the architecture allows unaligned access for cacheable memory. On arm64 this means that such memory needs to be "Normal NC". And since kernel drivers tend to map such memory using
ioremap_wc()that pretty much impliesioremap_wc()shoul use "Normal NC" as well isn't it?Quoted 35 lines
Quoted 10 lines
Quoted 10 lines
How do we translate this into something consistent? I'd like to see an actual description of what we *really* expect from WC on prefetchable PCI regions, turn that into a documented definition agreed across architectures, and then we can look at implementing it with one memory type or another on arm64.
Because once we expose that memory type at S2 for KVM guests, it becomes ABI and there is no turning back. So I want to get it right once and for all.
Quoted 7 lines
I agree that we need a precise definition for the Linux
ioremap_wcAPI wrt what drivers (kernel and userspace) can expect and whether memset/memcpy is expected to work or not and whether aligned accesses are a requirement.
To the extent ABI is set, I would think that the ABI is also already set in the host kernel for arm64 WC = Normal NC, so why should that not also be the ABI for same driver in VMs.KVM is an implementation of the ARM architecture, and doesn't really care about what WC is. If we come to the conclusion that
Normal_NCis the natural match for Prefetchable attributes, than we're good and we can haveNormal_NCbeing set by userspace, or even VFIO. But I don't want to set it only because "it works when bare-metal Linux uses it". Remember KVM doesn't only run Linux as guests.M.--
Without deviation from the norm, progress is not possible._______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 8 lines · On Mon, 3 May 2021 13:59:43 +0000 Vikram Sethi <[email protected]> wrote:
Quoted 2 lines
From: Mark Kettenis <[email protected]>
Quoted 1 line
From: Marc Zyngier <[email protected]>
snip
Quoted 4 lines
Quoted 4 lines
If, by enumerating the properties of Prefetchable, you can show that they are a strict superset of
Normal_NC, I'm on board. I haven't seen such an enumeration so far.
Quoted 6 lines
snip
Quoted 4 lines
Quoted 3 lines
Right, so we have made a small step in the direction of mapping "prefetchable" onto "
Normal_NC", thanks for that. What about all the other properties (unaligned accesses, ordering, gathering)?Regarding gathering/write combining, that is also allowed to prefetchable per PCI spec
As others have stated, gather/write combining itself is not well defined.
Quoted 4 lines
From 1.3.2.2 of 5/0 base spec:
A PCI Express Endpoint requesting memory resources through a BAR must set the BAR's Prefetchable bit unless the range contains locations with read side-effects or locations in which the Function does not tolerate write merging.
"write merging" This is a very specific thing, per PCI 3.0, 3.2.6:
Byte Merging – occurs when a sequence of individual memory writes (bytes or words) are merged into a single DWORD.
The semantics suggest quadword support in addition to dword, but don't require it. Writes to bytes within a dword can be merged, but duplicate writes cannot.
It seems like an extremely liberal application to suggest that this one write semantic encompasses full write combining semantics, which itself is not clearly defined.
Quoted 8 lines
Further 7.5.1.2.1 says " A Function is permitted
to mark a range as prefetchable if there are no side effects on reads, the Function returns all bytes on reads regardless of the byte enables, and host bridges can merge processor writes into this range139 without causing errors"The "regardless of byte enables" suggests to me that unaligned is OK, as only certain byte enables may be set, what do you think?
So to me prefetchable in PCIe spec allows for write combining, read without
Ironically here, the above PCI spec section defining write merging has separate sections for "combining", "merging", and "collapsing". Only merging is indicated as a requirement for prefetchable resources.
Quoted 6 lines
sideeffect (prefetch/speculative as long as uncached), and unaligned. Regarding ordering I didn't find a statement one way or other in PCIe prefetchable definition, but I think that goes beyond what PCIe says or doesn't say anyway since reordering can also happen in the CPU, and since driver must be aware of correctness issues in its producer/consumer models it will need the right barriers where they are required for correctness anyway (required for the driver/userspace to work on host w/
ioremap_wc).
A lot of hand waving here, imo. Thanks,
Alex
RE: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Alex,
Quoted 32 lines
From: Alex Williamson <[email protected]>
Quoted 8 lines · On Mon, 3 May 2021 13:59:43 +0000 Vikram Sethi <[email protected]> wrote:
Quoted 2 lines
From: Mark Kettenis <[email protected]>
Quoted 1 line
From: Marc Zyngier <[email protected]>
snip
Quoted 4 lines
Quoted 4 lines
If, by enumerating the properties of Prefetchable, you can show that they are a strict superset of
Normal_NC, I'm on board. I haven't seen such an enumeration so far.Quoted 7 lines
snip
Quoted 4 lines
Quoted 3 lines
Right, so we have made a small step in the direction of mapping "prefetchable" onto "
Normal_NC", thanks for that. What about all the other properties (unaligned accesses, ordering, gathering)?Regarding gathering/write combining, that is also allowed to prefetchable per PCI spec
As others have stated, gather/write combining itself is not well defined.
Quoted 4 lines
From 1.3.2.2 of 5/0 base spec:
A PCI Express Endpoint requesting memory resources through a BAR must set the BAR's Prefetchable bit unless the range contains locations with read side-effects or locations in which the Function does not toleratewrite merging.
"write merging" This is a very specific thing, per PCI 3.0, 3.2.6:
Byte Merging – occurs when a sequence of individual memory writes (bytes or words) are merged into a single DWORD.
Quoted 8 lines
The semantics suggest quadword support in addition to dword, but don't require it. Writes to bytes within a dword can be merged, but duplicate writes cannot.
It seems like an extremely liberal application to suggest that this one write semantic encompasses full write combining semantics, which itself is not clearly defined.
Talking to our PCIe SIG representative, PCIe switches are not allowed do any of the byte Merging/combining etc as defined in the PCI spec, and per a rather poorly worded Implementation note in the spec says that no known PCIe Host Briddges/Root ports do it either.
So for PCIe we don't think believe there is any byte merging that happens in the PCIe fabric so it's really a matter of what happens in the CPU core and interconnect before it gets to the PCIe hierarchy.
Stepping back from this patchset, do you agree that it is desirable to support Write combining as understood by ioremap_wc to work in all ISA guests including ARMv8?
You note that x86 virtualization doesn't have this issue, but KVM-ARM does because KVM maps all device BARs as Device Memory type nGnRE which doesn't allow ioremap_wc from within the guest to get the actual semantics desired.
Marc and others have suggested that userspace should provide the hints. But the question is how would qemu vfio do this either? We would be stuck in the same arguments as here, as to what is the correct way to determine the desired attributes for a given BAR such that eventually when a driver in the guest asks for ioremap_wc it actually has a chance of working in the guest, in all ISAs. Do you have any suggestions on how to make progress here?
A device specific list of which BARs are OK to allow ioremap_wc for seems terrible and I'm not sure if a commandline qemu option is any better. Is the user of device assignment/sysadmin supposed to know which BAR of which device is OK to allow ioremap_wc for?
Will/Catalin, perhaps you could explain your thought process on why you chose Normal NC for ioremap_wc on the armv8 linux port instead of Device GRE or other Device Gxx.
Thanks,
Vikram
Will Deacon
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 3 lines · Vikram Sethi
Will/Catalin, perhaps you could explain your thought process on why you chose Normal NC for
ioremap_wcon the armv8 linux port instead of Device GRE or other Device Gxx.
I think a combination of: compatibility with 32-bit Arm, the need to support unaligned accesses and the potential for higher performance.
Furthermore, ioremap() already gives you a Device memory type, and we're tight on MAIR space.
Will
Catalin Marinas
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 7 lines · Will Deacon
Quoted 3 lines · Vikram Sethi
Will/Catalin, perhaps you could explain your thought process on why you chose Normal NC for
ioremap_wcon the armv8 linux port instead of Device GRE or other Device Gxx.I think a combination of: compatibility with 32-bit Arm, the need to support unaligned accesses and the potential for higher performance.
IIRC the _wc suffix also matches the pgprot_writecombine() used by some drivers to map a video framebuffer into user space. Accesses to the framebuffer are not guaranteed to be aligned (memset/memcpy don't ensure alignment on arm64 and the user doesn't have a memset_io or memcpy_toio).
Quoted 2 lines
Furthermore,
ioremap()already gives you a Device memory type, and we're tight on MAIR space.
We have MT_DEVICE_GRE currently reserved though no in-kernel user, we might as well remove it.
Signature · 5 lines
Catalin _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 5 lines · Catalin Marinas
Quoted 2 lines
Furthermore,
ioremap()already gives you a Device memory type, and we're tight on MAIR space.We have
MT_DEVICE_GREcurrently reserved though no in-kernel user, we might as well remove it.
Please do. The more we can cut down on different memory types, the better.
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
In reply to Catalin MarinasHi Marc,
Quoted 10 lines · Catalin Marinas
Quoted 5 lines
Quoted 3 lines
Will/Catalin, perhaps you could explain your thought process on why you chose Normal NC for
ioremap_wcon the armv8 linux port instead of Device GRE or other Device Gxx.I think a combination of: compatibility with 32-bit Arm, the need to support unaligned accesses and the potential for higher performance.
IIRC the
_wcsuffix also matches thepgprot_writecombine()used by some drivers to map a video framebuffer into user space. Accesses to the framebuffer are not guaranteed to be aligned (memset/memcpy don't ensure alignment on arm64 and the user doesn't have amemset_ioormemcpy_toio).
Quoted 4 lines
Quoted 2 lines
Furthermore,
ioremap()already gives you a Device memory type, and we're tight on MAIR space.We have
MT_DEVICE_GREcurrently reserved though no in-kernel user, we might as well remove it.
@Marc, Could you provide your thoughts/guidance for the next step? The proposal of getting hints for prefetchable regions from VFIO/QEMU is not recommended, The only option left is to implement ARM64 dependent logic in KVM.
Option-1: I think we could take advantage of stage-1/2 combining rules to allow NORMAL_NC memory-type for device memory in VM. Always map device memory at stage-2 as NORMAL-NC and trust VM's stage-1 MT.
---------------------------------------------------------------
Stage-2 MT Stage-1 MT Resultant MT (combining-rules/FWB)
---------------------------------------------------------------
Normal-NC Normal-WT Normal-NC
- Normal-WB -
- Normal-NC -
- Device-<attr> Device-<attr>
---------------------------------------------------------------
We've been using this option internally for testing purpose and validated with NVME/Mellanox/GPU pass-through devices on Marvell-Thundex2 platform.
Option-2: Get resource properties associated with MMIO using lookup_resource() and map at stage-2 as Normal-NC if IORESOURCE_PREFETCH is set in flags.
Marc Zyngier
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Hi Shanker,
Quoted 27 lines · Shanker R Donthineni
Hi Marc,
Quoted 10 lines · Catalin Marinas
Quoted 5 lines
Quoted 3 lines
Will/Catalin, perhaps you could explain your thought process on why you chose Normal NC for
ioremap_wcon the armv8 linux port instead of Device GRE or other Device Gxx.I think a combination of: compatibility with 32-bit Arm, the need to support unaligned accesses and the potential for higher performance.
IIRC the
_wcsuffix also matches thepgprot_writecombine()used by some drivers to map a video framebuffer into user space. Accesses to the framebuffer are not guaranteed to be aligned (memset/memcpy don't ensure alignment on arm64 and the user doesn't have amemset_ioormemcpy_toio).Quoted 4 lines
Quoted 2 lines
Furthermore,
ioremap()already gives you a Device memory type, and we're tight on MAIR space.We have
MT_DEVICE_GREcurrently reserved though no in-kernel user, we might as well remove it.@Marc, Could you provide your thoughts/guidance for the next step? The proposal of getting hints for prefetchable regions from VFIO/QEMU is not recommended, The only option left is to implement ARM64 dependent logic in KVM.
Option-1: I think we could take advantage of stage-1/2 combining rules to allow
NORMAL_NCmemory-type for device memory in VM. Always map device memory at stage-2 as NORMAL-NC and trust VM's stage-1 MT.
Quoted 8 lines
---------------------------------------------------------------
Stage-2 MT Stage-1 MT Resultant MT (combining-rules/FWB)
---------------------------------------------------------------
Normal-NC Normal-WT Normal-NC
- Normal-WB -
- Normal-NC -
- Device-<attr> Device-<attr>
---------------------------------------------------------------
I think this is unwise.
Will recently debugged a pretty horrible situation when doing exactly
that: when S1 is off and S2 is on, the I-side is allowed to generate speculative accesses (see ARMv8 ARM G.a D5.2.9 for the details). And yes, implementations definitely do that. Add side-effect reads to the mix, and you're in for a treat.
Quoted 3 lines
We've been using this option internally for testing purpose and validated with NVME/Mellanox/GPU pass-through devices on Marvell-Thundex2 platform.
See above. It *will* break eventually.
Quoted 2 lines
Option-2: Get resource properties associated with MMIO using
lookup_resource()and map at stage-2 as Normal-NC ifIORESOURCE_PREFETCHis set in flags.
That's a pretty roundabout way of doing exactly the same thing you initially proposed. And it suffers from the exact same problems, which is that you change the semantics of the mapping without knowing what the guest's intent is.
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
In reply to Vikram SethiQuoted 41 lines · On Mon, 3 May 2021 22:03:59 +0000 Vikram Sethi <[email protected]> wrote:
Hi Alex,
Quoted 32 lines
From: Alex Williamson <[email protected]>
Quoted 8 lines · On Mon, 3 May 2021 13:59:43 +0000 Vikram Sethi <[email protected]> wrote:
Quoted 2 lines
From: Mark Kettenis <[email protected]>
Quoted 1 line
From: Marc Zyngier <[email protected]>
snip
Quoted 4 lines
Quoted 4 lines
If, by enumerating the properties of Prefetchable, you can show that they are a strict superset of
Normal_NC, I'm on board. I haven't seen such an enumeration so far.Quoted 7 lines
snip
Quoted 4 lines
Quoted 3 lines
Right, so we have made a small step in the direction of mapping "prefetchable" onto "
Normal_NC", thanks for that. What about all the other properties (unaligned accesses, ordering, gathering)?Regarding gathering/write combining, that is also allowed to prefetchable per PCI spec
As others have stated, gather/write combining itself is not well defined.
Quoted 4 lines
From 1.3.2.2 of 5/0 base spec:
A PCI Express Endpoint requesting memory resources through a BAR must set the BAR's Prefetchable bit unless the range contains locations with read side-effects or locations in which the Function does not toleratewrite merging.
"write merging" This is a very specific thing, per PCI 3.0, 3.2.6:
Byte Merging – occurs when a sequence of individual memory writes (bytes or words) are merged into a single DWORD.
Quoted 8 lines
The semantics suggest quadword support in addition to dword, but don't require it. Writes to bytes within a dword can be merged, but duplicate writes cannot.
It seems like an extremely liberal application to suggest that this one write semantic encompasses full write combining semantics, which itself is not clearly defined.
Quoted 7 lines
Talking to our PCIe SIG representative, PCIe switches are not allowed do any of the byte Merging/combining etc as defined in the PCI spec, and per a rather poorly worded Implementation note in the spec says that no known PCIe Host Briddges/Root ports do it either.
So for PCIe we don't think believe there is any byte merging that happens in the PCIe fabric so it's really a matter of what happens in the CPU core and interconnect before it gets to the PCIe hierarchy.
Yes, but merged writes, no matter where they happen, are still the only type of write combining that a prefetchable BAR on an endpoint is required to support.
Quoted 3 lines
Stepping back from this patchset, do you agree that it is desirable to support Write combining as understood by
ioremap_wcto work in all ISA guests including ARMv8?
Yes, a userspace vfio driver should be able to take advantage of the hardware capabilities. I think where we disagree is whether it's universally safe to assume write combining based on the PCI prefetchable capability of a BAR. If that's something that can be assumed universally for ARMv8 based on the architecture specification compatibility with the PCI definition of a prefetchable BAR, then I would expect a helper somewhere in arch code that returns the right page protection flags, so that arch maintainers don't need to scour device drivers for architecture hacks. Otherwise, it needs to be exposed through the vfio uAPI to allow the userspace device driver itself to select these semantics.
Quoted 10 lines
You note that x86 virtualization doesn't have this issue, but KVM-ARM does because KVM maps all device BARs as Device Memory type nGnRE which doesn't allow
ioremap_wcfrom within the guest to get the actual semantics desired.Marc and others have suggested that userspace should provide the hints. But the question is how would qemu vfio do this either? We would be stuck in the same arguments as here, as to what is the correct way to determine the desired attributes for a given BAR such that eventually when a driver in the guest asks for
ioremap_wcit actually has a chance of working in the guest, in all ISAs. Do you have any suggestions on how to make progress here?
We do need some way for userspace drivers to also make use of WC semantics, there were some discussions in the past, I think others have referenced them as well, but nothing has been proposed for a vfio API.
If we had that API, QEMU deciding to universally enable WC for all vfio prefetchable BARs seems only marginally better than this approach. Ultimately the mapping should be based on the guest driver semantics, and if you don't have any visibility to that on KVM/arm like we have on KVM/x86, then it seems like there's nothing to trigger a vfio API here anyway.
If that's the case, I'd probably go back to letting the arch/arm64 folks declare that WC is compatible with the definition of PCI prefetchable and export some sort of pgprot_pci_prefetchable() helper where the default would be to #define it as pgproc_noncached() #ifndef by the arch.
Quoted 4 lines
A device specific list of which BARs are OK to allow
ioremap_wcfor seems terrible and I'm not sure if a commandline qemu option is any better. Is the user of device assignment/sysadmin supposed to know which BAR of which device is OK to allowioremap_wcfor?
No, a device specific userspace driver should know such device semantics, but QEMU is not such a driver. Burdening the hypervisor user/admin is not a good solution either. I'd lean on KVM/arm64 folks to know how the guest driver semantics can be exposed to the hypervisor. Thanks,
Alex
Marc Zyngier
maintainerRe: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 92 lines · Alex Williamson
Quoted 41 lines · On Mon, 3 May 2021 22:03:59 +0000 Vikram Sethi <[email protected]> wrote:
Hi Alex,
Quoted 32 lines
From: Alex Williamson <[email protected]>
Quoted 8 lines · On Mon, 3 May 2021 13:59:43 +0000 Vikram Sethi <[email protected]> wrote:
Quoted 2 lines
From: Mark Kettenis <[email protected]>
Quoted 1 line
From: Marc Zyngier <[email protected]>
snip
Quoted 4 lines
Quoted 4 lines
If, by enumerating the properties of Prefetchable, you can show that they are a strict superset of
Normal_NC, I'm on board. I haven't seen such an enumeration so far.Quoted 7 lines
snip
Quoted 4 lines
Quoted 3 lines
Right, so we have made a small step in the direction of mapping "prefetchable" onto "
Normal_NC", thanks for that. What about all the other properties (unaligned accesses, ordering, gathering)?Regarding gathering/write combining, that is also allowed to prefetchable per PCI spec
As others have stated, gather/write combining itself is not well defined.
Quoted 4 lines
From 1.3.2.2 of 5/0 base spec:
A PCI Express Endpoint requesting memory resources through a BAR must set the BAR's Prefetchable bit unless the range contains locations with read side-effects or locations in which the Function does not toleratewrite merging.
"write merging" This is a very specific thing, per PCI 3.0, 3.2.6:
Byte Merging – occurs when a sequence of individual memory writes (bytes or words) are merged into a single DWORD.
Quoted 8 lines
The semantics suggest quadword support in addition to dword, but don't require it. Writes to bytes within a dword can be merged, but duplicate writes cannot.
It seems like an extremely liberal application to suggest that this one write semantic encompasses full write combining semantics, which itself is not clearly defined.
Quoted 8 lines
Talking to our PCIe SIG representative, PCIe switches are not allowed do any of the byte Merging/combining etc as defined in the PCI spec, and per a rather poorly worded Implementation note in the spec says that no known PCIe Host Briddges/Root ports do it either. So for PCIe we don't think believe there is any byte merging that happens in the PCIe fabric so it's really a matter of what happens in the CPU core and interconnect before it gets to the PCIe hierarchy.
Yes, but merged writes, no matter where they happen, are still the only type of write combining that a prefetchable BAR on an endpoint is required to support.
Quoted 3 lines
Stepping back from this patchset, do you agree that it is desirable to support Write combining as understood by
ioremap_wcto work in all ISA guests including ARMv8?Yes, a userspace vfio driver should be able to take advantage of the hardware capabilities. I think where we disagree is whether it's universally safe to assume write combining based on the PCI prefetchable capability of a BAR. If that's something that can be assumed universally for ARMv8 based on the architecture specification compatibility with the PCI definition of a prefetchable BAR, then I would expect a helper somewhere in arch code that returns the right page protection flags, so that arch maintainers don't need to scour device drivers for architecture hacks. Otherwise, it needs to be exposed through the vfio uAPI to allow the userspace device driver itself to select these semantics.
Quoted 5 lines
You note that x86 virtualization doesn't have this issue, but KVM-ARM does because KVM maps all device BARs as Device Memory type nGnRE which doesn't allow
ioremap_wcfrom within the guest to get the actual semantics desired.Quoted 8 lines
Marc and others have suggested that userspace should provide the hints. But the question is how would qemu vfio do this either? We would be stuck in the same arguments as here, as to what is the correct way to determine the desired attributes for a given BAR such that eventually when a driver in the guest asks for
ioremap_wcit actually has a chance of working in the guest, in all ISAs. Do you have any suggestions on how to make progress here?We do need some way for userspace drivers to also make use of WC semantics, there were some discussions in the past, I think others have referenced them as well, but nothing has been proposed for a vfio API.
Quoted 6 lines
If we had that API, QEMU deciding to universally enable WC for all vfio prefetchable BARs seems only marginally better than this approach. Ultimately the mapping should be based on the guest driver semantics, and if you don't have any visibility to that on KVM/arm like we have on KVM/x86, then it seems like there's nothing to trigger a vfio API here anyway.
There isn't much KVM/arm64 can do here unless it is being told what to do. We don't have visibility on the guest's page tables in a reliable way, and trusting them is not something I want to entertain anyway.
Quoted 11 lines
If that's the case, I'd probably go back to letting the
arch/arm64folks declare that WC is compatible with the definition of PCI prefetchable and export some sort ofpgprot_pci_prefetchable()helper where the default would be to #define it aspgproc_noncached()#ifndef by the arch.Quoted 4 lines
A device specific list of which BARs are OK to allow
ioremap_wcfor seems terrible and I'm not sure if a commandline qemu option is any better. Is the user of device assignment/sysadmin supposed to know which BAR of which device is OK to allowioremap_wcfor?
Quoted 5 lines
No, a device specific userspace driver should know such device semantics, but QEMU is not such a driver. Burdening the hypervisor user/admin is not a good solution either. I'd lean on KVM/arm64 folks to know how the guest driver semantics can be exposed to the hypervisor. Thanks,
I don't see a good way for that, unless we make it a per-guest buy-in where all PCI prefetchable mappings get the same treatment. I'm prepared to bet that this will break when two devices will have different requirements. It would also require userspace to buy into this scheme though, which is crap.
Exposing the guest's preference on a per-device basis seems difficult (KVM knows nothing about the PCI devices) and would require some PV interface that will quickly become unmaintainable.
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
In reply to Shanker Donthineni[+Jason, Ben]
Quoted 11 lines · Shanker Donthineni
For pass-through device assignment, the ARM64 KVM hypervisor retrieves the memory region properties physical address, size, and whether a region backed with struct page or not from VMA. The prefetchable attribute of a BAR region isn't visible to KVM to make an optimal decision for stage2 attributes.
This patch updates
vma->vm_page_protand maps with write-combine attribute if the associated BAR is prefetchable. For ARM64pgprot_writecombine()is mapped to memory-typeMT_NORMAL_NCwhich has no side effects on reads and multiple writes can be combined.
Quoted 4 lines
Signed-off-by Shanker Donthineni <[email protected]>--- drivers/vfio/pci/vfio_pci.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
A bit of background information that may be useful:
https://lore.kernel.org/linux-pci/2b539df4c9ec703458e46da2fc879ee3b310b31c.camel@kernel.crashing.org
Lorenzo
Quoted 20 lines
drivers/vfio/pci/vfio_pci.c +5
-1@@ -1703,7 +1703,11 @@ static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma) } vma->vm_private_data = vdev; - vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); +if (IS_ENABLED (CONFIG_ARM64 ) && + (pci_resource_flags (pdev, index) &IORESOURCE_PREFETCH )) + vma->vm_page_prot =pgprot_writecombine (vma->vm_page_prot); +else + vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); vma->vm_pgoff = (pci_resource_start (pdev, index) >>PAGE_SHIFT ) + pgoff;/* Signature · 1 line
2.17.1
Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA
Quoted 14 lines · Lorenzo Pieralisi
[+Jason, Ben]
Quoted 11 lines · Shanker Donthineni
For pass-through device assignment, the ARM64 KVM hypervisor retrieves the memory region properties physical address, size, and whether a region backed with struct page or not from VMA. The prefetchable attribute of a BAR region isn't visible to KVM to make an optimal decision for stage2 attributes.
This patch updates
vma->vm_page_protand maps with write-combine attribute if the associated BAR is prefetchable. For ARM64pgprot_writecombine()is mapped to memory-typeMT_NORMAL_NCwhich has no side effects on reads and multiple writes can be combined.
Quoted 7 lines
Quoted 3 lines
Signed-off-by Shanker Donthineni <[email protected]>drivers/vfio/pci/vfio_pci.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)A bit of background information that may be useful:
https://lore.kernel.org/linux-pci/2b539df4c9ec703458e46da2fc879ee3b310b31c.camel@kernel.crashing.org
This can't happen automatically.
writecombining or not is a uABI visible change. Userspace needs to explicitly request and know that the mmap it gets back is writecombining.
Jason
RFC 2/2 KVM: arm64: Add write-combine support for stage-2 entries
In reply to Shanker DonthineniIn the current implementation, the device memory is always mapped as DEVICE_nGnRE in stage-2. In the host kernel, device drivers have flexibility whether to choose a memory-type device or write-combine (Non-cacheable) depends on the use case. PCI specification has a prefetchable BAR concept where multiple writes can be combined and no side effects on reads. It provides huge performance improvement and also allows unaligned access.
NVIDIA GPU PCIe devices have 3 BAR regions. Two regions are mapped to video/compute memory and marked as prefetchable. The GPU driver takes advantage of the write-combine feature for higher performance. The same driver has no issues in the host kernel but crashes inside the virtual machine because of unaligned accesses.
This patch finds the PTE attributes for device memory in VMA. It updates the stage-2 attribute to NORMAL_NC for WC regions and the default type DEVICE_nGnRE for non-WC regions.
--- arch/arm64/include/asm/kvm_mmu.h | 3 ++- arch/arm64/include/asm/kvm_pgtable.h | 2 ++ arch/arm64/include/asm/memory.h | 4 +++- arch/arm64/kvm/hyp/pgtable.c | 9 +++++++-- arch/arm64/kvm/mmu.c | 21 ++++++++++++++++++--- arch/arm64/kvm/vgic/vgic-v2.c | 2 +- 6 files changed, 33 insertions(+), 8 deletions(-)
arch/arm64/include/asm/kvm_mmu.h +2 -1
@@ -160,7 +160,8 @@ void stage2_unmap_vm(struct kvm *kvm);int kvm_init_stage2_mmu (struct kvm *kvm,struct kvm_s2_mmu *mmu);void kvm_free_stage2_pgd (struct kvm_s2_mmu *mmu);int kvm_phys_addr_ioremap (struct kvm *kvm,phys_addr_t guest_ipa, -phys_addr_t pa,unsigned long size,bool writable); +phys_addr_t pa,unsigned long size,bool writable, +bool writecombine);int kvm_handle_guest_abort (struct kvm_vcpu *vcpu);
arch/arm64/include/asm/kvm_pgtable.h +2 -0
@@ -35,6 +35,7 @@ struct kvm_pgtable {* @KVM_PGTABLE_PROT_W: Write permission. * @KVM_PGTABLE_PROT_R: Read permission. * @KVM_PGTABLE_PROT_DEVICE: Device attributes. +* @KVM_PGTABLE_PROT_WC: Normal non-cacheable (WC). */ enum kvm_pgtable_prot {KVM_PGTABLE_PROT_X =BIT (0 ),
@@ -42,6 +43,7 @@ enum kvm_pgtable_prot {KVM_PGTABLE_PROT_R =BIT (2 ),KVM_PGTABLE_PROT_DEVICE =BIT (3 ), + KVM_PGTABLE_PROT_WC = BIT(4), };#define PAGE_HYP (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W)
arch/arm64/include/asm/memory.h +3 -1
@@ -144,13 +144,15 @@* Memory types for Stage-2 translation */ #define MT_S2_NORMAL 0xf +#define MT_S2_WRITE_COMBINE 5#define MT_S2_DEVICE_nGnRE 0x1/* * Memory types for Stage-2 translation when ID_AA64MMFR2_EL1.FWB is 0001 -* Stage-2 enforces Normal-WB and Device-nGnRE +* Stage-2 enforces Normal-WB, Normal-NC and Device-nGnRE */ #define MT_S2_FWB_NORMAL 6 +#define MT_S2_FWB_WRITE_COMBINE 5#define MT_S2_FWB_DEVICE_nGnRE 1#ifdef CONFIG_ARM64_4K_PAGES
arch/arm64/kvm/hyp/pgtable.c +7 -2
@@ -444,9 +444,14 @@ static int stage2_map_set_prot_attr(enum kvm_pgtable_prot prot, struct stage2_map_data *data) {bool device = prot &KVM_PGTABLE_PROT_DEVICE ; -kvm_pte_t attr = device ?PAGE_S2_MEMATTR (DEVICE_nGnRE) : -PAGE_S2_MEMATTR (NORMAL );u32 sh =KVM_PTE_LEAF_ATTR_LO_S2_SH_IS ; +kvm_pte_t attr =PAGE_S2_MEMATTR (NORMAL ); + +if (device) { + attr = (prot &KVM_PGTABLE_PROT_WC ) ? +PAGE_S2_MEMATTR (WRITE_COMBINE ) : +PAGE_S2_MEMATTR (DEVICE_nGnRE); + }if (!(prot &KVM_PGTABLE_PROT_X )) attr |=KVM_PTE_LEAF_ATTR_HI_S2_XN ;
arch/arm64/kvm/mmu.c +18 -3
@@ -487,6 +487,16 @@ void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu) } } +/** +* is_vma_write_combine - check if VMA is mapped with writecombine or not +* Return true if VMA mapped with MT_NORMAL_NC otherwise fasle +*/ +static bool inline is_vma_write_combine (struct vm_area_struct *vma) +{ +pteval_t pteval =pgprot_val (vma->vm_page_prot); +return ((pteval &PTE_ATTRINDX_MASK ) ==PTE_ATTRINDX (MT_NORMAL_NC )); +} +/** * kvm_phys_addr_ioremap - map a device range to guest IPA *
@@ -495,9 +505,11 @@ void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu)* @pa: The physical address of the device * @size: The size of the mapping * @writable: Whether or not to create a writable mapping +* @writecombine: Whether or not to create a writecombine mapping */ int kvm_phys_addr_ioremap (struct kvm *kvm,phys_addr_t guest_ipa, -phys_addr_t pa,unsigned long size,bool writable) +phys_addr_t pa,unsigned long size,bool writable, +bool writecombine) {phys_addr_t addr;int ret =0 ;
@@ -505,6 +517,7 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,struct kvm_pgtable *pgt = kvm->arch.mmu.pgt;enum kvm_pgtable_prot prot =KVM_PGTABLE_PROT_DEVICE |KVM_PGTABLE_PROT_R | + (writecombine ?KVM_PGTABLE_PROT_WC :0 ) | (writable ?KVM_PGTABLE_PROT_W :0 ); size +=offset_in_page (guest_ipa);
@@ -891,7 +904,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, }if (device) - prot |=KVM_PGTABLE_PROT_DEVICE ; + prot |=KVM_PGTABLE_PROT_DEVICE | + (is_vma_write_combine (vma) ?KVM_PGTABLE_PROT_WC :0 );else if (cpus_have_const_cap (ARM64_HAS_CACHE_DIC )) prot |=KVM_PGTABLE_PROT_X ;
@@ -1357,7 +1371,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm, ret =kvm_phys_addr_ioremap (kvm, gpa, pa, vm_end - vm_start, - writable); + writable, +is_vma_write_combine (vma));if (ret)break ; }
arch/arm64/kvm/vgic/vgic-v2.c +1 -1
@@ -336,7 +336,7 @@ int vgic_v2_map_resources(struct kvm *kvm)if (!static_branch_unlikely (&vgic_v2_cpuif_trap)) { ret =kvm_phys_addr_ioremap (kvm, dist->vgic_cpu_base, kvm_vgic_global_state.vcpu_base, -KVM_VGIC_V2_CPU_SIZE , true); +KVM_VGIC_V2_CPU_SIZE , true, false);if (ret) {kvm_err ("Unable to remap VGIC CPU to VCPU\n" );return ret;
Signature · 1 line
2.17.1
kernel test robot
build botRe: [RFC 2/2] KVM: arm64: Add write-combine support for stage-2 entries
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
Hi Shanker,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on vfio/next]
[also build test WARNING on arm64/for-next/core v5.12] [cannot apply to kvmarm/next next-20210429]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Shanker-Donthineni/Honor-PCI-prefetchable-attributes-for-a-virtual-machine-on-ARM64/20210430-003235 base: https://github.com/awilliam/linux-vfio.git next config: arm64-allyesconfig (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/29ca5fa04a11cb4ea736ef1128e48f0bdc9f2296
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Shanker-Donthineni/Honor-PCI-prefetchable-attributes-for-a-virtual-machine-on-ARM64/20210430-003235 git checkout 29ca5fa04a11cb4ea736ef1128e48f0bdc9f2296
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> arch/arm64/kvm/mmu.c:494:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
494 | static bool inline is_vma_write_combine(struct vm_area_struct *vma)
| ^~~~~~
vim +/inline +494 arch/arm64/kvm/mmu.c
489
490 /**
491 * is_vma_write_combine - check if VMA is mapped with writecombine or not
492 * Return true if VMA mapped with MT_NORMAL_NC otherwise fasle
493 */
> 494 static bool inline is_vma_write_combine(struct vm_area_struct *vma)
495 {
496 pteval_t pteval = pgprot_val(vma->vm_page_prot);
497 return ((pteval & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL_NC));
498 }
499
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Quoted 1 line · Shanker Donthineni
Problem statement: Virtual machine crashes when NVIDIA GPU driver access a prefetchable BAR space due to the unaligned reads/writes for pass-through devices. The same binary works fine as expected in the host kernel. Only one BAR has control & status registers (CSR) and other PCI BARs are marked as prefetchable. NVIDIA GPU driver uses the write-combine feature for mapping the prefetchable BARs to improve performance. This problem applies to all other drivers which want to enable WC.
Unless you mean the noveau drivers this simply does not matter. Please don't spam the kernel lists with issues with your broken and license violating drivers.