Re: [Intel-gfx] [PATCH v3 0/6] Convert the intel iommu driver to the dma-iommu api
On 2020-09-18 21:47, Logan Gunthorpe wrote: Hi Lu, On 2020-09-11 9:21 p.m., Lu Baolu wrote: Tom Murphy has almost done all the work. His latest patch series was posted here. https://lore.kernel.org/linux-iommu/20200903201839.7327-1-murph...@tcd.ie/ Thanks a lot! This series is a follow-up with below changes: 1. Add a quirk for the i915 driver issue described in Tom's cover letter. 2. Fix several bugs in patch "iommu: Allow the dma-iommu api to use bounce buffers" to make the bounce buffer work for untrusted devices. 3. Several cleanups in iommu/vt-d driver after the conversion. I'm trying to test this on an old Sandy Bridge, but found that I get spammed with warnings on boot. I've put a sample of a few of them below. They all seem to be related to ioat. I had the same issue with Tom's v2 but never saw this on his v1. I think this might have more to do with ioat being totally broken - AFAICS it appears to allocate descriptors with a size of 2MB, but free them with a size of 512KB. Try throwing CONFIG_DMA_API_DEBUG at it to confirm. Robin. Thanks, Logan [ 44.057877] [ cut here ] [ 44.063167] WARNING: CPU: 4 PID: 84 at drivers/iommu/dma-iommu.c:496 __iommu_dma_unmap+0x115/0x130 [ 44.073351] Modules linked in: [ 44.076882] CPU: 4 PID: 84 Comm: kworker/4:1 Tainted: GW 5.9.0-rc4-6-g110da1e703a2 #216 [ 44.087935] Hardware name: Supermicro SYS-7047GR-TRF/X9DRG-QF, BIOS 3.0a 12/05/2013 [ 44.096650] Workqueue: events work_for_cpu_fn [ 44.101644] RIP: 0010:__iommu_dma_unmap+0x115/0x130 [ 44.107225] Code: 48 8b 0c 24 48 c7 44 24 10 00 00 00 00 48 c7 44 24 18 00 00 00 00 48 c7 44 24 20 00 00 00 00 48 c7 44 24 08 ff ff ff ff eb 8d <0f> 0b e9 74 ff ff ff e8 1f 36 66 00 66 66 2e 0f 1f 84 00 00 00 00 [ 44.128443] RSP: :c90002397c38 EFLAGS: 00010206 [ 44.134413] RAX: 0020 RBX: 0008 RCX: [ 44.144487] RDX: 0403 RSI: 82a7fb20 RDI: 888477804f30 [ 44.152613] RBP: fec0 R08: 000fec00 R09: 000fedff [ 44.160733] R10: 0002 R11: 0004 R12: 888270c39000 [ 44.168861] R13: 888472d85ee8 R14: 0008 R15: [ 44.176980] FS: () GS:88847980() knlGS: [ 44.186198] CS: 0010 DS: ES: CR0: 80050033 [ 44.192761] CR2: CR3: 02a20001 CR4: 000606e0 [ 44.200871] Call Trace: [ 44.203716] ? lockdep_hardirqs_on_prepare+0xad/0x180 [ 44.209509] iommu_dma_free+0x18/0x30 [ 44.213734] ioat_free_chan_resources+0x19e/0x300 [ 44.219133] ioat_dma_self_test+0x2a0/0x3d0 [ 44.223964] ioat_pci_probe+0x60e/0x903 [ 44.228387] local_pci_probe+0x42/0x80 [ 44.232721] work_for_cpu_fn+0x16/0x20 [ 44.237037] process_one_work+0x292/0x630 [ 44.241644] ? __schedule+0x344/0x970 [ 44.245868] worker_thread+0x227/0x3e0 [ 44.250185] ? process_one_work+0x630/0x630 [ 44.254989] kthread+0x136/0x150 [ 44.258718] ? kthread_use_mm+0x100/0x100 [ 44.263334] ret_from_fork+0x22/0x30 [ 44.267474] irq event stamp: 1881262 [ 44.271602] hardirqs last enabled at (1881272): [] console_unlock+0x435/0x570 [ 44.281593] hardirqs last disabled at (1881281): [] console_unlock+0x42b/0x570 [ 44.291588] softirqs last enabled at (1747140): [] ioat_cleanup+0x65/0x470 [ 44.301285] softirqs last disabled at (1747144): [] ioat_free_chan_resources+0x6a/0x300 [ 44.312153] ---[ end trace ed0f88fd959a5a39 ]--- [ 44.353963] [ cut here ] [ 44.359291] WARNING: CPU: 4 PID: 84 at drivers/iommu/dma-iommu.c:496 __iommu_dma_unmap+0x115/0x130 [ 44.369482] Modules linked in: [ 44.373030] CPU: 4 PID: 84 Comm: kworker/4:1 Tainted: GW 5.9.0-rc4-6-g110da1e703a2 #216 [ 44.384097] Hardware name: Supermicro SYS-7047GR-TRF/X9DRG-QF, BIOS 3.0a 12/05/2013 [ 44.392825] Workqueue: events work_for_cpu_fn [ 44.397831] RIP: 0010:__iommu_dma_unmap+0x115/0x130 [ 44.403421] Code: 48 8b 0c 24 48 c7 44 24 10 00 00 00 00 48 c7 44 24 18 00 00 00 00 48 c7 44 24 20 00 00 00 00 48 c7 44 24 08 ff ff ff ff eb 8d <0f> 0b e9 74 ff ff ff e8 1f 36 66 00 66 66 2e 0f 1f 84 00 00 00 00 [ 44.424644] RSP: :c90002397c38 EFLAGS: 00010206 [ 44.430627] RAX: 0020 RBX: 0008 RCX: [ 44.438770] RDX: 0403 RSI: 82a7fb20 RDI: 888477804f30 [ 44.446885] RBP: ffc0 R08: 000ffc00 R09: 000ffdff [ 44.455000] R10: 0002 R11: 0004 R12: 888270c55000 [ 44.463119] R13: 888472dc7268 R14: 0008 R15: [ 44.471235] FS: () GS:88847980() knlGS: [ 44.480440] CS: 0010 DS: ES: CR0: 80050033 [ 44.487004] CR2: CR3: 02a20001 CR4: 000606e0 [
Re: [PATCH 15/18] dma-mapping: add a new dma_alloc_pages API
On Tue, Sep 15, 2020 at 05:51:19PM +0200, Christoph Hellwig wrote: > This API is the equivalent of alloc_pages, except that the returned memory > is guaranteed to be DMA addressable by the passed in device. The > implementation will also be used to provide a more sensible replacement > for DMA_ATTR_NON_CONSISTENT flag. > > Additionally dma_alloc_noncoherent is switched over to use dma_alloc_pages > as its backend. > > Signed-off-by: Christoph Hellwig > --- > Documentation/core-api/dma-attributes.rst | 8 --- > arch/alpha/kernel/pci_iommu.c | 2 + > arch/arm/mm/dma-mapping-nommu.c | 2 + > arch/arm/mm/dma-mapping.c | 4 ++ > arch/ia64/hp/common/sba_iommu.c | 2 + > arch/mips/jazz/jazzdma.c | 7 +-- > arch/powerpc/kernel/dma-iommu.c | 2 + > arch/powerpc/platforms/ps3/system-bus.c | 4 ++ > arch/powerpc/platforms/pseries/vio.c | 2 + > arch/s390/pci/pci_dma.c | 2 + > arch/x86/kernel/amd_gart_64.c | 2 + > drivers/iommu/dma-iommu.c | 2 + > drivers/iommu/intel/iommu.c | 4 ++ > drivers/parisc/ccio-dma.c | 2 + > drivers/parisc/sba_iommu.c| 2 + > drivers/xen/swiotlb-xen.c | 2 + > include/linux/dma-direct.h| 5 ++ > include/linux/dma-mapping.h | 34 ++-- > include/linux/dma-noncoherent.h | 3 -- > kernel/dma/direct.c | 52 ++- > kernel/dma/mapping.c | 63 +-- > kernel/dma/ops_helpers.c | 35 + > kernel/dma/virt.c | 2 + > 23 files changed, 206 insertions(+), 37 deletions(-) Acked-by: Thomas Bogendoerfer (MIPS part) -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 13/18] 53c700: convert to dma_alloc_noncoherent
On Tue, Sep 15, 2020 at 05:51:17PM +0200, Christoph Hellwig wrote: > Use the new non-coherent DMA API including proper ownership transfers. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/53c700.c | 20 ++-- > 1 file changed, 14 insertions(+), 6 deletions(-) Tested-by: Thomas Bogendoerfer -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 06/18] lib82596: move DMA allocation into the callers of i82596_probe
On Tue, Sep 15, 2020 at 05:51:10PM +0200, Christoph Hellwig wrote: > This allows us to get rid of the LIB82596_DMA_ATTR defined and prepare > for untangling the coherent vs non-coherent DMA allocation API. > > Signed-off-by: Christoph Hellwig > --- > drivers/net/ethernet/i825xx/lasi_82596.c | 24 ++-- > drivers/net/ethernet/i825xx/lib82596.c | 36 > drivers/net/ethernet/i825xx/sni_82596.c | 19 + > 3 files changed, 40 insertions(+), 39 deletions(-) Tested-by: Thomas Bogendoerfer (SNI part) -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 14/18] dma-mapping: remove dma_cache_sync
On Tue, Sep 15, 2020 at 05:51:18PM +0200, Christoph Hellwig wrote: > All users are gone now, remove the API. > > Signed-off-by: Christoph Hellwig > --- > arch/mips/Kconfig | 1 - > arch/mips/jazz/jazzdma.c| 1 - > arch/mips/mm/dma-noncoherent.c | 6 -- > arch/parisc/Kconfig | 1 - > arch/parisc/kernel/pci-dma.c| 6 -- > include/linux/dma-mapping.h | 8 > include/linux/dma-noncoherent.h | 10 -- > kernel/dma/Kconfig | 3 --- > kernel/dma/mapping.c| 14 -- > 9 files changed, 50 deletions(-) Acked-by: Thomas Bogendoerfer (MIPS part) -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 11/18] lib82596: convert to dma_alloc_noncoherent
On Tue, Sep 15, 2020 at 05:51:15PM +0200, Christoph Hellwig wrote: > Use the new non-coherent DMA API including proper ownership transfers. > This includes moving the DMA helpers to lib82596 based of an ifdef to > avoid include order problems. > > Signed-off-by: Christoph Hellwig > --- > drivers/net/ethernet/i825xx/lasi_82596.c | 25 ++--- > drivers/net/ethernet/i825xx/lib82596.c | 114 ++- > drivers/net/ethernet/i825xx/sni_82596.c | 4 - > 3 files changed, 80 insertions(+), 63 deletions(-) Tested-by: Thomas Bogendoerfer (SNI part) -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 07/18] 53c700: improve non-coherent DMA handling
On Tue, Sep 15, 2020 at 05:51:11PM +0200, Christoph Hellwig wrote: > Switch the 53c700 driver to only use non-coherent descriptor memory if it > really has to because dma_alloc_coherent fails. This doesn't matter for > any of the platforms it runs on currently, but that will change soon. > > To help with this two new helpers to transfer ownership to and from the > device are added that abstract the syncing of the non-coherent memory. > The two current bidirectional cases are mapped to transfers to the > device, as that appears to what they are used for. Note that for parisc, > which is the only architecture this driver needs to use non-coherent > memory on, the direction argument of dma_cache_sync is ignored, so this > will not change behavior in any way. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/53c700.c | 113 +++--- > drivers/scsi/53c700.h | 17 --- > 2 files changed, 72 insertions(+), 58 deletions(-) Tested-by: Thomas Bogendoerfer -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 09/18] sgiwd93: convert to dma_alloc_noncoherent
On Tue, Sep 15, 2020 at 05:51:13PM +0200, Christoph Hellwig wrote: > Use the new non-coherent DMA API including proper ownership transfers. > This also means we can allocate the memory as DMA_TO_DEVICE instead > of bidirectional. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/sgiwd93.c | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) Tested-by: Thomas Bogendoerfer -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/18] sgiseeq: convert to dma_alloc_noncoherent
On Tue, Sep 15, 2020 at 05:51:16PM +0200, Christoph Hellwig wrote: > Use the new non-coherent DMA API including proper ownership transfers. > This includes adding additional calls to dma_sync_desc_dev as the > old syncing was rather ad-hoc. > > Thanks to Thomas Bogendoerfer for debugging the ownership transfer > issues. > > Signed-off-by: Christoph Hellwig > --- > drivers/net/ethernet/seeq/sgiseeq.c | 28 ++-- > 1 file changed, 18 insertions(+), 10 deletions(-) Tested-by: Thomas Bogendoerfer -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 10/18] hal2: convert to dma_alloc_noncoherent
On Tue, Sep 15, 2020 at 05:51:14PM +0200, Christoph Hellwig wrote: > Use the new non-coherent DMA API including proper ownership transfers. > This also means we can allocate the buffer memory with the proper > direction instead of bidirectional. > > Signed-off-by: Christoph Hellwig > --- > sound/mips/hal2.c | 58 ++- > 1 file changed, 27 insertions(+), 31 deletions(-) Tested-by: Thomas Bogendoerfer -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/6] Convert the intel iommu driver to the dma-iommu api
On 2020-09-15 09:31, Tvrtko Ursulin wrote: On 15/09/2020 02:47, Lu Baolu wrote: Hi Tvrtko, On 9/14/20 4:04 PM, Tvrtko Ursulin wrote: Hi, On 12/09/2020 04:21, Lu Baolu wrote: Tom Murphy has almost done all the work. His latest patch series was posted here. https://lore.kernel.org/linux-iommu/20200903201839.7327-1-murph...@tcd.ie/ Thanks a lot! This series is a follow-up with below changes: 1. Add a quirk for the i915 driver issue described in Tom's cover letter. Last week I have copied you on an i915 series which appears to remove the need for this quirk. so if we get those i915 patches reviewed and merged, do you still want to pursue this quirk? It's up to the graphic guys. I don't know the details in i915 driver. I don't think my tests could cover all cases. I am the graphic guy. :) I just need some reviews (internally) for my series and then we can merge it, at which point you don't need the quirk patch any more. I'll try to accelerate this. With regards to testing, you could send your series with my patches on top to our trybot mailing list (intel-gfx-try...@lists.freedesktop.org / https://patchwork.freedesktop.org/project/intel-gfx-trybot/series/?ordering=-last_updated) which would show you if it is still hitting the DMAR issues in i915. 2. Fix several bugs in patch "iommu: Allow the dma-iommu api to use bounce buffers" to make the bounce buffer work for untrusted devices. 3. Several cleanups in iommu/vt-d driver after the conversion. With the previous version of the series I hit a problem on Ivybridge where apparently the dma engine width is not respected. At least that is my layman interpretation of the errors. From the older thread: <3> [209.526605] DMAR: intel_iommu_map: iommu width (39) is not sufficient for the mapped address (008000) Relevant iommu boot related messages are: <6>[ 0.184234] DMAR: Host address width 36 <6>[ 0.184245] DMAR: DRHD base: 0x00fed9 flags: 0x0 <6>[ 0.184288] DMAR: dmar0: reg_base_addr fed9 ver 1:0 cap c020e60262 ecap f0101a <6>[ 0.184308] DMAR: DRHD base: 0x00fed91000 flags: 0x1 <6>[ 0.184337] DMAR: dmar1: reg_base_addr fed91000 ver 1:0 cap c9008020660262 ecap f0105a <6>[ 0.184357] DMAR: RMRR base: 0x00d8d28000 end: 0x00d8d46fff <6>[ 0.184377] DMAR: RMRR base: 0x00db00 end: 0x00df1f <6>[ 0.184398] DMAR-IR: IOAPIC id 2 under DRHD base 0xfed91000 IOMMU 1 <6>[ 0.184414] DMAR-IR: HPET id 0 under DRHD base 0xfed91000 <6>[ 0.184428] DMAR-IR: Queued invalidation will be enabled to support x2apic and Intr-remapping. <6>[ 0.185173] DMAR-IR: Enabled IRQ remapping in x2apic mode <6>[ 0.878934] DMAR: No ATSR found <6>[ 0.878966] DMAR: dmar0: Using Queued invalidation <6>[ 0.879007] DMAR: dmar1: Using Queued invalidation <6>[ 0.915032] DMAR: Intel(R) Virtualization Technology for Directed I/O <6>[ 0.915060] PCI-DMA: Using software bounce buffering for IO (SWIOTLB) <6>[ 0.915084] software IO TLB: mapped [mem 0xc80d4000-0xcc0d4000] (64MB) (Full boot log at https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_7054/fi-ivb-3770/boot0.txt, failures at https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_7054/fi-ivb-3770/igt@i915_selftest@l...@blt.html.) Does this look familiar or at least plausible to you? Is this something your new series has fixed? This happens during attaching a domain to device. It has nothing to do with this patch series. I will look into this issue, but not in this email thread context. I am not sure what step is attaching domain to device, but these type messages: <3> [209.526605] DMAR: intel_iommu_map: iommu width (39) is not >> sufficient for the mapped address (008000) They definitely appear to happen at runtime, as i915 is getting exercised by userspace. AFAICS this certainly might be related to this series - iommu-dma will constrain IOVA allocation based on the domain geometry that the driver reports, which in this case is set only once when first allocating the domain. Thus it looks like both the dmar_domain->gaw adjustment in prepare_domain_attach_device() and the domain_use_first_level() business in intel_alloc_iova() effectively get lost in this conversion, since the domain geometry never gets updated to reflect those additional constraints. Robin. Regards, Tvrtko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/3] dma-mapping: better document dma_addr_t and DMA_MAPPING_ERROR
Move the comment documenting dma_addr_t away from the dma_map_ops
definition which isn't very related to it, and toward DMA_MAPPING_ERROR,
which is somewhat related. Add a little blurb about DMA_MAPPING_ERROR
as well.
Signed-off-by: Christoph Hellwig
---
include/linux/dma-mapping.h | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 51e93d44b826c8..c4395cf7e265dd 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -67,12 +67,6 @@
*/
#define DMA_ATTR_PRIVILEGED(1UL << 9)
-/*
- * A dma_addr_t can hold any valid DMA or bus address for the platform.
- * It can be given to a device to use as a DMA source or target. A CPU cannot
- * reference a dma_addr_t directly because there may be translation between
- * its physical address space and the bus address space.
- */
struct dma_map_ops {
void* (*alloc)(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp,
@@ -131,6 +125,16 @@ struct dma_map_ops {
unsigned long (*get_merge_boundary)(struct device *dev);
};
+/*
+ * A dma_addr_t can hold any valid DMA or bus address for the platform. It can
+ * be given to a device to use as a DMA source or target. A CPU cannot
+ * reference a dma_addr_t directly because there may be translation between its
+ * physical address space and the bus address space.
+ *
+ * DMA_MAPPING_ERROR is the magic error code if a mapping failed. It should
not
+ * be used directly in drivers, but checked for using dma_mapping_error()
+ * instead.
+ */
#define DMA_MAPPING_ERROR (~(dma_addr_t)0)
extern const struct dma_map_ops dma_virt_ops;
--
2.28.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
a few trivial dma-mapping header cleanups
Hi all, these three patches clean up dma-mapping.h a little bit ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] dma-mapping: move valid_dma_direction to dma-direction.h
Move the valid_dma_direction helper to a more suitable header, and
clean it up to use the proper enum as well as removing pointless braces.
Signed-off-by: Christoph Hellwig
---
include/linux/dma-direction.h | 8 +++-
include/linux/dma-mapping.h | 7 ---
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/include/linux/dma-direction.h b/include/linux/dma-direction.h
index 9c96e30e6a0bb0..a2fe4571bc9279 100644
--- a/include/linux/dma-direction.h
+++ b/include/linux/dma-direction.h
@@ -9,4 +9,10 @@ enum dma_data_direction {
DMA_NONE = 3,
};
-#endif
+static inline int valid_dma_direction(enum dma_data_direction dir)
+{
+ return dir == DMA_BIDIRECTIONAL || dir == DMA_TO_DEVICE ||
+ dir == DMA_FROM_DEVICE;
+}
+
+#endif /* _LINUX_DMA_DIRECTION_H */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index e074588d753ff6..51e93d44b826c8 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -138,13 +138,6 @@ extern const struct dma_map_ops dma_dummy_ops;
#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
-static inline int valid_dma_direction(int dma_direction)
-{
- return ((dma_direction == DMA_BIDIRECTIONAL) ||
- (dma_direction == DMA_TO_DEVICE) ||
- (dma_direction == DMA_FROM_DEVICE));
-}
-
#ifdef CONFIG_DMA_DECLARE_COHERENT
/*
* These three functions are only for dma allocator.
--
2.28.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3] dma-mapping: remove DMA_MASK_NONE
This value is only used by a PCMCIA driver and not very useful.
Signed-off-by: Christoph Hellwig
---
drivers/pcmcia/ds.c | 2 +-
include/linux/dma-mapping.h | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pcmcia/ds.c b/drivers/pcmcia/ds.c
index 09d06b082f8b8c..72114907c0e4d2 100644
--- a/drivers/pcmcia/ds.c
+++ b/drivers/pcmcia/ds.c
@@ -516,7 +516,7 @@ static struct pcmcia_device *pcmcia_device_add(struct
pcmcia_socket *s,
p_dev->dev.parent = s->dev.parent;
p_dev->dev.release = pcmcia_release_dev;
/* by default don't allow DMA */
- p_dev->dma_mask = DMA_MASK_NONE;
+ p_dev->dma_mask = 0;
p_dev->dev.dma_mask = &p_dev->dma_mask;
dev_set_name(&p_dev->dev, "%d.%d", p_dev->socket->sock,
p_dev->device_no);
if (!dev_name(&p_dev->dev))
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index bb138ac6f5e63e..e074588d753ff6 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -138,8 +138,6 @@ extern const struct dma_map_ops dma_dummy_ops;
#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
-#define DMA_MASK_NONE 0x0ULL
-
static inline int valid_dma_direction(int dma_direction)
{
return ((dma_direction == DMA_BIDIRECTIONAL) ||
--
2.28.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 3/3] dma-mapping: better document dma_addr_t and DMA_MAPPING_ERROR
From: Christoph Hellwig
> Sent: 22 September 2020 14:40
...
> @@ -131,6 +125,16 @@ struct dma_map_ops {
> unsigned long (*get_merge_boundary)(struct device *dev);
> };
>
> +/*
> + * A dma_addr_t can hold any valid DMA or bus address for the platform. It
> can
> + * be given to a device to use as a DMA source or target. A CPU cannot
> + * reference a dma_addr_t directly because there may be translation between
> its
> + * physical address space and the bus address space.
It can't access it 'directly' because it isn't a virtual address
> + *
> + * DMA_MAPPING_ERROR is the magic error code if a mapping failed. It should
> not
> + * be used directly in drivers, but checked for using dma_mapping_error()
> + * instead.
> + */
I think it might be worth adding:
A dma_addr_t value may be device dependant and differ from the
'physical address' of the memory.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] dma-mapping: remove DMA_MASK_NONE
On Tue, Sep 22, 2020 at 03:40:00PM +0200, Christoph Hellwig wrote: > This value is only used by a PCMCIA driver and not very useful. > > Signed-off-by: Christoph Hellwig Acked-by: Dominik Brodowski ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 3/3] arm64: dts: meson: Describe G12b GPU as coherent
According to a downstream commit I found in the Khadas vendor kernel,
the GPU on G12b is wired up for ACE-lite, so (now that Panfrost knows
how to handle this properly) we should describe it as such. Otherwise
the mismatch leads to all manner of fun with mismatched attributes and
inadvertently snooping stale data from caches, which would account for
at least some of the brokenness observed on this platform.
Reviewed-by: Neil Armstrong
Tested-by: Neil Armstrong
Signed-off-by: Robin Murphy
---
arch/arm64/boot/dts/amlogic/meson-g12b.dtsi | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi
b/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi
index 9b8548e5f6e5..ee8fcae9f9f0 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi
@@ -135,3 +135,7 @@ map1 {
};
};
};
+
+&mali {
+ dma-coherent;
+};
--
2.28.0.dirty
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/3] iommu/io-pgtable-arm: Support coherency for Mali LPAE
Midgard GPUs have ACE-Lite master interfaces which allows systems to integrate them in an I/O-coherent manner. It seems that from the GPU's viewpoint, the rest of the system is its outer shareable domain, and so even when snoop signals are wired up, they are only emitted for outer shareable accesses. As such, setting the TTBR_SHARE_OUTER bit does indeed get coherent pagetable walks working nicely for the coherent T620 in the Arm Juno SoC. Reviewed-by: Steven Price Tested-by: Neil Armstrong Signed-off-by: Robin Murphy --- drivers/iommu/io-pgtable-arm.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c index dc7bcf858b6d..b4072a18e45d 100644 --- a/drivers/iommu/io-pgtable-arm.c +++ b/drivers/iommu/io-pgtable-arm.c @@ -440,7 +440,13 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, << ARM_LPAE_PTE_ATTRINDX_SHIFT); } - if (prot & IOMMU_CACHE) + /* +* Also Mali has its own notions of shareability wherein its Inner +* domain covers the cores within the GPU, and its Outer domain is +* "outside the GPU" (i.e. either the Inner or System domain in CPU +* terms, depending on coherency). +*/ + if (prot & IOMMU_CACHE && data->iop.fmt != ARM_MALI_LPAE) pte |= ARM_LPAE_PTE_SH_IS; else pte |= ARM_LPAE_PTE_SH_OS; @@ -1049,6 +1055,9 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) cfg->arm_mali_lpae_cfg.transtab = virt_to_phys(data->pgd) | ARM_MALI_LPAE_TTBR_READ_INNER | ARM_MALI_LPAE_TTBR_ADRMODE_TABLE; + if (cfg->coherent_walk) + cfg->arm_mali_lpae_cfg.transtab |= ARM_MALI_LPAE_TTBR_SHARE_OUTER; + return &data->iop; out_free_data: -- 2.28.0.dirty ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 0/3] drm: panfrost: Coherency support
Hi all, Here's a quick v2 with the tags so far picked up and some inline commentary about the shareability domains for the pagetable code. Robin. Robin Murphy (3): iommu/io-pgtable-arm: Support coherency for Mali LPAE drm/panfrost: Support cache-coherent integrations arm64: dts: meson: Describe G12b GPU as coherent arch/arm64/boot/dts/amlogic/meson-g12b.dtsi | 4 drivers/gpu/drm/panfrost/panfrost_device.h | 1 + drivers/gpu/drm/panfrost/panfrost_drv.c | 2 ++ drivers/gpu/drm/panfrost/panfrost_gem.c | 2 ++ drivers/gpu/drm/panfrost/panfrost_mmu.c | 1 + drivers/iommu/io-pgtable-arm.c | 11 ++- 6 files changed, 20 insertions(+), 1 deletion(-) -- 2.28.0.dirty ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 2/3] drm/panfrost: Support cache-coherent integrations
When the GPU's ACE-Lite interface is fully wired up and capable of
snooping CPU caches, it may be described as "dma-coherent" in
devicetree, which will already inform the DMA layer not to perform
unnecessary cache maintenance. However, we still need to ensure that
the GPU uses the appropriate cacheable outer-shareable attributes in
order to generate the requisite snoop signals, and that CPU mappings
don't create a mismatch by using a non-cacheable type either.
Reviewed-by: Steven Price
Tested-by: Neil Armstrong
Signed-off-by: Robin Murphy
---
drivers/gpu/drm/panfrost/panfrost_device.h | 1 +
drivers/gpu/drm/panfrost/panfrost_drv.c| 2 ++
drivers/gpu/drm/panfrost/panfrost_gem.c| 2 ++
drivers/gpu/drm/panfrost/panfrost_mmu.c| 1 +
4 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index c30c719a8059..b31f45315e96 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -84,6 +84,7 @@ struct panfrost_device {
/* pm_domains for devices with more than one. */
struct device *pm_domain_devs[MAX_PM_DOMAINS];
struct device_link *pm_domain_links[MAX_PM_DOMAINS];
+ bool coherent;
struct panfrost_features features;
const struct panfrost_compatible *comp;
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index ada51df9a7a3..2a6f2f716b2f 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -588,6 +588,8 @@ static int panfrost_probe(struct platform_device *pdev)
if (!pfdev->comp)
return -ENODEV;
+ pfdev->coherent = device_get_dma_attr(&pdev->dev) == DEV_DMA_COHERENT;
+
/* Allocate and initialze the DRM device. */
ddev = drm_dev_alloc(&panfrost_drm_driver, &pdev->dev);
if (IS_ERR(ddev))
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 33355dd302f1..cdf1a8754eba 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -220,6 +220,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs
= {
*/
struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev,
size_t size)
{
+ struct panfrost_device *pfdev = dev->dev_private;
struct panfrost_gem_object *obj;
obj = kzalloc(sizeof(*obj), GFP_KERNEL);
@@ -229,6 +230,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct
drm_device *dev, size_t
INIT_LIST_HEAD(&obj->mappings.list);
mutex_init(&obj->mappings.lock);
obj->base.base.funcs = &panfrost_gem_funcs;
+ obj->base.map_cached = pfdev->coherent;
return &obj->base.base;
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index e8f7b11352d2..8852fd378f7a 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -371,6 +371,7 @@ int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv
*priv)
.pgsize_bitmap = SZ_4K | SZ_2M,
.ias= FIELD_GET(0xff, pfdev->features.mmu_features),
.oas= FIELD_GET(0xff00,
pfdev->features.mmu_features),
+ .coherent_walk = pfdev->coherent,
.tlb= &mmu_tlb_ops,
.iommu_dev = pfdev->dev,
};
--
2.28.0.dirty
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Intel-gfx] [PATCH v3 0/6] Convert the intel iommu driver to the dma-iommu api
On 2020-09-21 6:24 p.m., Lu Baolu wrote: I'm trying to test this on an old Sandy Bridge, but found that I get spammed with warnings on boot. I've put a sample of a few of them below. They all seem to be related to ioat. I had the same issue with Tom's v2 but never saw this on his v1. >>> >>> Have you verified whether this could be reproduced with the lasted >>> upstream kernel (without this patch series)? >> >> Yes. > > I am sorry. Just want to make sure I understand you correctly. :-) When > you say "yes", do you mean you could reproduce this with pure upstream > kernel (5.9-rc6)? I mean I've verified the bug does *not* exist without this patch set. > >> Also, it's hitting a warning in the dma-iommu code which would not >> be hit without this patch set. > > Without this series, DMA APIs don't go through dma-iommu. Do you mind > posting the warning message? It was on my original email but here it is again: [ 44.057877] [ cut here ] [ 44.063167] WARNING: CPU: 4 PID: 84 at drivers/iommu/dma-iommu.c:496 __iommu_dma_unmap+0x115/0x130 [ 44.073351] Modules linked in: [ 44.076882] CPU: 4 PID: 84 Comm: kworker/4:1 Tainted: GW 5.9.0-rc4-6-g110da1e703a2 #216 [ 44.087935] Hardware name: Supermicro SYS-7047GR-TRF/X9DRG-QF, BIOS 3.0a 12/05/2013 [ 44.096650] Workqueue: events work_for_cpu_fn [ 44.101644] RIP: 0010:__iommu_dma_unmap+0x115/0x130 [ 44.107225] Code: 48 8b 0c 24 48 c7 44 24 10 00 00 00 00 48 c7 44 24 18 00 00 00 00 48 c7 44 24 20 00 00 00 00 48 c7 44 24 08 ff ff ff ff eb 8d <0f> 0b e9 74 ff ff ff e8 1f 36 66 00 66 66 2e 0f 1f 84 00 00 00 00 [ 44.128443] RSP: :c90002397c38 EFLAGS: 00010206 [ 44.134413] RAX: 0020 RBX: 0008 RCX: [ 44.144487] RDX: 0403 RSI: 82a7fb20 RDI: 888477804f30 [ 44.152613] RBP: fec0 R08: 000fec00 R09: 000fedff [ 44.160733] R10: 0002 R11: 0004 R12: 888270c39000 [ 44.168861] R13: 888472d85ee8 R14: 0008 R15: [ 44.176980] FS: () GS:88847980() knlGS: [ 44.186198] CS: 0010 DS: ES: CR0: 80050033 [ 44.192761] CR2: CR3: 02a20001 CR4: 000606e0 [ 44.200871] Call Trace: [ 44.203716] ? lockdep_hardirqs_on_prepare+0xad/0x180 [ 44.209509] iommu_dma_free+0x18/0x30 [ 44.213734] ioat_free_chan_resources+0x19e/0x300 [ 44.219133] ioat_dma_self_test+0x2a0/0x3d0 [ 44.223964] ioat_pci_probe+0x60e/0x903 [ 44.228387] local_pci_probe+0x42/0x80 [ 44.232721] work_for_cpu_fn+0x16/0x20 [ 44.237037] process_one_work+0x292/0x630 [ 44.241644] ? __schedule+0x344/0x970 [ 44.245868] worker_thread+0x227/0x3e0 [ 44.250185] ? process_one_work+0x630/0x630 [ 44.254989] kthread+0x136/0x150 [ 44.258718] ? kthread_use_mm+0x100/0x100 [ 44.263334] ret_from_fork+0x22/0x30 [ 44.267474] irq event stamp: 1881262 [ 44.271602] hardirqs last enabled at (1881272): [] console_unlock+0x435/0x570 [ 44.281593] hardirqs last disabled at (1881281): [] console_unlock+0x42b/0x570 [ 44.291588] softirqs last enabled at (1747140): [] ioat_cleanup+0x65/0x470 [ 44.301285] softirqs last disabled at (1747144): [] ioat_free_chan_resources+0x6a/0x300 [ 44.312153] ---[ end trace ed0f88fd959a5a39 ]--- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/3] drm: panfrost: Coherency support
Series is: Acked-by: Alyssa Rosenzweig On Tue, Sep 22, 2020 at 03:16:47PM +0100, Robin Murphy wrote: > Hi all, > > Here's a quick v2 with the tags so far picked up and some inline > commentary about the shareability domains for the pagetable code. > > Robin. > > > Robin Murphy (3): > iommu/io-pgtable-arm: Support coherency for Mali LPAE > drm/panfrost: Support cache-coherent integrations > arm64: dts: meson: Describe G12b GPU as coherent > > arch/arm64/boot/dts/amlogic/meson-g12b.dtsi | 4 > drivers/gpu/drm/panfrost/panfrost_device.h | 1 + > drivers/gpu/drm/panfrost/panfrost_drv.c | 2 ++ > drivers/gpu/drm/panfrost/panfrost_gem.c | 2 ++ > drivers/gpu/drm/panfrost/panfrost_mmu.c | 1 + > drivers/iommu/io-pgtable-arm.c | 11 ++- > 6 files changed, 20 insertions(+), 1 deletion(-) > > -- > 2.28.0.dirty > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Intel-gfx] [PATCH v3 0/6] Convert the intel iommu driver to the dma-iommu api
On 2020-09-22 3:51 a.m., Robin Murphy wrote:
> On 2020-09-18 21:47, Logan Gunthorpe wrote:
>> Hi Lu,
>>
>> On 2020-09-11 9:21 p.m., Lu Baolu wrote:
>>> Tom Murphy has almost done all the work. His latest patch series was
>>> posted here.
>>>
>>> https://lore.kernel.org/linux-iommu/20200903201839.7327-1-murph...@tcd.ie/
>>>
>>> Thanks a lot!
>>>
>>> This series is a follow-up with below changes:
>>>
>>> 1. Add a quirk for the i915 driver issue described in Tom's cover
>>> letter.
>>> 2. Fix several bugs in patch "iommu: Allow the dma-iommu api to use
>>> bounce buffers" to make the bounce buffer work for untrusted devices.
>>> 3. Several cleanups in iommu/vt-d driver after the conversion.
>>>
>>
>> I'm trying to test this on an old Sandy Bridge, but found that I get
>> spammed with warnings on boot. I've put a sample of a few of them below.
>> They all seem to be related to ioat.
>>
>> I had the same issue with Tom's v2 but never saw this on his v1.
>
> I think this might have more to do with ioat being totally broken -
> AFAICS it appears to allocate descriptors with a size of 2MB, but free
> them with a size of 512KB. Try throwing CONFIG_DMA_API_DEBUG at it to
> confirm.
Ah, yes, nice catch. Looks like it was broken recently by the following
commit, but nobody noticed and the dma-iommu patch set added a warning
which caught it.
a02254f8a676 ("dmaengine: ioat: Decreasing allocation chunk size 2M->512K")
Reverting that fixes the issue. I'll try to send patch or two for this.
Logan
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v10 0/7] IOMMU user API enhancement
IOMMU user API header was introduced to support nested DMA translation and related fault handling. The current UAPI data structures consist of three areas that cover the interactions between host kernel and guest: - fault handling - cache invalidation - bind guest page tables, i.e. guest PASID Future extensions are likely to support more architectures and vIOMMU features. In the previous discussion, using user-filled data size and feature flags is made a preferred approach over a unified version number. https://lkml.org/lkml/2020/1/29/45 In addition to introduce argsz field to data structures, this patchset is also trying to document the UAPI design, usage, and extension rules. VT-d driver changes to utilize the new argsz field is included, VFIO usage is to follow. This set is available at: https://github.com/jacobpan/linux.git vsva_v5.9_uapi_v10 Thanks, Jacob Changelog: v10 - Documentation grammar fixes based on Randy's review v9 - Directly pass PASID value to iommu_sva_unbind_gpasid() without the superfluous data in struct iommu_gpasid_bind_data. v8 - Rebased to v5.9-rc2 - Addressed review comments from Eric Auger 1. added a check for the unused vendor flags 2. commit message improvements v7 - Added PASID data format enum for range checking - Tidy up based on reviews from Alex W. - Removed doc section for vIOMMU fault handling v6 - Renamed all UAPI functions with iommu_uapi_ prefix - Replaced argsz maxsz checking with flag specific size checks - Documentation improvements based on suggestions by Eric Auger Replaced example code with a pointer to the actual code - Added more checks for illegal flags combinations - Added doc file to MAINTAINERS v5 - Addjusted paddings in UAPI data to be 8 byte aligned - Do not clobber argsz in IOMMU core before passing on to vendor driver - Removed pr_warn_ for invalid UAPI data check, just return -EINVAL - Clarified VFIO responsibility in UAPI data handling - Use iommu_uapi prefix to differentiate APIs has in-kernel caller - Added comment for unchecked flags of invalidation granularity - Added example in doc to show vendor data checking v4 - Added checks of UAPI data for reserved fields, version, and flags. - Removed version check from vendor driver (vt-d) - Relaxed argsz check to match the UAPI struct size instead of variable union size - Updated documentation v3: - Rewrote backward compatibility rule to support existing code re-compiled with newer kernel UAPI header that runs on older kernel. Based on review comment from Alex W. https://lore.kernel.org/linux-iommu/20200611094741.6d118...@w520.home/ - Take user pointer directly in UAPI functions. Perform argsz check and copy_from_user() in IOMMU driver. Eliminate the need for VFIO or other upper layer to parse IOMMU data. - Create wrapper function for in-kernel users of UAPI functions v2: - Removed unified API version and helper - Introduced argsz for each UAPI data - Introduced UAPI doc Jacob Pan (7): docs: IOMMU user API iommu/uapi: Add argsz for user filled data iommu/uapi: Introduce enum type for PASID data format iommu/uapi: Use named union for user data iommu/uapi: Rename uapi functions iommu/uapi: Handle data and argsz filled by users iommu/vt-d: Check UAPI data processed by IOMMU core Documentation/userspace-api/iommu.rst | 210 ++ MAINTAINERS | 1 + drivers/iommu/intel/iommu.c | 25 ++-- drivers/iommu/intel/svm.c | 13 ++- drivers/iommu/iommu.c | 201 ++-- include/linux/iommu.h | 35 -- include/uapi/linux/iommu.h| 25 ++-- 7 files changed, 467 insertions(+), 43 deletions(-) create mode 100644 Documentation/userspace-api/iommu.rst -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v10 6/7] iommu/uapi: Handle data and argsz filled by users
IOMMU user APIs are responsible for processing user data. This patch
changes the interface such that user pointers can be passed into IOMMU
code directly. Separate kernel APIs without user pointers are introduced
for in-kernel users of the UAPI functionality.
IOMMU UAPI data has a user filled argsz field which indicates the data
length of the structure. User data is not trusted, argsz must be
validated based on the current kernel data size, mandatory data size,
and feature flags.
User data may also be extended, resulting in possible argsz increase.
Backward compatibility is ensured based on size and flags (or
the functional equivalent fields) checking.
This patch adds sanity checks in the IOMMU layer. In addition to argsz,
reserved/unused fields in padding, flags, and version are also checked.
Details are documented in Documentation/userspace-api/iommu.rst
Signed-off-by: Liu Yi L
Signed-off-by: Jacob Pan
---
drivers/iommu/iommu.c | 199 --
include/linux/iommu.h | 28 ---
2 files changed, 211 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 4ae02291ccc2..5c1b7ae48aae 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1961,34 +1961,219 @@ int iommu_attach_device(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_attach_device);
+/*
+ * Check flags and other user provided data for valid combinations. We also
+ * make sure no reserved fields or unused flags are set. This is to ensure
+ * not breaking userspace in the future when these fields or flags are used.
+ */
+static int iommu_check_cache_invl_data(struct iommu_cache_invalidate_info
*info)
+{
+ u32 mask;
+ int i;
+
+ if (info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
+ return -EINVAL;
+
+ mask = (1 << IOMMU_CACHE_INV_TYPE_NR) - 1;
+ if (info->cache & ~mask)
+ return -EINVAL;
+
+ if (info->granularity >= IOMMU_INV_GRANU_NR)
+ return -EINVAL;
+
+ switch (info->granularity) {
+ case IOMMU_INV_GRANU_ADDR:
+ if (info->cache & IOMMU_CACHE_INV_TYPE_PASID)
+ return -EINVAL;
+
+ mask = IOMMU_INV_ADDR_FLAGS_PASID |
+ IOMMU_INV_ADDR_FLAGS_ARCHID |
+ IOMMU_INV_ADDR_FLAGS_LEAF;
+
+ if (info->granu.addr_info.flags & ~mask)
+ return -EINVAL;
+ break;
+ case IOMMU_INV_GRANU_PASID:
+ mask = IOMMU_INV_PASID_FLAGS_PASID |
+ IOMMU_INV_PASID_FLAGS_ARCHID;
+ if (info->granu.pasid_info.flags & ~mask)
+ return -EINVAL;
+
+ break;
+ case IOMMU_INV_GRANU_DOMAIN:
+ if (info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB)
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Check reserved padding fields */
+ for (i = 0; i < sizeof(info->padding); i++) {
+ if (info->padding[i])
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
int iommu_uapi_cache_invalidate(struct iommu_domain *domain, struct device
*dev,
- struct iommu_cache_invalidate_info *inv_info)
+ void __user *uinfo)
{
+ struct iommu_cache_invalidate_info inv_info = { 0 };
+ u32 minsz;
+ int ret = 0;
+
if (unlikely(!domain->ops->cache_invalidate))
return -ENODEV;
- return domain->ops->cache_invalidate(domain, dev, inv_info);
+ /*
+* No new spaces can be added before the variable sized union, the
+* minimum size is the offset to the union.
+*/
+ minsz = offsetof(struct iommu_cache_invalidate_info, granu);
+
+ /* Copy minsz from user to get flags and argsz */
+ if (copy_from_user(&inv_info, uinfo, minsz))
+ return -EFAULT;
+
+ /* Fields before variable size union is mandatory */
+ if (inv_info.argsz < minsz)
+ return -EINVAL;
+
+ /* PASID and address granu require additional info beyond minsz */
+ if (inv_info.argsz == minsz &&
+ ((inv_info.granularity == IOMMU_INV_GRANU_PASID) ||
+ (inv_info.granularity == IOMMU_INV_GRANU_ADDR)))
+ return -EINVAL;
+
+ if (inv_info.granularity == IOMMU_INV_GRANU_PASID &&
+ inv_info.argsz < offsetofend(struct iommu_cache_invalidate_info,
granu.pasid_info))
+ return -EINVAL;
+
+ if (inv_info.granularity == IOMMU_INV_GRANU_ADDR &&
+ inv_info.argsz < offsetofend(struct iommu_cache_invalidate_info,
granu.addr_info))
+ return -EINVAL;
+
+ /*
+* User might be using a newer UAPI header which has a larger data
+* size, we shall support the existing flag
[PATCH v10 4/7] iommu/uapi: Use named union for user data
IOMMU UAPI data size is filled by the user space which must be validated
by the kernel. To ensure backward compatibility, user data can only be
extended by either re-purpose padding bytes or extend the variable sized
union at the end. No size change is allowed before the union. Therefore,
the minimum size is the offset of the union.
To use offsetof() on the union, we must make it named.
Link: https://lore.kernel.org/linux-iommu/20200611145518.0c281...@x1.home/
Signed-off-by: Jacob Pan
Reviewed-by: Lu Baolu
Reviewed-by: Eric Auger
---
drivers/iommu/intel/iommu.c | 22 +++---
drivers/iommu/intel/svm.c | 2 +-
include/uapi/linux/iommu.h | 4 ++--
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 87b17bac04c2..461f3a6864d4 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -5434,8 +5434,8 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
/* Size is only valid in address selective invalidation */
if (inv_info->granularity == IOMMU_INV_GRANU_ADDR)
- size = to_vtd_size(inv_info->addr_info.granule_size,
- inv_info->addr_info.nb_granules);
+ size = to_vtd_size(inv_info->granu.addr_info.granule_size,
+ inv_info->granu.addr_info.nb_granules);
for_each_set_bit(cache_type,
(unsigned long *)&inv_info->cache,
@@ -5456,20 +5456,20 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
* granularity.
*/
if (inv_info->granularity == IOMMU_INV_GRANU_PASID &&
- (inv_info->pasid_info.flags & IOMMU_INV_PASID_FLAGS_PASID))
- pasid = inv_info->pasid_info.pasid;
+ (inv_info->granu.pasid_info.flags &
IOMMU_INV_PASID_FLAGS_PASID))
+ pasid = inv_info->granu.pasid_info.pasid;
else if (inv_info->granularity == IOMMU_INV_GRANU_ADDR &&
-(inv_info->addr_info.flags &
IOMMU_INV_ADDR_FLAGS_PASID))
- pasid = inv_info->addr_info.pasid;
+(inv_info->granu.addr_info.flags &
IOMMU_INV_ADDR_FLAGS_PASID))
+ pasid = inv_info->granu.addr_info.pasid;
switch (BIT(cache_type)) {
case IOMMU_CACHE_INV_TYPE_IOTLB:
/* HW will ignore LSB bits based on address mask */
if (inv_info->granularity == IOMMU_INV_GRANU_ADDR &&
size &&
- (inv_info->addr_info.addr & ((BIT(VTD_PAGE_SHIFT +
size)) - 1))) {
+ (inv_info->granu.addr_info.addr &
((BIT(VTD_PAGE_SHIFT + size)) - 1))) {
pr_err_ratelimited("User address not aligned,
0x%llx, size order %llu\n",
- inv_info->addr_info.addr,
size);
+
inv_info->granu.addr_info.addr, size);
}
/*
@@ -5477,9 +5477,9 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
* We use npages = -1 to indicate that.
*/
qi_flush_piotlb(iommu, did, pasid,
- mm_to_dma_pfn(inv_info->addr_info.addr),
+
mm_to_dma_pfn(inv_info->granu.addr_info.addr),
(granu == QI_GRAN_NONG_PASID) ? -1 : 1
<< size,
- inv_info->addr_info.flags &
IOMMU_INV_ADDR_FLAGS_LEAF);
+ inv_info->granu.addr_info.flags &
IOMMU_INV_ADDR_FLAGS_LEAF);
if (!info->ats_enabled)
break;
@@ -5502,7 +5502,7 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
size = 64 - VTD_PAGE_SHIFT;
addr = 0;
} else if (inv_info->granularity ==
IOMMU_INV_GRANU_ADDR) {
- addr = inv_info->addr_info.addr;
+ addr = inv_info->granu.addr_info.addr;
}
if (info->ats_enabled)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 95c3164a2302..99353d6468fa 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -370,7 +370,7 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain,
struct device *dev,
spin_lock(&iommu->lock);
ret = intel_pasid_setup_nested(iommu, dev,
(pgd_t *)(uintptr_t)data->gpgd,
- data->hpasid,
[PATCH v10 5/7] iommu/uapi: Rename uapi functions
User APIs such as iommu_sva_unbind_gpasid() may also be used by the
kernel. Since we introduced user pointer to the UAPI functions,
in-kernel callers cannot share the same APIs. In-kernel callers are also
trusted, there is no need to validate the data.
We plan to have two flavors of the same API functions, one called
through ioctls, carrying a user pointer and one called directly with
valid IOMMU UAPI structs. To differentiate both, let's rename existing
functions with an iommu_uapi_ prefix.
Suggested-by: Alex Williamson
Reviewed-by: Eric Auger
Signed-off-by: Jacob Pan
---
drivers/iommu/iommu.c | 18 +-
include/linux/iommu.h | 31 ---
2 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 609bd25bf154..4ae02291ccc2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1961,35 +1961,35 @@ int iommu_attach_device(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_attach_device);
-int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
- struct iommu_cache_invalidate_info *inv_info)
+int iommu_uapi_cache_invalidate(struct iommu_domain *domain, struct device
*dev,
+ struct iommu_cache_invalidate_info *inv_info)
{
if (unlikely(!domain->ops->cache_invalidate))
return -ENODEV;
return domain->ops->cache_invalidate(domain, dev, inv_info);
}
-EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
+EXPORT_SYMBOL_GPL(iommu_uapi_cache_invalidate);
-int iommu_sva_bind_gpasid(struct iommu_domain *domain,
- struct device *dev, struct iommu_gpasid_bind_data
*data)
+int iommu_uapi_sva_bind_gpasid(struct iommu_domain *domain,
+ struct device *dev, struct
iommu_gpasid_bind_data *data)
{
if (unlikely(!domain->ops->sva_bind_gpasid))
return -ENODEV;
return domain->ops->sva_bind_gpasid(domain, dev, data);
}
-EXPORT_SYMBOL_GPL(iommu_sva_bind_gpasid);
+EXPORT_SYMBOL_GPL(iommu_uapi_sva_bind_gpasid);
-int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev,
-ioasid_t pasid)
+int iommu_uapi_sva_unbind_gpasid(struct iommu_domain *domain, struct device
*dev,
+ioasid_t pasid)
{
if (unlikely(!domain->ops->sva_unbind_gpasid))
return -ENODEV;
return domain->ops->sva_unbind_gpasid(dev, pasid);
}
-EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid);
+EXPORT_SYMBOL_GPL(iommu_uapi_sva_unbind_gpasid);
static void __iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index fee209efb756..710d5d2691eb 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -424,13 +424,13 @@ extern int iommu_attach_device(struct iommu_domain
*domain,
struct device *dev);
extern void iommu_detach_device(struct iommu_domain *domain,
struct device *dev);
-extern int iommu_cache_invalidate(struct iommu_domain *domain,
- struct device *dev,
- struct iommu_cache_invalidate_info *inv_info);
-extern int iommu_sva_bind_gpasid(struct iommu_domain *domain,
- struct device *dev, struct iommu_gpasid_bind_data *data);
-extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain,
- struct device *dev, ioasid_t pasid);
+extern int iommu_uapi_cache_invalidate(struct iommu_domain *domain,
+ struct device *dev,
+ struct iommu_cache_invalidate_info
*inv_info);
+extern int iommu_uapi_sva_bind_gpasid(struct iommu_domain *domain,
+ struct device *dev, struct
iommu_gpasid_bind_data *data);
+extern int iommu_uapi_sva_unbind_gpasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1032,21 +1032,22 @@ static inline int iommu_sva_get_pasid(struct iommu_sva
*handle)
return IOMMU_PASID_INVALID;
}
-static inline int
-iommu_cache_invalidate(struct iommu_domain *domain,
- struct device *dev,
- struct iommu_cache_invalidate_info *inv_info)
+static inline int iommu_uapi_cache_invalidate(struct iommu_domain *domain,
+ struct device *dev,
+ struct
iommu_cache_invalidate_info *inv_info)
{
return -ENODEV;
}
-static inline int iom
[PATCH v10 2/7] iommu/uapi: Add argsz for user filled data
As IOMMU UAPI gets extended, user data size may increase. To support
backward compatibiliy, this patch introduces a size field to each UAPI
data structures. It is *always* the responsibility for the user to fill in
the correct size. Padding fields are adjusted to ensure 8 byte alignment.
Specific scenarios for user data handling are documented in:
Documentation/userspace-api/iommu.rst
As there is no current users of the API, struct version is not
incremented.
Reviewed-by: Eric Auger
Signed-off-by: Liu Yi L
Signed-off-by: Jacob Pan
---
include/uapi/linux/iommu.h | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index c2b2caf9ed41..b42acc8fe007 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -139,6 +139,7 @@ enum iommu_page_response_code {
/**
* struct iommu_page_response - Generic page response information
+ * @argsz: User filled size of this data
* @version: API version of this structure
* @flags: encodes whether the corresponding fields are valid
* (IOMMU_FAULT_PAGE_RESPONSE_* values)
@@ -147,6 +148,7 @@ enum iommu_page_response_code {
* @code: response code from &enum iommu_page_response_code
*/
struct iommu_page_response {
+ __u32 argsz;
#define IOMMU_PAGE_RESP_VERSION_1 1
__u32 version;
#define IOMMU_PAGE_RESP_PASID_VALID(1 << 0)
@@ -222,6 +224,7 @@ struct iommu_inv_pasid_info {
/**
* struct iommu_cache_invalidate_info - First level/stage invalidation
* information
+ * @argsz: User filled size of this data
* @version: API version of this structure
* @cache: bitfield that allows to select which caches to invalidate
* @granularity: defines the lowest granularity used for the invalidation:
@@ -250,6 +253,7 @@ struct iommu_inv_pasid_info {
* must support the used granularity.
*/
struct iommu_cache_invalidate_info {
+ __u32 argsz;
#define IOMMU_CACHE_INVALIDATE_INFO_VERSION_1 1
__u32 version;
/* IOMMU paging structure cache */
@@ -259,7 +263,7 @@ struct iommu_cache_invalidate_info {
#define IOMMU_CACHE_INV_TYPE_NR(3)
__u8cache;
__u8granularity;
- __u8padding[2];
+ __u8padding[6];
union {
struct iommu_inv_pasid_info pasid_info;
struct iommu_inv_addr_info addr_info;
@@ -296,6 +300,7 @@ struct iommu_gpasid_bind_data_vtd {
/**
* struct iommu_gpasid_bind_data - Information about device and guest PASID
binding
+ * @argsz: User filled size of this data
* @version: Version of this data structure
* @format:PASID table entry format
* @flags: Additional information on guest bind request
@@ -313,17 +318,18 @@ struct iommu_gpasid_bind_data_vtd {
* PASID to host PASID based on this bind data.
*/
struct iommu_gpasid_bind_data {
+ __u32 argsz;
#define IOMMU_GPASID_BIND_VERSION_11
__u32 version;
#define IOMMU_PASID_FORMAT_INTEL_VTD 1
__u32 format;
+ __u32 addr_width;
#define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID valid */
__u64 flags;
__u64 gpgd;
__u64 hpasid;
__u64 gpasid;
- __u32 addr_width;
- __u8 padding[12];
+ __u8 padding[8];
/* Vendor specific data */
union {
struct iommu_gpasid_bind_data_vtd vtd;
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v10 3/7] iommu/uapi: Introduce enum type for PASID data format
There can be multiple vendor-specific PASID data formats used in UAPI
structures. This patch adds enum type with a last entry which makes
range checking much easier.
Suggested-by: Alex Williamson
Reviewed-by: Eric Auger
Signed-off-by: Jacob Pan
---
include/uapi/linux/iommu.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index b42acc8fe007..7cc6ee6c41f7 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -298,11 +298,16 @@ struct iommu_gpasid_bind_data_vtd {
IOMMU_SVA_VTD_GPASID_PCD | \
IOMMU_SVA_VTD_GPASID_PWT)
+enum iommu_pasid_data_format {
+ IOMMU_PASID_FORMAT_INTEL_VTD = 1,
+ IOMMU_PASID_FORMAT_LAST,
+};
+
/**
* struct iommu_gpasid_bind_data - Information about device and guest PASID
binding
* @argsz: User filled size of this data
* @version: Version of this data structure
- * @format:PASID table entry format
+ * @format:PASID table entry format of enum iommu_pasid_data_format type
* @flags: Additional information on guest bind request
* @gpgd: Guest page directory base of the guest mm to bind
* @hpasid:Process address space ID used for the guest mm in host IOMMU
@@ -321,7 +326,6 @@ struct iommu_gpasid_bind_data {
__u32 argsz;
#define IOMMU_GPASID_BIND_VERSION_11
__u32 version;
-#define IOMMU_PASID_FORMAT_INTEL_VTD 1
__u32 format;
__u32 addr_width;
#define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID valid */
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v10 7/7] iommu/vt-d: Check UAPI data processed by IOMMU core
IOMMU generic layer already does sanity checks on UAPI data for version
match and argsz range based on generic information.
This patch adjusts the following data checking responsibilities:
- removes the redundant version check from VT-d driver
- removes the check for vendor specific data size
- adds check for the use of reserved/undefined flags
Signed-off-by: Jacob Pan
---
drivers/iommu/intel/iommu.c | 3 +--
drivers/iommu/intel/svm.c | 11 +--
include/uapi/linux/iommu.h | 1 +
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 461f3a6864d4..18ed3b3c70d7 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -5408,8 +5408,7 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
int ret = 0;
u64 size = 0;
- if (!inv_info || !dmar_domain ||
- inv_info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
+ if (!inv_info || !dmar_domain)
return -EINVAL;
if (!dev || !dev_is_pci(dev))
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 99353d6468fa..0cb9a15f1112 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -284,8 +284,15 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain,
struct device *dev,
if (WARN_ON(!iommu) || !data)
return -EINVAL;
- if (data->version != IOMMU_GPASID_BIND_VERSION_1 ||
- data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
+ if (data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
+ return -EINVAL;
+
+ /* IOMMU core ensures argsz is more than the start of the union */
+ if (data->argsz < offsetofend(struct iommu_gpasid_bind_data,
vendor.vtd))
+ return -EINVAL;
+
+ /* Make sure no undefined flags are used in vendor data */
+ if (data->vendor.vtd.flags & ~(IOMMU_SVA_VTD_GPASID_LAST - 1))
return -EINVAL;
if (!dev_is_pci(dev))
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index c64bca5af419..1ebc23df4fbc 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -288,6 +288,7 @@ struct iommu_gpasid_bind_data_vtd {
#define IOMMU_SVA_VTD_GPASID_PWT (1 << 3) /* page-level write through */
#define IOMMU_SVA_VTD_GPASID_EMTE (1 << 4) /* extended mem type enable */
#define IOMMU_SVA_VTD_GPASID_CD(1 << 5) /* PASID-level cache
disable */
+#define IOMMU_SVA_VTD_GPASID_LAST (1 << 6)
__u64 flags;
__u32 pat;
__u32 emt;
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v10 1/7] docs: IOMMU user API
IOMMU UAPI is newly introduced to support communications between guest virtual IOMMU and host IOMMU. There has been lots of discussions on how it should work with VFIO UAPI and userspace in general. This document is intended to clarify the UAPI design and usage. The mechanics of how future extensions should be achieved are also covered in this documentation. Cc: linux-...@vger.kernel.org Cc: Jonathan Corbet Cc: linux-...@vger.kernel.org Reviewed-by: Eric Auger Signed-off-by: Liu Yi L Signed-off-by: Jacob Pan --- Documentation/userspace-api/iommu.rst | 210 ++ MAINTAINERS | 1 + 2 files changed, 211 insertions(+) create mode 100644 Documentation/userspace-api/iommu.rst diff --git a/Documentation/userspace-api/iommu.rst b/Documentation/userspace-api/iommu.rst new file mode 100644 index ..591609863cdc --- /dev/null +++ b/Documentation/userspace-api/iommu.rst @@ -0,0 +1,210 @@ +.. SPDX-License-Identifier: GPL-2.0 +.. iommu: + += +IOMMU Userspace API += + +IOMMU UAPI is used for virtualization cases where communications are +needed between physical and virtual IOMMU drivers. For baremetal +usage, the IOMMU is a system device which does not need to communicate +with userspace directly. + +The primary use cases are guest Shared Virtual Address (SVA) and +guest IO virtual address (IOVA), wherein the vIOMMU implementation +relies on the physical IOMMU and for this reason requires interactions +with the host driver. + +.. contents:: :local: + +Functionalities +=== +Communications of user and kernel involve both directions. The +supported user-kernel APIs are as follows: + +1. Alloc/Free PASID +2. Bind/Unbind guest PASID (e.g. Intel VT-d) +3. Bind/Unbind guest PASID table (e.g. ARM SMMU) +4. Invalidate IOMMU caches upon guest requests +5. Report errors to the guest and serve page requests + +Requirements + +The IOMMU UAPIs are generic and extensible to meet the following +requirements: + +1. Emulated and para-virtualised vIOMMUs +2. Multiple vendors (Intel VT-d, ARM SMMU, etc.) +3. Extensions to the UAPI shall not break existing userspace + +Interfaces +== +Although the data structures defined in IOMMU UAPI are self-contained, +there are no user API functions introduced. Instead, IOMMU UAPI is +designed to work with existing user driver frameworks such as VFIO. + +Extension Rules & Precautions +- +When IOMMU UAPI gets extended, the data structures can *only* be +modified in two ways: + +1. Adding new fields by re-purposing the padding[] field. No size change. +2. Adding new union members at the end. May increase the structure sizes. + +No new fields can be added *after* the variable sized union in that it +will break backward compatibility when offset moves. A new flag must +be introduced whenever a change affects the structure using either +method. The IOMMU driver processes the data based on flags which +ensures backward compatibility. + +Version field is only reserved for the unlikely event of UAPI upgrade +at its entirety. + +It's *always* the caller's responsibility to indicate the size of the +structure passed by setting argsz appropriately. +Though at the same time, argsz is user provided data which is not +trusted. The argsz field allows the user app to indicate how much data +it is providing; it's still the kernel's responsibility to validate +whether it's correct and sufficient for the requested operation. + +Compatibility Checking +-- +When IOMMU UAPI extension results in some structure size increase, +IOMMU UAPI code shall handle the following cases: + +1. User and kernel has exact size match +2. An older user with older kernel header (smaller UAPI size) running on a + newer kernel (larger UAPI size) +3. A newer user with newer kernel header (larger UAPI size) running + on an older kernel. +4. A malicious/misbehaving user passing illegal/invalid size but within + range. The data may contain garbage. + +Feature Checking + +While launching a guest with vIOMMU, it is strongly advised to check +the compatibility upfront, as some subsequent errors happening during +vIOMMU operation, such as cache invalidation failures cannot be nicely +escalated to the guest due to IOMMU specifications. This can lead to +catastrophic failures for the users. + +User applications such as QEMU are expected to import kernel UAPI +headers. Backward compatibility is supported per feature flags. +For example, an older QEMU (with older kernel header) can run on newer +kernel. Newer QEMU (with new kernel header) may refuse to initialize +on an older kernel if new feature flags are not supported by older +kernel. Simply recompiling existing code with newer kernel header should +not be an issue in that only existing flags are used. + +IOMMU vendor driver should report the below features
Re: [PATCH v9 3/7] iommu/uapi: Introduce enum type for PASID data format
Hi Joerg,
I sent out v10 with Randy's comments addressed but I didn't change this
patch. Does my explanation below make sense? I am hoping to make it in
v5.10 since many other pieces depend on it, your guidance is much
appreciated.
Jacob
On Fri, 18 Sep 2020 10:11:08 -0700, Jacob Pan
wrote:
> Hi Joerg,
>
> On Fri, 18 Sep 2020 11:44:50 +0200, Joerg Roedel wrote:
>
> > On Fri, Sep 11, 2020 at 02:57:52PM -0700, Jacob Pan wrote:
> > > There can be multiple vendor-specific PASID data formats used in UAPI
> > > structures. This patch adds enum type with a last entry which makes
> > > range checking much easier.
> >
> > But it also makes it much easier to screw up the numbers (which are ABI)
> > by inserting a new value into the middle. I prefer defines here, or
> > alternativly BUILD_BUG_ON() checks for the numbers.
> >
> I am not following, the purpose of IOMMU_PASID_FORMAT_LAST *is* for
> preparing the future insertion of new value into the middle.
> The checking against IOMMU_PASID_FORMAT_LAST is to protect ABI
> compatibility by making sure that out of range format are rejected in all
> versions of the ABI.
> For example, in v5.10, ABI has IOMMU_PASID_FORMAT_LAST = 2, then user data
> with format = 2 will be rejected. So this user app will not work or
> released.
>
> Now say in v5.11, we add one more format in the middle and set
> IOMMU_PASID_FORMAT_LAST = 3. Then user data with the new format = 2 can
> be supported.
>
> Without the checking for IOMMU_PASID_FORMAT_LAST, at v5.10 time the user
> binary may succeed and become legacy binary that we cannot break in v5.11.
> This renders format = 2 unusable for v5.11.
>
> I thought enum makes it less susceptible to programming errors than
> defines by making sure the ascending order. I might have missed your
> point, could you elaborate?
>
> > Regards,
> >
> > Joerg
> >
> > >
> > > Suggested-by: Alex Williamson
> > > Reviewed-by: Eric Auger
> > > Signed-off-by: Jacob Pan
> > > ---
> > > include/uapi/linux/iommu.h | 8 ++--
> > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> > > index b42acc8fe007..7cc6ee6c41f7 100644
> > > --- a/include/uapi/linux/iommu.h
> > > +++ b/include/uapi/linux/iommu.h
> > > @@ -298,11 +298,16 @@ struct iommu_gpasid_bind_data_vtd {
> > >IOMMU_SVA_VTD_GPASID_PCD |
> > > \ IOMMU_SVA_VTD_GPASID_PWT)
> > >
> > > +enum iommu_pasid_data_format {
> > > + IOMMU_PASID_FORMAT_INTEL_VTD = 1,
> > > + IOMMU_PASID_FORMAT_LAST,
> > > +};
> > > +
> > > /**
> > > * struct iommu_gpasid_bind_data - Information about device and guest
> > > PASID binding
> > > * @argsz: User filled size of this data
> > > * @version: Version of this data structure
> > > - * @format: PASID table entry format
> > > + * @format: PASID table entry format of enum
> > > iommu_pasid_data_format type
> > > * @flags: Additional information on guest bind request
> > > * @gpgd:Guest page directory base of the guest mm to bind
> > > * @hpasid: Process address space ID used for the guest mm in
> > > host IOMMU @@ -321,7 +326,6 @@ struct iommu_gpasid_bind_data {
> > > __u32 argsz;
> > > #define IOMMU_GPASID_BIND_VERSION_1 1
> > > __u32 version;
> > > -#define IOMMU_PASID_FORMAT_INTEL_VTD 1
> > > __u32 format;
> > > __u32 addr_width;
> > > #define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID valid */
> > > --
> > > 2.7.4
>
>
> Thanks,
>
> Jacob
Thanks,
Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 05/38] drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays()
Hi Marek,
On Tue, 22 Sep 2020, Marek Szyprowski wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Alex,
>
> On 22.09.2020 01:15, Alex Goins wrote:
> > Tested-by: Alex Goins
> >
> > This change fixes a regression with drm_prime_sg_to_page_addr_arrays() and
> > AMDGPU in v5.9.
>
> Thanks for testing!
>
> > Commit 39913934 similarly revamped AMDGPU to use sgtable helper functions.
> > When
> > it changed from dma_map_sg_attrs() to dma_map_sgtable(), as a side effect it
> > started correctly updating sgt->nents to the return value of
> > dma_map_sg_attrs().
> > However, drm_prime_sg_to_page_addr_arrays() incorrectly uses sgt->nents to
> > iterate over pages, rather than sgt->orig_nents, resulting in it now
> > returning
> > the incorrect number of pages on AMDGPU.
> >
> > I had written a patch that changes drm_prime_sg_to_page_addr_arrays() to use
> > for_each_sgtable_sg() instead of for_each_sg(), iterating using
> > sgt->orig_nents:
> >
> > - for_each_sg(sgt->sgl, sg, sgt->nents, count) {
> > + for_each_sgtable_sg(sgt, sg, count) {
> >
> > This patch takes it further, but still has the effect of fixing the number
> > of
> > pages that drm_prime_sg_to_page_addr_arrays() returns. Something like this
> > should be included in v5.9 to prevent a regression with AMDGPU.
>
> Probably the easiest way to handle a fix for v5.9 would be to simply
> merge the latest version of this patch also to v5.9-rcX:
> https://lore.kernel.org/dri-devel/20200904131711.12950-3-m.szyprow...@samsung.com/
Tested-by: Alex Goins that version too.
>
> This way we would get it fixed and avoid possible conflict in the -next.
> Do you have any AMDGPU fixes for v5.9 in the queue? Maybe you can add that
> patch to the queue?
I don't have any more AMDGPU fixes, just want to ensure that this makes it in.
Thanks,
Alex
> Dave: would it be okay that way?
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/4] iommu/vt-d: Report out when IOMMU features have inconsistencies
IOMMU features as below can have incompatibilities among IOMMUs.
Audit IOMMU Capability/Extended Capability and check if the IOMMUs have
the consistent value for features as below. Report out when features as
below have incompatibility among IOMMUs.
Report out features when below features are mismatched:
- First Level 5 Level Paging Support (FL5LP)
- First Level 1 GByte Page Support (FL1GP)
- Read Draining (DRD)
- Write Draining (DWD)
- Page Selective Invalidation (PSI)
- Caching Mode (CM)
- Protected High/Low-Memory Region (PHMR/PLMR)
- Scalable Mode Page Walk Coherency (SMPWC)
- First Level Translation Support (FLTS)
- Second Level Translation Support (SLTS)
- Second Level Accessed/Dirty Support (SLADS)
- Virtual Command Support (VCS)
- Scalable Mode Translation Support (SMTS)
- Device TLB Invalidation Throttle (DIT)
- Page Drain Support (PDS)
- Nested Translation Support (NEST)
- Snoop Control (SC)
- Pass Through (PT)
- Device TLB Support (DT)
- Queued Invalidation (QI)
- Page walk Coherency (C)
Signed-off-by: Kyung Min Park
---
drivers/iommu/intel/audit.c | 48 +
drivers/iommu/intel/audit.h | 43 +
include/linux/intel-iommu.h | 2 ++
3 files changed, 93 insertions(+)
diff --git a/drivers/iommu/intel/audit.c b/drivers/iommu/intel/audit.c
index 2893170f5b6c..f783acabb402 100644
--- a/drivers/iommu/intel/audit.c
+++ b/drivers/iommu/intel/audit.c
@@ -13,6 +13,8 @@
#include "audit.h"
static bool svm_sanity_check = true;
+/* global variables that hold feature consistency and minimum features */
+static u64 intel_iommu_cap_sanity = ~0ULL;
static u64 intel_iommu_ecap_sanity = ~0ULL;
static void set_cap_audit_svm_sanity(bool svm_sanity)
@@ -30,12 +32,58 @@ static inline void check_dmar_capabilities(struct
intel_iommu *a,
{
if (MINIMAL_SVM_ECAP & (a->ecap ^ b->ecap))
set_cap_audit_svm_sanity(false);
+
+ CHECK_FEATURE_MISMATCH(a, b, cap, 5lp_support, CAP_FL5LP_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, cap, fl1gp_support, CAP_FL1GP_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, cap, read_drain, CAP_RD_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, cap, write_drain, CAP_WD_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, cap, pgsel_inv, CAP_PSI_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, cap, caching_mode, CAP_CM_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, cap, phmr, CAP_PHMR_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, cap, plmr, CAP_PLMR_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, smpwc, ECAP_SMPWC_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, flts, ECAP_FLTS_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, slts, ECAP_SLTS_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, slads, ECAP_SLADS_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, vcs, ECAP_VCS_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, smts, ECAP_SMTS_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, pds, ECAP_PDS_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, dit, ECAP_DIT_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, nest, ECAP_NEST_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, sc_support, ECAP_SC_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, pass_through, ECAP_PT_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, dev_iotlb_support, ECAP_DT_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, qis, ECAP_QI_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, coherent, ECAP_C_MASK);
}
static int audit_iommu_capabilities_hotplug(struct intel_iommu *hot_iommu)
{
bool mismatch = false;
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, 5lp_support,
CAP_FL5LP_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, fl1gp_support,
CAP_FL1GP_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, read_drain, CAP_RD_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, write_drain,
CAP_WD_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, pgsel_inv, CAP_PSI_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, caching_mode,
CAP_CM_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, phmr, CAP_PHMR_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, plmr, CAP_PLMR_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, smpwc, ECAP_SMPWC_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, flts, ECAP_FLTS_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, slts, ECAP_SLTS_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, slads, ECAP_SLADS_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, vcs, ECAP_VCS_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, smts, ECAP_SMTS_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, pds, ECAP_PDS_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, dit, ECAP_DIT_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, nest, ECAP_NEST_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu,
[PATCH 4/4] iommu/vt-d: Scale capability to the lowest supported between the IOMMUs
Audit IOMMU Capability/Extended Capabilities and check if the IOMMUs
have the consistent value for features as below. Find common denominator
for the features and set to the lowest supported value for each IOMMU.
Abort hot plug when the hot plugged IOMMU does not meet the aforementioned
common denominator.
Set capability to the lowest supported when below features are mismatched:
- Maximum Address Mask Value (MAMV)
- Second Level Large Page Support (SLLPS)
- Maximum Guest Address Width (MGAW)
- Supported Adjusted Guest Address Width (SAGAW)
- Number of Domains supported (ND)
- Pasid Size Supported (PSS)
Signed-off-by: Kyung Min Park
---
drivers/iommu/intel/audit.c | 23 +++
drivers/iommu/intel/audit.h | 27 +++
include/linux/intel-iommu.h | 1 +
3 files changed, 51 insertions(+)
diff --git a/drivers/iommu/intel/audit.c b/drivers/iommu/intel/audit.c
index e005bc61770a..7e12c963c2b7 100644
--- a/drivers/iommu/intel/audit.c
+++ b/drivers/iommu/intel/audit.c
@@ -40,6 +40,13 @@ static inline void check_dmar_capabilities(struct
intel_iommu *a,
if (MINIMAL_SVM_ECAP & (a->ecap ^ b->ecap))
set_cap_audit_svm_sanity(false);
+ MINIMAL_FEATURE_IOMMU(b, cap, CAP_MAMV_MASK);
+ MINIMAL_FEATURE_IOMMU(b, cap, CAP_SLLPS_MASK);
+ MINIMAL_FEATURE_IOMMU(b, cap, CAP_MGAW_MASK);
+ MINIMAL_FEATURE_IOMMU(b, cap, CAP_SAGAW_MASK);
+ MINIMAL_FEATURE_IOMMU(b, cap, CAP_NDOMS_MASK);
+ MINIMAL_FEATURE_IOMMU(b, ecap, ECAP_PSS_MASK);
+
CHECK_FEATURE_MISMATCH(a, b, cap, 5lp_support, CAP_FL5LP_MASK);
CHECK_FEATURE_MISMATCH(a, b, cap, fl1gp_support, CAP_FL1GP_MASK);
CHECK_FEATURE_MISMATCH(a, b, cap, read_drain, CAP_RD_MASK);
@@ -98,6 +105,14 @@ static int audit_iommu_capabilities_hotplug(struct
intel_iommu *hot_iommu,
CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, qis, ECAP_QI_MASK);
CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, coherent, ECAP_C_MASK);
+ /* Abort hot plug if the hot plug iommu feature is smaller than global
*/
+ MINIMAL_FEATURE_HOTPLUG(hot_iommu, cap, mamv, CAP_MAMV_MASK, mismatch);
+ MINIMAL_FEATURE_HOTPLUG(hot_iommu, cap, super_page_val, CAP_SLLPS_MASK,
mismatch);
+ MINIMAL_FEATURE_HOTPLUG(hot_iommu, cap, mgaw, CAP_MGAW_MASK, mismatch);
+ MINIMAL_FEATURE_HOTPLUG(hot_iommu, cap, sagaw, CAP_SAGAW_MASK,
mismatch);
+ MINIMAL_FEATURE_HOTPLUG(hot_iommu, cap, ndoms, CAP_NDOMS_MASK,
mismatch);
+ MINIMAL_FEATURE_HOTPLUG(hot_iommu, ecap, pss, ECAP_PSS_MASK, mismatch);
+
if (!IS_ENABLED(CONFIG_INTEL_IOMMU_SVM))
goto out;
@@ -147,6 +162,14 @@ static int audit_iommu_capabilities(bool audit_irq)
intel_iommu_ecap_sanity = (intel_iommu_ecap_sanity &
~MINIMAL_SVM_ECAP) |
(first_drhd->iommu->ecap &
MINIMAL_SVM_ECAP);
+ /* scale capability to the lowest supported value */
+ for_each_active_iommu(iommu, drhd) {
+ iommu->cap = (intel_iommu_cap_sanity & MINIMAL_FEATURE_CAP) |
+(~MINIMAL_FEATURE_CAP & iommu->cap);
+ iommu->ecap = (intel_iommu_ecap_sanity & ECAP_PSS_MASK) |
+ (~ECAP_PSS_MASK & iommu->ecap);
+ }
+
ret = 0;
out:
rcu_read_unlock();
diff --git a/drivers/iommu/intel/audit.h b/drivers/iommu/intel/audit.h
index 6dfebe8e8fbe..a293e71ce9ab 100644
--- a/drivers/iommu/intel/audit.h
+++ b/drivers/iommu/intel/audit.h
@@ -13,9 +13,14 @@
#define CAP_FL5LP_MASK BIT(60)
#define CAP_PI_MASKBIT(59)
#define CAP_FL1GP_MASK BIT(56)
+#define CAP_MAMV_MASK GENMASK_ULL(53, 48)
#define CAP_RD_MASKBIT(55)
#define CAP_WD_MASKBIT(54)
#define CAP_PSI_MASK BIT(39)
+#define CAP_SLLPS_MASK GENMASK_ULL(37, 34)
+#define CAP_MGAW_MASK GENMASK_ULL(21, 16)
+#define CAP_SAGAW_MASK GENMASK_ULL(12, 8)
+#define CAP_NDOMS_MASK GENMASK_ULL(2, 0)
#define CAP_CM_MASKBIT(7)
#define CAP_PHMR_MASK BIT(6)
#define CAP_PLMR_MASK BIT(5)
@@ -32,6 +37,7 @@
#define ECAP_PDS_MASK BIT(42)
#define ECAP_DIT_MASK BIT(41)
#define ECAP_PASID_MASKBIT(40)
+#define ECAP_PSS_MASK GENMASK_ULL(39, 35)
#define ECAP_EAFS_MASK BIT(34)
#define ECAP_SRS_MASK BIT(31)
#define ECAP_ERS_MASK BIT(30)
@@ -47,6 +53,9 @@
#define MINIMAL_SVM_ECAP (ECAP_FLTS_MASK | ECAP_PASID_MASK | ECAP_EAFS_MASK | \
ECAP_SRS_MASK | ECAP_ERS_MASK | ECAP_PRS_MASK)
+#define MINIMAL_FEATURE_CAP (CAP_MAMV_MASK | CAP_SLLPS_MASK | CAP_MGAW_MASK | \
+CAP_SAGAW_MASK | CAP_NDOMS_MASK)
+
#define DO_CHECK_FEATURE_MISMATCH(a, b, cap, feature, MASK) \
do { \
if (cap##_##feature(a) != cap##_##feature(b)) { \
@@ -65,6 +74,24 @@ do { \
[PATCH 3/4] iommu/vt-d: Audit IOMMUs for Interrupt Remapping features
Audit IOMMU Capability/Extended Capabilities for Interrupt Remapping.
Check if the IOMMUs have the consistent value for the features as below.
When the features are not matched among IOMMUs, report out the IOMMU
features during irq remapping initialization. Audit IOMMUs again
when a device is hot plugged.
Report out features when below features are mismatched:
- Posted Interrupts (PI)
- Extended Interrupt Mode (EIM)
Signed-off-by: Kyung Min Park
---
drivers/iommu/intel/Makefile| 2 +-
drivers/iommu/intel/audit.c | 39 -
drivers/iommu/intel/audit.h | 4 +++
drivers/iommu/intel/irq_remapping.c | 8 ++
4 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/intel/Makefile b/drivers/iommu/intel/Makefile
index 02c26acb479f..b5760c4abc54 100644
--- a/drivers/iommu/intel/Makefile
+++ b/drivers/iommu/intel/Makefile
@@ -4,4 +4,4 @@ obj-$(CONFIG_INTEL_IOMMU) += iommu.o pasid.o audit.o
obj-$(CONFIG_INTEL_IOMMU) += trace.o
obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += debugfs.o
obj-$(CONFIG_INTEL_IOMMU_SVM) += svm.o
-obj-$(CONFIG_IRQ_REMAP) += irq_remapping.o
+obj-$(CONFIG_IRQ_REMAP) += irq_remapping.o audit.o
diff --git a/drivers/iommu/intel/audit.c b/drivers/iommu/intel/audit.c
index f783acabb402..e005bc61770a 100644
--- a/drivers/iommu/intel/audit.c
+++ b/drivers/iommu/intel/audit.c
@@ -27,6 +27,13 @@ bool get_cap_audit_svm_sanity(void)
return svm_sanity_check;
}
+static inline void check_irq_capabilities(struct intel_iommu *a,
+ struct intel_iommu *b)
+{
+ CHECK_FEATURE_MISMATCH(a, b, cap, pi_support, CAP_PI_MASK);
+ CHECK_FEATURE_MISMATCH(a, b, ecap, eim_support, ECAP_EIM_MASK);
+}
+
static inline void check_dmar_capabilities(struct intel_iommu *a,
struct intel_iommu *b)
{
@@ -57,10 +64,17 @@ static inline void check_dmar_capabilities(struct
intel_iommu *a,
CHECK_FEATURE_MISMATCH(a, b, ecap, coherent, ECAP_C_MASK);
}
-static int audit_iommu_capabilities_hotplug(struct intel_iommu *hot_iommu)
+static int audit_iommu_capabilities_hotplug(struct intel_iommu *hot_iommu,
+ bool audit_irq)
{
bool mismatch = false;
+ if (audit_irq) {
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, pi_support,
CAP_PI_MASK);
+ CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, ecap, eim_support,
ECAP_EIM_MASK);
+ goto out;
+ }
+
CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, 5lp_support,
CAP_FL5LP_MASK);
CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, fl1gp_support,
CAP_FL1GP_MASK);
CHECK_FEATURE_MISMATCH_HOTPLUG(hot_iommu, cap, read_drain, CAP_RD_MASK);
@@ -103,7 +117,7 @@ static int audit_iommu_capabilities_hotplug(struct
intel_iommu *hot_iommu)
return 0;
}
-static int audit_iommu_capabilities(void)
+static int audit_iommu_capabilities(bool audit_irq)
{
struct dmar_drhd_unit *first_drhd, *drhd;
struct intel_iommu *iommu;
@@ -117,8 +131,17 @@ static int audit_iommu_capabilities(void)
goto out;
}
- for_each_active_iommu(iommu, drhd)
- check_dmar_capabilities(first_drhd->iommu, iommu);
+ for_each_active_iommu(iommu, drhd) {
+ if (audit_irq)
+ check_irq_capabilities(first_drhd->iommu, iommu);
+ else
+ check_dmar_capabilities(first_drhd->iommu, iommu);
+ }
+
+ if (audit_irq) {
+ ret = 0;
+ goto out;
+ }
if (get_cap_audit_svm_sanity())
intel_iommu_ecap_sanity = (intel_iommu_ecap_sanity &
~MINIMAL_SVM_ECAP) |
@@ -134,9 +157,13 @@ int intel_iommu_audit_capabilities(enum cap_audit_type
type, struct intel_iommu
{
switch (type) {
case CAP_AUDIT_STATIC_DMAR:
- return audit_iommu_capabilities();
+ return audit_iommu_capabilities(false);
+ case CAP_AUDIT_STATIC_IRQR:
+ return audit_iommu_capabilities(true);
case CAP_AUDIT_HOTPLUG_DMAR:
- return audit_iommu_capabilities_hotplug(iommu);
+ return audit_iommu_capabilities_hotplug(iommu, false);
+ case CAP_AUDIT_HOTPLUG_IRQR:
+ return audit_iommu_capabilities_hotplug(iommu, true);
default:
return -EFAULT;
}
diff --git a/drivers/iommu/intel/audit.h b/drivers/iommu/intel/audit.h
index e3a370405f82..6dfebe8e8fbe 100644
--- a/drivers/iommu/intel/audit.h
+++ b/drivers/iommu/intel/audit.h
@@ -11,6 +11,7 @@
* Capability Register Mask
*/
#define CAP_FL5LP_MASK BIT(60)
+#define CAP_PI_MASKBIT(59)
#define CAP_FL1GP_MASK BIT(56)
#define CAP_RD_MASKBIT(55)
#define CAP_WD_MASKBIT(54)
@@ -38,6 +39,7 @@
#define ECAP_NEST_MASK BIT(26)
#define ECAP_SC_MASK
[PATCH 0/4] Audit Capability and Extended Capability among IOMMUs
Modern platforms have more than one IOMMU. Each IOMMU has its own feature set. Some of these features must be consistent among IOMMUs. Otherwise, these differences can lead to improper behavior in the system. On the other hand, for some features, each IOMMU can have different capacity values. So, different actions are required to deal with the inconsistencies depending on the IOMMU features. Currently, some inconsistencies are ignored by the IOMMU driver. This patchset checks IOMMU capabilities and extended capabilities centralizedly during boot and take different actions according to the impacts caused by the mismatches. For example: 1. Disable Shared Virtual Memory. 2. Use common capacity values (normally the lowest capacity value) for all IOMMUs. 3. Report feature mismatches. Detailed information on the IOMMU Capability / Extended Capability can be found in Intel VT-d Specification. Link: https://software.intel.com/sites/default/files/managed/c5/15/vt-directed-io-spec.pdf Kyung Min Park (4): iommu/vt-d: Disable SVM in the platform when IOMMUs have inconsistencies iommu/vt-d: Report out when IOMMU features have inconsistencies iommu/vt-d: Audit IOMMUs for Interrupt Remapping features iommu/vt-d: Scale capability to the lowest supported between the IOMMUs drivers/iommu/intel/Makefile| 4 +- drivers/iommu/intel/audit.c | 193 drivers/iommu/intel/audit.h | 103 +++ drivers/iommu/intel/iommu.c | 12 +- drivers/iommu/intel/irq_remapping.c | 8 ++ include/linux/intel-iommu.h | 3 + 6 files changed, 320 insertions(+), 3 deletions(-) create mode 100644 drivers/iommu/intel/audit.c create mode 100644 drivers/iommu/intel/audit.h -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/4] iommu/vt-d: Disable SVM in the platform when IOMMUs have inconsistencies
Some IOMMU Capabilities must be consistent for Shared Virtual Memory (SVM).
Audit IOMMU Capability/Extended Capabilities and check if IOMMUs have
the consistent value for features as below. When the features are not
matched among IOMMUs, disable SVMs in the platform during DMAR
initialization. Audit IOMMUs again when a device is hot plugged.
Disable Shared Virtual Memory when below features are mistmatched:
- First Level Translation Support (FLTS)
- Process Address Space ID Support (PASID)
- Extended Accessed Flag Support (EAFS)
- Supervisor Support (SRS)
- Execute Request Support (ERS)
- Page Request Support (PRS)
Signed-off-by: Kyung Min Park
---
drivers/iommu/intel/Makefile | 2 +-
drivers/iommu/intel/audit.c | 95
drivers/iommu/intel/audit.h | 29 +++
drivers/iommu/intel/iommu.c | 12 -
4 files changed, 136 insertions(+), 2 deletions(-)
create mode 100644 drivers/iommu/intel/audit.c
create mode 100644 drivers/iommu/intel/audit.h
diff --git a/drivers/iommu/intel/Makefile b/drivers/iommu/intel/Makefile
index fb8e1e8c8029..02c26acb479f 100644
--- a/drivers/iommu/intel/Makefile
+++ b/drivers/iommu/intel/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_DMAR_TABLE) += dmar.o
-obj-$(CONFIG_INTEL_IOMMU) += iommu.o pasid.o
+obj-$(CONFIG_INTEL_IOMMU) += iommu.o pasid.o audit.o
obj-$(CONFIG_INTEL_IOMMU) += trace.o
obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += debugfs.o
obj-$(CONFIG_INTEL_IOMMU_SVM) += svm.o
diff --git a/drivers/iommu/intel/audit.c b/drivers/iommu/intel/audit.c
new file mode 100644
index ..2893170f5b6c
--- /dev/null
+++ b/drivers/iommu/intel/audit.c
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * audit.c - audit iommu capabilities for boot time and hot plug
+ *
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * Author: Kyung Min Park
+ */
+
+#define pr_fmt(fmt)"DMAR: " fmt
+
+#include
+#include "audit.h"
+
+static bool svm_sanity_check = true;
+static u64 intel_iommu_ecap_sanity = ~0ULL;
+
+static void set_cap_audit_svm_sanity(bool svm_sanity)
+{
+ svm_sanity_check = svm_sanity;
+}
+
+bool get_cap_audit_svm_sanity(void)
+{
+ return svm_sanity_check;
+}
+
+static inline void check_dmar_capabilities(struct intel_iommu *a,
+ struct intel_iommu *b)
+{
+ if (MINIMAL_SVM_ECAP & (a->ecap ^ b->ecap))
+ set_cap_audit_svm_sanity(false);
+}
+
+static int audit_iommu_capabilities_hotplug(struct intel_iommu *hot_iommu)
+{
+ bool mismatch = false;
+
+ if (!IS_ENABLED(CONFIG_INTEL_IOMMU_SVM))
+ goto out;
+
+ if (!get_cap_audit_svm_sanity() && (hot_iommu->flags &
VTD_FLAG_SVM_CAPABLE)) {
+ pr_warn("Disable SVM in the IOMMU: SVM disabled at boot
time.\n");
+ hot_iommu->flags = hot_iommu->flags & ~VTD_FLAG_SVM_CAPABLE;
+ } else if (get_cap_audit_svm_sanity() && (MINIMAL_SVM_ECAP &
+ (hot_iommu->ecap ^ intel_iommu_ecap_sanity))) {
+ pr_warn("Abort Hot Plug IOMMU: SVM inconsistent.\n");
+ mismatch = true;
+ }
+
+out:
+ if (mismatch)
+ return -EFAULT;
+
+ return 0;
+}
+
+static int audit_iommu_capabilities(void)
+{
+ struct dmar_drhd_unit *first_drhd, *drhd;
+ struct intel_iommu *iommu;
+ int ret = -EFAULT;
+
+ rcu_read_lock();
+ first_drhd = list_first_or_null_rcu(&dmar_drhd_units, typeof(*drhd),
+ list);
+ if (!first_drhd) {
+ ret = 0;
+ goto out;
+ }
+
+ for_each_active_iommu(iommu, drhd)
+ check_dmar_capabilities(first_drhd->iommu, iommu);
+
+ if (get_cap_audit_svm_sanity())
+ intel_iommu_ecap_sanity = (intel_iommu_ecap_sanity &
~MINIMAL_SVM_ECAP) |
+ (first_drhd->iommu->ecap &
MINIMAL_SVM_ECAP);
+
+ ret = 0;
+out:
+ rcu_read_unlock();
+ return ret;
+}
+
+int intel_iommu_audit_capabilities(enum cap_audit_type type, struct
intel_iommu *iommu)
+{
+ switch (type) {
+ case CAP_AUDIT_STATIC_DMAR:
+ return audit_iommu_capabilities();
+ case CAP_AUDIT_HOTPLUG_DMAR:
+ return audit_iommu_capabilities_hotplug(iommu);
+ default:
+ return -EFAULT;
+ }
+}
diff --git a/drivers/iommu/intel/audit.h b/drivers/iommu/intel/audit.h
new file mode 100644
index ..887900d9517d
--- /dev/null
+++ b/drivers/iommu/intel/audit.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * audit.h - audit iommu capabilities header
+ *
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * Author: Kyung Min Park
+ */
+
+/*
+ * Extended Capability Register Mask
+ */
+#define ECAP_FLTS_MASK BIT(47)
+#define ECAP_PASID_MASKBIT(40)
+#define ECAP_EAFS_MASK BIT(34)
+#define ECAP_SRS_MASK
Re: [PATCH v2] iommu/arm: Add module parameter to set msi iova address
Sure, that's a great suggestion, I'll rework on the patch and post again. Vennila On 9/21/2020 1:45 PM, Will Deacon wrote: On Mon, Sep 14, 2020 at 11:13:07AM -0700, Vennila Megavannan wrote: From: Srinath Mannam Add provision to change default value of MSI IOVA base to platform's suitable IOVA using module parameter. The present hardcoded MSI IOVA base may not be the accessible IOVA ranges of platform. If any platform has the limitaion to access default MSI IOVA, then it can be changed using "arm-smmu.msi_iova_base=0xa000" command line argument. Signed-off-by: Srinath Mannam Co-developed-by: Vennila Megavannan Signed-off-by: Vennila Megavannan --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 - drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 - 2 files changed, 8 insertions(+), 2 deletions(-) This feels pretty fragile. Wouldn't it be better to realise that there's a region conflict with iommu_dma_get_resv_regions() and move the MSI window accordingly at runtime? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] Revert "iommu/amd: Treat per-device exclusion ranges as r/w unity-mapped regions"
A regression failure of kdump kernel boot was reported on a HPE system.
Bisect points at commit 387caf0b759ac43 ("iommu/amd: Treat per-device
exclusion ranges as r/w unity-mapped regions") as criminal. Reverting it
fix the failure.
With the commit, kdump kernel will always print below error message, then
naturally AMD iommu can't function normally during kdump kernel bootup.
~
AMD-Vi: [Firmware Bug]: IVRS invalid checksum
Why commit 387caf0b759ac43 causing it haven't been made clear.
>From the commit log, a discussion thread link is pasted. In that discussion
thread, Adrian told the fix is for a system with already broken BIOS, and
Joerg suggested two options. Finally option 2) is taken. Maybe option 1)
should be the right approach?
1) Bail out and disable the IOMMU as the BIOS screwed up
2) Treat per-device exclusion ranges just as r/w unity-mapped
regions.
https://lists.linuxfoundation.org/pipermail/iommu/2019-November/040117.html
Signed-off-by: Baoquan He
---
drivers/iommu/amd/init.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 9aa1eae26634..bbe7ceae5949 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1109,17 +1109,22 @@ static int __init add_early_maps(void)
*/
static void __init set_device_exclusion_range(u16 devid, struct ivmd_header *m)
{
+ struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
+
if (!(m->flags & IVMD_FLAG_EXCL_RANGE))
return;
- /*
-* Treat per-device exclusion ranges as r/w unity-mapped regions
-* since some buggy BIOSes might lead to the overwritten exclusion
-* range (exclusion_start and exclusion_length members). This
-* happens when there are multiple exclusion ranges (IVMD entries)
-* defined in ACPI table.
-*/
- m->flags = (IVMD_FLAG_IW | IVMD_FLAG_IR | IVMD_FLAG_UNITY_MAP);
+ if (iommu) {
+ /*
+* We only can configure exclusion ranges per IOMMU, not
+* per device. But we can enable the exclusion range per
+* device. This is done here
+*/
+ set_dev_entry_bit(devid, DEV_ENTRY_EX);
+ iommu->exclusion_start = m->range_start;
+ iommu->exclusion_length = m->range_length;
+ }
+
}
/*
--
2.17.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] Revert "iommu/amd: Treat per-device exclusion ranges as r/w unity-mapped regions"
Forgot CC-ing Jerry, add him.
On 09/23/20 at 10:26am, Baoquan He wrote:
> A regression failure of kdump kernel boot was reported on a HPE system.
> Bisect points at commit 387caf0b759ac43 ("iommu/amd: Treat per-device
> exclusion ranges as r/w unity-mapped regions") as criminal. Reverting it
> fix the failure.
>
> With the commit, kdump kernel will always print below error message, then
> naturally AMD iommu can't function normally during kdump kernel bootup.
>
> ~
> AMD-Vi: [Firmware Bug]: IVRS invalid checksum
>
> Why commit 387caf0b759ac43 causing it haven't been made clear.
Hi Joerg, Adrian
We only have one machine which can reproduce the issue, it's a gen10-01
of HPE. If any log or info are needed, please let me know, I can attach
here.
Thanks
Baoquan
>
> From the commit log, a discussion thread link is pasted. In that discussion
> thread, Adrian told the fix is for a system with already broken BIOS, and
> Joerg suggested two options. Finally option 2) is taken. Maybe option 1)
> should be the right approach?
>
> 1) Bail out and disable the IOMMU as the BIOS screwed up
> 2) Treat per-device exclusion ranges just as r/w unity-mapped
> regions.
>
> https://lists.linuxfoundation.org/pipermail/iommu/2019-November/040117.html
> Signed-off-by: Baoquan He
> ---
> drivers/iommu/amd/init.c | 21 +
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 9aa1eae26634..bbe7ceae5949 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -1109,17 +1109,22 @@ static int __init add_early_maps(void)
> */
> static void __init set_device_exclusion_range(u16 devid, struct ivmd_header
> *m)
> {
> + struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
> +
> if (!(m->flags & IVMD_FLAG_EXCL_RANGE))
> return;
>
> - /*
> - * Treat per-device exclusion ranges as r/w unity-mapped regions
> - * since some buggy BIOSes might lead to the overwritten exclusion
> - * range (exclusion_start and exclusion_length members). This
> - * happens when there are multiple exclusion ranges (IVMD entries)
> - * defined in ACPI table.
> - */
> - m->flags = (IVMD_FLAG_IW | IVMD_FLAG_IR | IVMD_FLAG_UNITY_MAP);
> + if (iommu) {
> + /*
> + * We only can configure exclusion ranges per IOMMU, not
> + * per device. But we can enable the exclusion range per
> + * device. This is done here
> + */
> + set_dev_entry_bit(devid, DEV_ENTRY_EX);
> + iommu->exclusion_start = m->range_start;
> + iommu->exclusion_length = m->range_length;
> + }
> +
> }
>
> /*
> --
> 2.17.2
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/qcom: add missing put_device() call in qcom_iommu_of_xlate()
Hi Yu, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on iommu/next] [also build test WARNING on linus/master v5.9-rc6 next-20200922] [cannot apply to robclark/msm-next] [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/Yu-Kuai/iommu-qcom-add-missing-put_device-call-in-qcom_iommu_of_xlate/20200918-091341 base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next config: arm64-randconfig-m031-20200923 (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 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from arch/arm64/include/asm/atomic.h:12, from include/linux/atomic.h:7, from drivers/iommu/arm/arm-smmu/qcom_iommu.c:9: drivers/iommu/arm/arm-smmu/qcom_iommu.c: In function 'qcom_iommu_of_xlate': >> include/linux/compiler.h:56:23: warning: this 'if' clause does not guard... >> [-Wmisleading-indentation] 56 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) ) | ^~ drivers/iommu/arm/arm-smmu/qcom_iommu.c:599:3: note: in expansion of macro 'if' 599 | if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev))) | ^~ drivers/iommu/arm/arm-smmu/qcom_iommu.c:601:4: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' 601 |return -EINVAL; |^~ # https://github.com/0day-ci/linux/commit/2d982fb40897ad8088dd9dba06aee499f51c73ba git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Yu-Kuai/iommu-qcom-add-missing-put_device-call-in-qcom_iommu_of_xlate/20200918-091341 git checkout 2d982fb40897ad8088dd9dba06aee499f51c73ba vim +/if +56 include/linux/compiler.h 2bcd521a684cc9 Steven Rostedt 2008-11-21 50 2bcd521a684cc9 Steven Rostedt 2008-11-21 51 #ifdef CONFIG_PROFILE_ALL_BRANCHES 2bcd521a684cc9 Steven Rostedt 2008-11-21 52 /* 2bcd521a684cc9 Steven Rostedt 2008-11-21 53 * "Define 'is'", Bill Clinton 2bcd521a684cc9 Steven Rostedt 2008-11-21 54 * "Define 'if'", Steven Rostedt 2bcd521a684cc9 Steven Rostedt 2008-11-21 55 */ a15fd609ad53a6 Linus Torvalds 2019-03-20 @56 #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) ) a15fd609ad53a6 Linus Torvalds 2019-03-20 57 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/6] Convert the intel iommu driver to the dma-iommu api
On 9/22/20 7:05 PM, Robin Murphy wrote: With the previous version of the series I hit a problem on Ivybridge where apparently the dma engine width is not respected. At least that is my layman interpretation of the errors. From the older thread: <3> [209.526605] DMAR: intel_iommu_map: iommu width (39) is not sufficient for the mapped address (008000) Relevant iommu boot related messages are: <6>[ 0.184234] DMAR: Host address width 36 <6>[ 0.184245] DMAR: DRHD base: 0x00fed9 flags: 0x0 <6>[ 0.184288] DMAR: dmar0: reg_base_addr fed9 ver 1:0 cap c020e60262 ecap f0101a <6>[ 0.184308] DMAR: DRHD base: 0x00fed91000 flags: 0x1 <6>[ 0.184337] DMAR: dmar1: reg_base_addr fed91000 ver 1:0 cap c9008020660262 ecap f0105a <6>[ 0.184357] DMAR: RMRR base: 0x00d8d28000 end: 0x00d8d46fff <6>[ 0.184377] DMAR: RMRR base: 0x00db00 end: 0x00df1f <6>[ 0.184398] DMAR-IR: IOAPIC id 2 under DRHD base 0xfed91000 IOMMU 1 <6>[ 0.184414] DMAR-IR: HPET id 0 under DRHD base 0xfed91000 <6>[ 0.184428] DMAR-IR: Queued invalidation will be enabled to support x2apic and Intr-remapping. <6>[ 0.185173] DMAR-IR: Enabled IRQ remapping in x2apic mode <6>[ 0.878934] DMAR: No ATSR found <6>[ 0.878966] DMAR: dmar0: Using Queued invalidation <6>[ 0.879007] DMAR: dmar1: Using Queued invalidation <6>[ 0.915032] DMAR: Intel(R) Virtualization Technology for Directed I/O <6>[ 0.915060] PCI-DMA: Using software bounce buffering for IO (SWIOTLB) <6>[ 0.915084] software IO TLB: mapped [mem 0xc80d4000-0xcc0d4000] (64MB) (Full boot log at https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_7054/fi-ivb-3770/boot0.txt, failures at https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_7054/fi-ivb-3770/igt@i915_selftest@l...@blt.html.) Does this look familiar or at least plausible to you? Is this something your new series has fixed? This happens during attaching a domain to device. It has nothing to do with this patch series. I will look into this issue, but not in this email thread context. I am not sure what step is attaching domain to device, but these type messages: <3> [209.526605] DMAR: intel_iommu_map: iommu width (39) is not >> sufficient for the mapped address (008000) They definitely appear to happen at runtime, as i915 is getting exercised by userspace. AFAICS this certainly might be related to this series - iommu-dma will Oh! I looked at the wrong function. prepare_domain_attach_device() prints a similar message which made me believe that it was not caused by the this patches series. constrain IOVA allocation based on the domain geometry that the driver reports, which in this case is set only once when first allocating the domain. Thus it looks like both the dmar_domain->gaw adjustment in prepare_domain_attach_device() and the domain_use_first_level() business in intel_alloc_iova() effectively get lost in this conversion, since the domain geometry never gets updated to reflect those additional constraints. Sounds reasonable. I will look into the code and work out a fix. > Robin. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/arm: Add module parameter to set msi iova address
Hi Will,
On 9/21/20 10:45 PM, Will Deacon wrote:
> On Mon, Sep 14, 2020 at 11:13:07AM -0700, Vennila Megavannan wrote:
>> From: Srinath Mannam
>>
>> Add provision to change default value of MSI IOVA base to platform's
>> suitable IOVA using module parameter. The present hardcoded MSI IOVA base
>> may not be the accessible IOVA ranges of platform.
>>
>> If any platform has the limitaion to access default MSI IOVA, then it can
>> be changed using "arm-smmu.msi_iova_base=0xa000" command line argument.
>>
>> Signed-off-by: Srinath Mannam
>> Co-developed-by: Vennila Megavannan
>> Signed-off-by: Vennila Megavannan
>> ---
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 -
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 -
>> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> This feels pretty fragile. Wouldn't it be better to realise that there's
> a region conflict with iommu_dma_get_resv_regions() and move the MSI window
> accordingly at runtime?
Since cd2c9fcf5c66 ("iommu/dma: Move PCI window region reservation back
into dma specific path"), the PCI host bridge windows are not exposed by
iommu_dma_get_resv_regions() anymore. If I understood correctly, what is
attempted here is to avoid the collision between such PCI host bridge
window and the MSI IOVA range.
Thanks
Eric
>
> Will
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: IOVA allocation dependency between firmware buffer and remaining buffers
Hi Shaik, I've run into similar problem while adapting S5P-MFC and Exynos4-IS drivers for generic IOMMU-DMA framework. Here is my first solution: https://lore.kernel.org/linux-samsung-soc/20200918144833.14618-1-m.szyprow...@samsung.com/T/ It allows to remap given buffer at the specific IOVA address, although it doesn't guarantee that those specific addresses won't be later used by the IOVA allocator. Probably it would make sense to add an API for generic IOMMU-DMA framework to mark the given IOVA range as reserved/unused to protect them. Best regards Marek Szyprowski, PhD Samsung R&D Institute Poland On 24.04.2020 18:15, Shaik Ameer Basha wrote: > On Fri, Apr 24, 2020 at 8:59 PM Robin Murphy wrote: >> On 2020-04-24 4:04 pm, Ajay kumar wrote: >>> Can someone check this? >>> >>> On Mon, Apr 20, 2020 at 9:24 PM Ajay kumar wrote: Hi All, I have an IOMMU master which has limitations as mentioned below: 1) The IOMMU master internally executes a firmware, and the firmware memory is allocated by the same master driver. The firmware buffer address should be of the lowest range than other address allocated by the device, or in other words, all the remaining buffer addresses should always be in a higher range than the firmware address. 2) None of the buffer addresses should go beyond 0xC000_ >> That particular constraint could (and perhaps should) be expressed as a >> DMA mask/limit for the device, but if you have specific requirements to > Yes Robin. We do use 0xC000_ address to set the DMA mask in our driver. > >> place buffers at particular addresses then you might be better off >> managing your own IOMMU domain like some other (mostly DRM) drivers do. > If you remember any of such drivers can you please point the driver path ? > >> The DMA APIs don't offer any guarantees about what addresses you'll get >> other than that they won't exceed the appropriate mask. > True, we have gone through most of the APIs and didn't find any way to match > our > requirements with the existing DMA APIs > example: If firmware buffer address is buf_fw = 0x8000_5000; All other addresses given to the device should be greater than (0x8000_5000 + firmware size) and less than 0xC000_ >> Out of curiosity, how do you control that in the no-IOMMU or IOMMU >> passthrough cases? > We manage the no-IOMMU or pass through cases using the reserved-memory. > >> Robin. >> Currently, this is being handled with one of the below hacks: 1) By keeping dma_mask in lower range while allocating firmware buffer, and then increasing the dma_mask to higher range for other buffers. 2) By reserving IOVA for firmware at the lowest range and creating direct mappings for the same. I want to know if there is a better way this can be handled with current framework, or if anybody is facing similar problems with their devices, please share how it is taken care. I also think there should be some way the masters can specify the IOVA range they want to limit to for current allocation. Something like a new iommu_ops callback like below: limit_iova_alloc_range(dev, iova_start, iova_end) And, in my driver, the sequence will be: limit_iova_alloc_range(dev, 0x_, 0x1000_); /* via helpers */ alloc( ) firmware buffer using DMA API limit_iova_alloc_range(dev, 0x1000_, 0xC000_); /* via helpers */ alloc( ) other buffers using DMA API > Just want to understand more from you, on the new iommu_ops we suggested. > Shouldn't device have that flexibility to allocate IOVA as per it's > requirement? > If you see our device as example, we need to have control on the > allocated IOVA region > based on where device is using this buffer. > > If we have these callbacks in place, then the low level IOMMU driver > can implement and > manage such requests when needed. > > If this can't be taken forward for some right reasons, then we will > definitely try to understand > on how to manage the IOMMU domain from our driver as per your suggestion > > - Shaik. > Thanks, Ajay Kumar ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: IOVA allocation dependency between firmware buffer and remaining buffers
On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote: > Hi Shaik, > > I've run into similar problem while adapting S5P-MFC and Exynos4-IS > drivers for generic IOMMU-DMA framework. Here is my first solution: > https://lore.kernel.org/linux-samsung-soc/20200918144833.14618-1-m.szyprow...@samsung.com/T/ > > > > It allows to remap given buffer at the specific IOVA address, although > it doesn't guarantee that those specific addresses won't be later used > by the IOVA allocator. Probably it would make sense to add an API for > generic IOMMU-DMA framework to mark the given IOVA range as > reserved/unused to protect them. If you want to use IOVA addresses in a device otherwise managed by dma-iommu we need to expose an API through the dma API. Can you please include the iommu list in the discussion of your series? I don't think using the raw IOMMU API is a very idea in these drivers, we probably want a way to change the allocator algorithm or hint the next IOVA and keep using the normal DMA API. Maybe Robin has a better idea. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
