All of lore.kernel.org
 help / color / mirror / Atom feed
* ehci-pci breakage with dma-mapping changes in 5.4-rc2
@ 2019-10-07  2:24 Arvind Sankar
  2019-10-07  7:34 ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Arvind Sankar @ 2019-10-07  2:24 UTC (permalink / raw)
  To: linux-kernel; +Cc: Christoph Hellwig

Hi,
Commit 249baa547901 ("dma-mapping: provide a better default
->get_required_mask") causes an error on ehci-pci for me.

Either reverting the commit or disabling iommu=pt seems to fix this.

[    9.000081] usb 1-1: new high-speed USB device number 2 using ehci-pci
[    9.000755] ehci-pci 0000:00:1a.0: swiotlb buffer is full (sz: 8 bytes), total 0 (slots), used 0 (slots)
[    9.001179] ehci-pci 0000:00:1a.0: overflow 0x000000042f541790+8 of DMA mask ffffffff bus mask 0
[    9.001552] ------------[ cut here ]------------
[    9.001933] WARNING: CPU: 0 PID: 7 at kernel/dma/direct.c:35 report_addr+0x3c/0x70
[    9.002355] Modules linked in:
[    9.002802] CPU: 0 PID: 7 Comm: kworker/0:1 Not tainted 5.4.0-rc2-stable-rani-zfs+ #67
[    9.003270] Hardware name: ASUSTeK COMPUTER INC. Z10PE-D8 WS/Z10PE-D8 WS, BIOS 3703 04/13/2018
[    9.003761] Workqueue: usb_hub_wq hub_event
[    9.004241] RIP: 0010:report_addr+0x3c/0x70
[    9.004722] Code: 48 89 34 24 48 8b 85 e8 01 00 00 48 85 c0 74 30 4c 8b 00 b8 fe ff ff ff 49 39 c0 76 17 80 3d 86 6f 23 01 00 0f 84 df 06 00 00 <0f> 0b 48 83 c4 08 5d 41 5c c3 48 83 bd f8 01 00 00 00 74 ec eb dd
[    9.005743] RSP: 0000:ffffb25706307af8 EFLAGS: 00010286
[    9.006261] RAX: 0000000000000000 RBX: ffffe0e3d0bd5040 RCX: 00000000000006fc
[    9.006783] RDX: 0000000000000000 RSI: 0000000000000092 RDI: ffffffffab163fe8
[    9.007301] RBP: ffffa38c678a00b0 R08: 0000000000000000 R09: 00000000000006fc
[    9.007826] R10: 0000000000aaaaaa R11: 00000000ff000000 R12: 0000000000000008
[    9.008339] R13: 0000000000000000 R14: 0000000000000790 R15: ffffa38c678a00b0
[    9.008849] FS:  0000000000000000(0000) GS:ffffa38b5f600000(0000) knlGS:0000000000000000
[    9.009355] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    9.009866] CR2: ffffa396b4001000 CR3: 00000013b340a001 CR4: 00000000003606f0
[    9.010498] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    9.011034] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    9.011580] Call Trace:
[    9.012110]  dma_direct_map_page+0xf8/0x110
[    9.012632]  usb_hcd_map_urb_for_dma+0x204/0x530
[    9.013149]  usb_hcd_submit_urb+0x375/0xb70
[    9.013668]  usb_start_wait_urb+0x8a/0x190
[    9.014188]  usb_control_msg+0xe5/0x150
[    9.014693]  hub_port_init+0x21b/0xb40
[    9.015190]  hub_event+0xb21/0x14f0
[    9.015710]  process_one_work+0x1e5/0x390
[    9.016215]  worker_thread+0x4d/0x3d0
[    9.016720]  kthread+0xfd/0x130
[    9.017207]  ? process_one_work+0x390/0x390
[    9.017687]  ? kthread_park+0x90/0x90
[    9.018163]  ret_from_fork+0x3a/0x50
[    9.018642] ---[ end trace 55eaa8968ea11ab5 ]---
[    9.019124] ehci-pci 0000:00:1a.0: swiotlb buffer is full (sz: 8 bytes), total 0 (slots), used 0 (slots)

cpuinfo:

processor	: 0
vendor_id	: GenuineIntel
cpu family	: 6
model		: 79
model name	: Intel(R) Xeon(R) CPU E5-2696 v4 @ 2.20GHz
stepping	: 1
microcode	: 0xb000038
cpu MHz		: 3353.679
cache size	: 28160 KB
physical id	: 0
siblings	: 44
core id		: 0
cpu cores	: 22
apicid		: 0
initial apicid	: 0
fpu		: yes
fpu_exception	: yes
cpuid level	: 20
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb cat_l3 cdp_l3 invpcid_single pti intel_ppin ssbd ibrs ibpb stibp tpr_shadow vnmi flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm cqm rdt_a rdseed adx smap intel_pt xsaveopt cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local dtherm ida arat pln pts hwp md_clear flush_l1d
bugs		: cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf mds swapgs
bogomips	: 4390.26
clflush size	: 64
cache_alignment	: 64
address sizes	: 46 bits physical, 48 bits virtual
power management:

lspci:

00:1a.0 USB controller: Intel Corporation C610/X99 series chipset USB Enhanced Host Controller #2 (rev 05) (prog-if 20 [EHCI])
	Subsystem: ASUSTeK Computer Inc. C610/X99 series chipset USB Enhanced Host Controller
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin C routed to IRQ 19
	NUMA node: 0
	Region 0: Memory at bc112000 (32-bit, non-prefetchable) [size=1K]
	Capabilities: [50] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [58] Debug port: BAR=1 offset=00a0
	Capabilities: [98] PCI Advanced Features
		AFCap: TP+ FLR+
		AFCtrl: FLR-
		AFStatus: TP-
	Kernel driver in use: ehci-pci


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07  2:24 ehci-pci breakage with dma-mapping changes in 5.4-rc2 Arvind Sankar
@ 2019-10-07  7:34 ` Christoph Hellwig
  2019-10-07 17:54   ` Arvind Sankar
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-07  7:34 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: linux-kernel, Christoph Hellwig

Hi Arvind,

can you try the patch below?


diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3f974919d3bd..52b709bf2b55 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3775,6 +3775,13 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 	return nelems;
 }
 
+static u64 intel_get_required_mask(struct device *dev)
+{
+	if (!iommu_need_mapping(dev))
+		return dma_direct_get_required_mask(dev);
+	return DMA_BIT_MASK(32);
+}
+
 static const struct dma_map_ops intel_dma_ops = {
 	.alloc = intel_alloc_coherent,
 	.free = intel_free_coherent,
@@ -3787,6 +3794,7 @@ static const struct dma_map_ops intel_dma_ops = {
 	.dma_supported = dma_direct_supported,
 	.mmap = dma_common_mmap,
 	.get_sgtable = dma_common_get_sgtable,
+	.get_required_mask = intel_get_required_mask,
 };
 
 static void

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07  7:34 ` Christoph Hellwig
@ 2019-10-07 17:54   ` Arvind Sankar
  2019-10-07 17:55     ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Arvind Sankar @ 2019-10-07 17:54 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Arvind Sankar, linux-kernel

On Mon, Oct 07, 2019 at 09:34:48AM +0200, Christoph Hellwig wrote:
> Hi Arvind,
> 
> can you try the patch below?
> 
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 3f974919d3bd..52b709bf2b55 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -3775,6 +3775,13 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
>  	return nelems;
>  }
>  
> +static u64 intel_get_required_mask(struct device *dev)
> +{
> +	if (!iommu_need_mapping(dev))
> +		return dma_direct_get_required_mask(dev);
> +	return DMA_BIT_MASK(32);
> +}
> +
>  static const struct dma_map_ops intel_dma_ops = {
>  	.alloc = intel_alloc_coherent,
>  	.free = intel_free_coherent,
> @@ -3787,6 +3794,7 @@ static const struct dma_map_ops intel_dma_ops = {
>  	.dma_supported = dma_direct_supported,
>  	.mmap = dma_common_mmap,
>  	.get_sgtable = dma_common_get_sgtable,
> +	.get_required_mask = intel_get_required_mask,
>  };
>  
>  static void

It doesn't boot with the patch. Won't it go
	dma_get_required_mask
	-> intel_get_required_mask
	-> iommu_need_mapping
	-> dma_get_required_mask
?

Should the call to dma_get_required_mask in iommu_need_mapping be
replaced with dma_direct_get_required_mask on top of your patch?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 17:54   ` Arvind Sankar
@ 2019-10-07 17:55     ` Christoph Hellwig
  2019-10-07 17:56       ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-07 17:55 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Christoph Hellwig, linux-kernel

On Mon, Oct 07, 2019 at 01:54:32PM -0400, Arvind Sankar wrote:
> It doesn't boot with the patch. Won't it go
> 	dma_get_required_mask
> 	-> intel_get_required_mask
> 	-> iommu_need_mapping
> 	-> dma_get_required_mask
> ?
> 
> Should the call to dma_get_required_mask in iommu_need_mapping be
> replaced with dma_direct_get_required_mask on top of your patch?

Yes, sorry.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 17:55     ` Christoph Hellwig
@ 2019-10-07 17:56       ` Christoph Hellwig
  2019-10-07 17:58         ` Arvind Sankar
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-07 17:56 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Arvind Sankar, linux-kernel

On Mon, Oct 07, 2019 at 07:55:28PM +0200, Christoph Hellwig wrote:
> On Mon, Oct 07, 2019 at 01:54:32PM -0400, Arvind Sankar wrote:
> > It doesn't boot with the patch. Won't it go
> > 	dma_get_required_mask
> > 	-> intel_get_required_mask
> > 	-> iommu_need_mapping
> > 	-> dma_get_required_mask
> > ?
> > 
> > Should the call to dma_get_required_mask in iommu_need_mapping be
> > replaced with dma_direct_get_required_mask on top of your patch?
> 
> Yes, sorry.

Actually my patch already calls dma_direct_get_required_mask.
How did you get the loop?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 17:56       ` Christoph Hellwig
@ 2019-10-07 17:58         ` Arvind Sankar
  2019-10-07 18:32           ` Arvind Sankar
  0 siblings, 1 reply; 30+ messages in thread
From: Arvind Sankar @ 2019-10-07 17:58 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Christoph Hellwig, Arvind Sankar, linux-kernel

On Mon, Oct 07, 2019 at 10:56:30AM -0700, Christoph Hellwig wrote:
> On Mon, Oct 07, 2019 at 07:55:28PM +0200, Christoph Hellwig wrote:
> > On Mon, Oct 07, 2019 at 01:54:32PM -0400, Arvind Sankar wrote:
> > > It doesn't boot with the patch. Won't it go
> > > 	dma_get_required_mask
> > > 	-> intel_get_required_mask
> > > 	-> iommu_need_mapping
> > > 	-> dma_get_required_mask
> > > ?
> > > 
> > > Should the call to dma_get_required_mask in iommu_need_mapping be
> > > replaced with dma_direct_get_required_mask on top of your patch?
> > 
> > Yes, sorry.
> 
> Actually my patch already calls dma_direct_get_required_mask.
> How did you get the loop?

The function iommu_need_mapping (not changed by your patch) calls
dma_get_required_mask internally, to check whether the device's dma_mask
is big enough or not. That's the call I was asking whether it needs to
be changed.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 17:58         ` Arvind Sankar
@ 2019-10-07 18:32           ` Arvind Sankar
  2019-10-07 18:47             ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Arvind Sankar @ 2019-10-07 18:32 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Christoph Hellwig, Christoph Hellwig, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

On Mon, Oct 07, 2019 at 01:58:57PM -0400, Arvind Sankar wrote:
> On Mon, Oct 07, 2019 at 10:56:30AM -0700, Christoph Hellwig wrote:
> > On Mon, Oct 07, 2019 at 07:55:28PM +0200, Christoph Hellwig wrote:
> > > On Mon, Oct 07, 2019 at 01:54:32PM -0400, Arvind Sankar wrote:
> > > > It doesn't boot with the patch. Won't it go
> > > > 	dma_get_required_mask
> > > > 	-> intel_get_required_mask
> > > > 	-> iommu_need_mapping
> > > > 	-> dma_get_required_mask
> > > > ?
> > > > 
> > > > Should the call to dma_get_required_mask in iommu_need_mapping be
> > > > replaced with dma_direct_get_required_mask on top of your patch?
> > > 
> > > Yes, sorry.
> > 
> > Actually my patch already calls dma_direct_get_required_mask.
> > How did you get the loop?
> 
> The function iommu_need_mapping (not changed by your patch) calls
> dma_get_required_mask internally, to check whether the device's dma_mask
> is big enough or not. That's the call I was asking whether it needs to
> be changed.

Yeah the attached patch seems to fix it.

[-- Attachment #2: 0001-iommu-vt-d-Return-the-correct-dma-mask-when-we-are-b.patch --]
[-- Type: text/x-diff, Size: 1903 bytes --]

From 074e8cc145dde514427c40124913a90c4552dd6e Mon Sep 17 00:00:00 2001
From: Arvind Sankar <nivedita@alum.mit.edu>
Date: Mon, 7 Oct 2019 14:19:12 -0400
Subject: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing
 the IOMMU

We must return a mask covering the full physical RAM when bypassing the
IOMMU mapping. Also, in iommu_need_mapping, we need to check using
dma_direct_get_required_mask to ensure that the device's dma_mask can
cover physical RAM before deciding to bypass IOMMU mapping.

Fixes: 249baa547901 ("dma-mapping: provide a better default ->get_required_mask")
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/iommu/intel-iommu.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3f974919d3bd..79e35b3180ac 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3471,7 +3471,7 @@ static bool iommu_need_mapping(struct device *dev)
 		if (dev->coherent_dma_mask && dev->coherent_dma_mask < dma_mask)
 			dma_mask = dev->coherent_dma_mask;
 
-		if (dma_mask >= dma_get_required_mask(dev))
+		if (dma_mask >= dma_direct_get_required_mask(dev))
 			return false;
 
 		/*
@@ -3775,6 +3775,13 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 	return nelems;
 }
 
+static u64 intel_get_required_mask(struct device *dev)
+{
+	if (!iommu_need_mapping(dev))
+		return dma_direct_get_required_mask(dev);
+	return DMA_BIT_MASK(32);
+}
+
 static const struct dma_map_ops intel_dma_ops = {
 	.alloc = intel_alloc_coherent,
 	.free = intel_free_coherent,
@@ -3787,6 +3794,7 @@ static const struct dma_map_ops intel_dma_ops = {
 	.dma_supported = dma_direct_supported,
 	.mmap = dma_common_mmap,
 	.get_sgtable = dma_common_get_sgtable,
+	.get_required_mask = intel_get_required_mask,
 };
 
 static void
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 18:32           ` Arvind Sankar
@ 2019-10-07 18:47             ` Christoph Hellwig
  2019-10-07 22:10               ` Arvind Sankar
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-07 18:47 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Christoph Hellwig, Christoph Hellwig, linux-kernel

On Mon, Oct 07, 2019 at 02:32:07PM -0400, Arvind Sankar wrote:
> On Mon, Oct 07, 2019 at 01:58:57PM -0400, Arvind Sankar wrote:
> > On Mon, Oct 07, 2019 at 10:56:30AM -0700, Christoph Hellwig wrote:
> > > On Mon, Oct 07, 2019 at 07:55:28PM +0200, Christoph Hellwig wrote:
> > > > On Mon, Oct 07, 2019 at 01:54:32PM -0400, Arvind Sankar wrote:
> > > > > It doesn't boot with the patch. Won't it go
> > > > > 	dma_get_required_mask
> > > > > 	-> intel_get_required_mask
> > > > > 	-> iommu_need_mapping
> > > > > 	-> dma_get_required_mask
> > > > > ?
> > > > > 
> > > > > Should the call to dma_get_required_mask in iommu_need_mapping be
> > > > > replaced with dma_direct_get_required_mask on top of your patch?
> > > > 
> > > > Yes, sorry.
> > > 
> > > Actually my patch already calls dma_direct_get_required_mask.
> > > How did you get the loop?
> > 
> > The function iommu_need_mapping (not changed by your patch) calls
> > dma_get_required_mask internally, to check whether the device's dma_mask
> > is big enough or not. That's the call I was asking whether it needs to
> > be changed.
> 
> Yeah the attached patch seems to fix it.

That looks fine to me:

Acked-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 18:47             ` Christoph Hellwig
@ 2019-10-07 22:10               ` Arvind Sankar
  2019-10-07 23:54                 ` Arvind Sankar
  2019-10-08  7:29                 ` Christoph Hellwig
  0 siblings, 2 replies; 30+ messages in thread
From: Arvind Sankar @ 2019-10-07 22:10 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Arvind Sankar, Christoph Hellwig, linux-kernel

On Mon, Oct 07, 2019 at 08:47:54PM +0200, Christoph Hellwig wrote:
> On Mon, Oct 07, 2019 at 02:32:07PM -0400, Arvind Sankar wrote:
> > On Mon, Oct 07, 2019 at 01:58:57PM -0400, Arvind Sankar wrote:
> > > On Mon, Oct 07, 2019 at 10:56:30AM -0700, Christoph Hellwig wrote:
> > > > On Mon, Oct 07, 2019 at 07:55:28PM +0200, Christoph Hellwig wrote:
> > > > > On Mon, Oct 07, 2019 at 01:54:32PM -0400, Arvind Sankar wrote:
> > > > > > It doesn't boot with the patch. Won't it go
> > > > > > 	dma_get_required_mask
> > > > > > 	-> intel_get_required_mask
> > > > > > 	-> iommu_need_mapping
> > > > > > 	-> dma_get_required_mask
> > > > > > ?
> > > > > > 
> > > > > > Should the call to dma_get_required_mask in iommu_need_mapping be
> > > > > > replaced with dma_direct_get_required_mask on top of your patch?
> > > > > 
> > > > > Yes, sorry.
> > > > 
> > > > Actually my patch already calls dma_direct_get_required_mask.
> > > > How did you get the loop?
> > > 
> > > The function iommu_need_mapping (not changed by your patch) calls
> > > dma_get_required_mask internally, to check whether the device's dma_mask
> > > is big enough or not. That's the call I was asking whether it needs to
> > > be changed.
> > 
> > Yeah the attached patch seems to fix it.
> 
> That looks fine to me:
> 
> Acked-by: Christoph Hellwig <hch@lst.de>

Do you want me to resend the patch as its own mail, or do you just take
it with a Tested-by: from me? If the former, I assume you're ok with me
adding your Signed-off-by?

Thanks

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 22:10               ` Arvind Sankar
@ 2019-10-07 23:54                 ` Arvind Sankar
  2019-10-08  7:32                   ` Christoph Hellwig
  2019-10-08  7:29                 ` Christoph Hellwig
  1 sibling, 1 reply; 30+ messages in thread
From: Arvind Sankar @ 2019-10-07 23:54 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Christoph Hellwig, Christoph Hellwig, linux-kernel

On Mon, Oct 07, 2019 at 06:10:55PM -0400, Arvind Sankar wrote:
> On Mon, Oct 07, 2019 at 08:47:54PM +0200, Christoph Hellwig wrote:
> > On Mon, Oct 07, 2019 at 02:32:07PM -0400, Arvind Sankar wrote:
> > > On Mon, Oct 07, 2019 at 01:58:57PM -0400, Arvind Sankar wrote:
> > > > On Mon, Oct 07, 2019 at 10:56:30AM -0700, Christoph Hellwig wrote:
> > > > > On Mon, Oct 07, 2019 at 07:55:28PM +0200, Christoph Hellwig wrote:
> > > > > > On Mon, Oct 07, 2019 at 01:54:32PM -0400, Arvind Sankar wrote:
> > > > > > > It doesn't boot with the patch. Won't it go
> > > > > > > 	dma_get_required_mask
> > > > > > > 	-> intel_get_required_mask
> > > > > > > 	-> iommu_need_mapping
> > > > > > > 	-> dma_get_required_mask
> > > > > > > ?
> > > > > > > 
> > > > > > > Should the call to dma_get_required_mask in iommu_need_mapping be
> > > > > > > replaced with dma_direct_get_required_mask on top of your patch?
> > > > > > 
> > > > > > Yes, sorry.
> > > > > 
> > > > > Actually my patch already calls dma_direct_get_required_mask.
> > > > > How did you get the loop?
> > > > 
> > > > The function iommu_need_mapping (not changed by your patch) calls
> > > > dma_get_required_mask internally, to check whether the device's dma_mask
> > > > is big enough or not. That's the call I was asking whether it needs to
> > > > be changed.
> > > 
> > > Yeah the attached patch seems to fix it.
> > 
> > That looks fine to me:
> > 
> > Acked-by: Christoph Hellwig <hch@lst.de>
> 
> Do you want me to resend the patch as its own mail, or do you just take
> it with a Tested-by: from me? If the former, I assume you're ok with me
> adding your Signed-off-by?
> 
> Thanks

A question on the original change though -- what happens if a single
device (or a single IOMMU domain really) does want >4G DMA address
space? Was that not previously allowed either?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 22:10               ` Arvind Sankar
  2019-10-07 23:54                 ` Arvind Sankar
@ 2019-10-08  7:29                 ` Christoph Hellwig
  2019-10-08 14:33                   ` [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU Arvind Sankar
  1 sibling, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-08  7:29 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Christoph Hellwig, Christoph Hellwig, linux-kernel

On Mon, Oct 07, 2019 at 06:10:55PM -0400, Arvind Sankar wrote:
> > Acked-by: Christoph Hellwig <hch@lst.de>
> 
> Do you want me to resend the patch as its own mail, or do you just take
> it with a Tested-by: from me? If the former, I assume you're ok with me
> adding your Signed-off-by?

Either way is fine with me.  The original patch was from me, but you
actually did all the work of reporting it, fixing it and testing it so
I'm perfectly fine with you sending it under your name.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-07 23:54                 ` Arvind Sankar
@ 2019-10-08  7:32                   ` Christoph Hellwig
  2019-10-08 11:51                     ` Arvind Sankar
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-08  7:32 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Christoph Hellwig, Christoph Hellwig, linux-kernel

On Mon, Oct 07, 2019 at 07:54:02PM -0400, Arvind Sankar wrote:
> > Do you want me to resend the patch as its own mail, or do you just take
> > it with a Tested-by: from me? If the former, I assume you're ok with me
> > adding your Signed-off-by?
> > 
> > Thanks
> 
> A question on the original change though -- what happens if a single
> device (or a single IOMMU domain really) does want >4G DMA address
> space? Was that not previously allowed either?

Your EHCI device actually supports the larger addressing.  Without an
IOMMU (or with accidentally enabled passthrough mode as in your report)
that will use bounce buffers for physical address that are too large.
With an iommu we can just remap, and by default those remap addresses
are under 32-bit just to make everyones life easier.

The dma_get_required_mask function is misnamed unfortunately, what it
really means is the optimal mask, that is one that avoids bounce
buffering or other complications.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-08  7:32                   ` Christoph Hellwig
@ 2019-10-08 11:51                     ` Arvind Sankar
  2019-10-08 12:50                       ` Christoph Hellwig
  2019-10-08 15:47                       ` Arvind Sankar
  0 siblings, 2 replies; 30+ messages in thread
From: Arvind Sankar @ 2019-10-08 11:51 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Arvind Sankar, Christoph Hellwig, linux-kernel

On Tue, Oct 08, 2019 at 09:32:10AM +0200, Christoph Hellwig wrote:
> On Mon, Oct 07, 2019 at 07:54:02PM -0400, Arvind Sankar wrote:
> > > Do you want me to resend the patch as its own mail, or do you just take
> > > it with a Tested-by: from me? If the former, I assume you're ok with me
> > > adding your Signed-off-by?
> > > 
> > > Thanks
> > 
> > A question on the original change though -- what happens if a single
> > device (or a single IOMMU domain really) does want >4G DMA address
> > space? Was that not previously allowed either?
> 
> Your EHCI device actually supports the larger addressing.  Without an
> IOMMU (or with accidentally enabled passthrough mode as in your report)
> that will use bounce buffers for physical address that are too large.
> With an iommu we can just remap, and by default those remap addresses
> are under 32-bit just to make everyones life easier.
> 
> The dma_get_required_mask function is misnamed unfortunately, what it
> really means is the optimal mask, that is one that avoids bounce
> buffering or other complications.

I understand that my EHCI device, even though it only supports 32-bit
adddressing, will be able to DMA into anywhere in physical RAM, whether
below 4G or not, via the IOMMU or bounce buffering.

What I mean is, do there exist devices (which would necessarily support
64-bit DMA) that want to DMA using bigger than 4Gb buffers. Eg a GPU
accelerator card with 16Gb of RAM on-board that wants to map 6Gb for DMA
in one go, or 5 accelerator cards that are in one IOMMU domain and want
to simultaneously map 1Gb each.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-08 11:51                     ` Arvind Sankar
@ 2019-10-08 12:50                       ` Christoph Hellwig
  2019-10-08 15:47                       ` Arvind Sankar
  1 sibling, 0 replies; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-08 12:50 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Christoph Hellwig, Christoph Hellwig, linux-kernel

On Tue, Oct 08, 2019 at 07:51:03AM -0400, Arvind Sankar wrote:
> What I mean is, do there exist devices (which would necessarily support
> 64-bit DMA) that want to DMA using bigger than 4Gb buffers. Eg a GPU
> accelerator card with 16Gb of RAM on-board that wants to map 6Gb for DMA
> in one go, or 5 accelerator cards that are in one IOMMU domain and want
> to simultaneously map 1Gb each.

If you allocate more than 32-bits worth of address space the IOMMU
address space allocator will dip into 64-bit values if the device
dma mask supports that.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-08  7:29                 ` Christoph Hellwig
@ 2019-10-08 14:33                   ` Arvind Sankar
  2019-10-09  2:45                     ` Lu Baolu
  2019-10-10  1:26                     ` Lu Baolu
  0 siblings, 2 replies; 30+ messages in thread
From: Arvind Sankar @ 2019-10-08 14:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: Christoph Hellwig

We must return a mask covering the full physical RAM when bypassing the
IOMMU mapping. Also, in iommu_need_mapping, we need to check using
dma_direct_get_required_mask to ensure that the device's dma_mask can
cover physical RAM before deciding to bypass IOMMU mapping.

Fixes: 249baa547901 ("dma-mapping: provide a better default ->get_required_mask")
Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
Originally-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/iommu/intel-iommu.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3f974919d3bd..79e35b3180ac 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3471,7 +3471,7 @@ static bool iommu_need_mapping(struct device *dev)
 		if (dev->coherent_dma_mask && dev->coherent_dma_mask < dma_mask)
 			dma_mask = dev->coherent_dma_mask;
 
-		if (dma_mask >= dma_get_required_mask(dev))
+		if (dma_mask >= dma_direct_get_required_mask(dev))
 			return false;
 
 		/*
@@ -3775,6 +3775,13 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 	return nelems;
 }
 
+static u64 intel_get_required_mask(struct device *dev)
+{
+	if (!iommu_need_mapping(dev))
+		return dma_direct_get_required_mask(dev);
+	return DMA_BIT_MASK(32);
+}
+
 static const struct dma_map_ops intel_dma_ops = {
 	.alloc = intel_alloc_coherent,
 	.free = intel_free_coherent,
@@ -3787,6 +3794,7 @@ static const struct dma_map_ops intel_dma_ops = {
 	.dma_supported = dma_direct_supported,
 	.mmap = dma_common_mmap,
 	.get_sgtable = dma_common_get_sgtable,
+	.get_required_mask = intel_get_required_mask,
 };
 
 static void
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-08 11:51                     ` Arvind Sankar
  2019-10-08 12:50                       ` Christoph Hellwig
@ 2019-10-08 15:47                       ` Arvind Sankar
  2019-10-09  6:50                         ` Christoph Hellwig
  1 sibling, 1 reply; 30+ messages in thread
From: Arvind Sankar @ 2019-10-08 15:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Arvind Sankar, Christoph Hellwig, linux-kernel

On Tue, Oct 08, 2019 at 07:51:03AM -0400, Arvind Sankar wrote:
> On Tue, Oct 08, 2019 at 09:32:10AM +0200, Christoph Hellwig wrote:
> > On Mon, Oct 07, 2019 at 07:54:02PM -0400, Arvind Sankar wrote:
> > > > Do you want me to resend the patch as its own mail, or do you just take
> > > > it with a Tested-by: from me? If the former, I assume you're ok with me
> > > > adding your Signed-off-by?
> > > > 
> > > > Thanks
> > > 
> > > A question on the original change though -- what happens if a single
> > > device (or a single IOMMU domain really) does want >4G DMA address
> > > space? Was that not previously allowed either?
> > 
> > Your EHCI device actually supports the larger addressing.  Without an
> > IOMMU (or with accidentally enabled passthrough mode as in your report)
> > that will use bounce buffers for physical address that are too large.
> > With an iommu we can just remap, and by default those remap addresses
> > are under 32-bit just to make everyones life easier.
> > 
> > The dma_get_required_mask function is misnamed unfortunately, what it
> > really means is the optimal mask, that is one that avoids bounce
> > buffering or other complications.
> 
> I understand that my EHCI device, even though it only supports 32-bit
> adddressing, will be able to DMA into anywhere in physical RAM, whether
> below 4G or not, via the IOMMU or bounce buffering.
> 
> What I mean is, do there exist devices (which would necessarily support
> 64-bit DMA) that want to DMA using bigger than 4Gb buffers. Eg a GPU
> accelerator card with 16Gb of RAM on-board that wants to map 6Gb for DMA
> in one go, or 5 accelerator cards that are in one IOMMU domain and want
> to simultaneously map 1Gb each.

Ok, I see that almost nothing actually uses dma_get_required_mask. So if
something did need >4Gb space, the IOMMU would allocate it anyway
regardless of dma_get_required_mask.

I'm thinking this means that we actually only needed to change
dma_get_required_mask to dma_direct_get_required_mask in
iommu_need_mapping, and the rest of the change is unnecessary?

Below is list of stuff calling dma_get_required_mask currently:

/usr/src/git/linux # find . -type f -name \*.[ch] | xargs grep -w dma_get_required_mask

./drivers/mmc/host/sdhci-of-dwcmshc.c:  extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M);
./drivers/pci/controller/vmd.c: return dma_get_required_mask(to_vmd_dev(dev));
./drivers/gpu/drm/etnaviv/etnaviv_gpu.c:                u32 dma_mask = (u32)dma_get_required_mask(gpu->dev);
./drivers/message/fusion/mptbase.c:             const uint64_t required_mask = dma_get_required_mask
./drivers/scsi/dpt_i2o.c:           dma_get_required_mask(&pDev->dev) > DMA_BIT_MASK(32) &&
./drivers/scsi/aic7xxx/aic79xx_osm_pci.c:               const u64 required_mask = dma_get_required_mask(dev);
./drivers/scsi/aic7xxx/aic7xxx_osm_pci.c:           && dma_get_required_mask(dev) > DMA_BIT_MASK(32)) {
./drivers/scsi/mpt3sas/mpt3sas_base.c:  required_mask = dma_get_required_mask(&pdev->dev);
./drivers/scsi/qla2xxx/qla_os.c:                if (MSD(dma_get_required_mask(&ha->pdev->dev)) &&
./drivers/scsi/aacraid/aachba.c:        if (dma_get_required_mask(&dev->pdev->dev) > DMA_BIT_MASK(32))
./drivers/scsi/aacraid/comminit.c:                              dma_get_required_mask(&dev->pdev->dev) >> 12;
./drivers/scsi/esas2r/esas2r_init.c:        dma_get_required_mask(&pcid->dev) > DMA_BIT_MASK(32) &&
./drivers/infiniband/sw/rxe/rxe_verbs.c:                                     dma_get_required_mask(&dev->dev));
./include/linux/dma-mapping.h:u64 dma_get_required_mask(struct device *dev);
./include/linux/dma-mapping.h:static inline u64 dma_get_required_mask(struct device *dev)
./include/linux/dma-mapping.h:                      dma_get_required_mask(dev);
./kernel/dma/mapping.c:u64 dma_get_required_mask(struct device *dev)
./kernel/dma/mapping.c:EXPORT_SYMBOL_GPL(dma_get_required_mask);

For the drivers that do currently use dma_get_required_mask, I believe
we will need to replace most of them with dma_direct_get_required_mask
as well to maintain passthrough functionality -- the fusion, scsi, and
infinband drivers all seem to be using this call to determine whether to
expose the device's 64-bit DMA capability or not. With the change they
will think they only need 32-bit DMA, which in turn will disable
passthrough on them.

The etnaviv driver is doing something funky that I'm not sure about, but
I *think* that one might want the real physical range as well. The mmc
driver I think might be ok with the 32-bit range.

The vmd controller one not sure about.

In dma-mapping.h, the function is used in dma_addressing_limited, which
in turn is used in a couple of amd drm drivers, again not sure about
these.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-08 14:33                   ` [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU Arvind Sankar
@ 2019-10-09  2:45                     ` Lu Baolu
  2019-10-09  6:51                       ` Christoph Hellwig
  2019-10-10  1:26                     ` Lu Baolu
  1 sibling, 1 reply; 30+ messages in thread
From: Lu Baolu @ 2019-10-09  2:45 UTC (permalink / raw)
  To: Arvind Sankar, linux-kernel; +Cc: baolu.lu, Christoph Hellwig

Hi,

On 10/8/19 10:33 PM, Arvind Sankar wrote:
> We must return a mask covering the full physical RAM when bypassing the
> IOMMU mapping. Also, in iommu_need_mapping, we need to check using
> dma_direct_get_required_mask to ensure that the device's dma_mask can
> cover physical RAM before deciding to bypass IOMMU mapping.
> 
> Fixes: 249baa547901 ("dma-mapping: provide a better default ->get_required_mask")
> Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> Originally-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>   drivers/iommu/intel-iommu.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 3f974919d3bd..79e35b3180ac 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -3471,7 +3471,7 @@ static bool iommu_need_mapping(struct device *dev)
>   		if (dev->coherent_dma_mask && dev->coherent_dma_mask < dma_mask)
>   			dma_mask = dev->coherent_dma_mask;
>   
> -		if (dma_mask >= dma_get_required_mask(dev))
> +		if (dma_mask >= dma_direct_get_required_mask(dev))
>   			return false;
>   
>   		/*
> @@ -3775,6 +3775,13 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
>   	return nelems;
>   }
>   
> +static u64 intel_get_required_mask(struct device *dev)
> +{
> +	if (!iommu_need_mapping(dev))
> +		return dma_direct_get_required_mask(dev);
> +	return DMA_BIT_MASK(32);

Do you mind explaining why we always return 32 bit here?

Best regards,
Baolu

> +}
> +
>   static const struct dma_map_ops intel_dma_ops = {
>   	.alloc = intel_alloc_coherent,
>   	.free = intel_free_coherent,
> @@ -3787,6 +3794,7 @@ static const struct dma_map_ops intel_dma_ops = {
>   	.dma_supported = dma_direct_supported,
>   	.mmap = dma_common_mmap,
>   	.get_sgtable = dma_common_get_sgtable,
> +	.get_required_mask = intel_get_required_mask,
>   };
>   
>   static void
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-08 15:47                       ` Arvind Sankar
@ 2019-10-09  6:50                         ` Christoph Hellwig
  2019-10-09 13:48                           ` Arvind Sankar
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-09  6:50 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Christoph Hellwig, Christoph Hellwig, linux-kernel

On Tue, Oct 08, 2019 at 11:47:31AM -0400, Arvind Sankar wrote:
> Ok, I see that almost nothing actually uses dma_get_required_mask. So if
> something did need >4Gb space, the IOMMU would allocate it anyway
> regardless of dma_get_required_mask.

Yes.  And with the direct mapping it also isn't an issue.

> I'm thinking this means that we actually only needed to change
> dma_get_required_mask to dma_direct_get_required_mask in
> iommu_need_mapping, and the rest of the change is unnecessary?
> 
> Below is list of stuff calling dma_get_required_mask currently:

I guess that would actually work ok, but I prefer the more verbose
version as it explain what is going on, and will lead people to do
the right thing if we split the iommu vs passthrough case into
different ops (we already had a patch for that out on the list).

> For the drivers that do currently use dma_get_required_mask, I believe
> we will need to replace most of them with dma_direct_get_required_mask
> as well to maintain passthrough functionality -- the fusion, scsi, and
> infinband drivers all seem to be using this call to determine whether to
> expose the device's 64-bit DMA capability or not. With the change they
> will think they only need 32-bit DMA, which in turn will disable
> passthrough on them.

At least for some of the legacy SCSI drivers that is intentional, and
the reason why dma_get_required_mask was originally added.  On actual
PCI (and PCI-X, but not PCIe which everyone uses now) the 64-bit
addressing even if supported is not very efficient as it needs two
bus cycles.  So we prefer to just use the iommu.

> The etnaviv driver is doing something funky that I'm not sure about, but
> I *think* that one might want the real physical range as well. The mmc
> driver I think might be ok with the 32-bit range.

etnaviv is never used on systems with the intel iommu anyway.

> The vmd controller one not sure about.

That just passes through the dma ops to work around really stupid
intel chipsets.

> In dma-mapping.h, the function is used in dma_addressing_limited, which
> in turn is used in a couple of amd drm drivers, again not sure about
> these.
---end quoted text---

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-09  2:45                     ` Lu Baolu
@ 2019-10-09  6:51                       ` Christoph Hellwig
  2019-10-10  1:24                         ` Lu Baolu
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-09  6:51 UTC (permalink / raw)
  To: Lu Baolu; +Cc: Arvind Sankar, linux-kernel, Christoph Hellwig

On Wed, Oct 09, 2019 at 10:45:15AM +0800, Lu Baolu wrote:
> Do you mind explaining why we always return 32 bit here?

See the comment in dma_get_required_mask().

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ehci-pci breakage with dma-mapping changes in 5.4-rc2
  2019-10-09  6:50                         ` Christoph Hellwig
@ 2019-10-09 13:48                           ` Arvind Sankar
  0 siblings, 0 replies; 30+ messages in thread
From: Arvind Sankar @ 2019-10-09 13:48 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Arvind Sankar, Christoph Hellwig, linux-kernel

On Wed, Oct 09, 2019 at 08:50:43AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 08, 2019 at 11:47:31AM -0400, Arvind Sankar wrote:
> > Ok, I see that almost nothing actually uses dma_get_required_mask. So if
> > something did need >4Gb space, the IOMMU would allocate it anyway
> > regardless of dma_get_required_mask.
> 
> Yes.  And with the direct mapping it also isn't an issue.
> 
> > I'm thinking this means that we actually only needed to change
> > dma_get_required_mask to dma_direct_get_required_mask in
> > iommu_need_mapping, and the rest of the change is unnecessary?
> > 
> > Below is list of stuff calling dma_get_required_mask currently:
> 
> I guess that would actually work ok, but I prefer the more verbose
> version as it explain what is going on, and will lead people to do
> the right thing if we split the iommu vs passthrough case into
> different ops (we already had a patch for that out on the list).
> 
> > For the drivers that do currently use dma_get_required_mask, I believe
> > we will need to replace most of them with dma_direct_get_required_mask
> > as well to maintain passthrough functionality -- the fusion, scsi, and
> > infinband drivers all seem to be using this call to determine whether to
> > expose the device's 64-bit DMA capability or not. With the change they
> > will think they only need 32-bit DMA, which in turn will disable
> > passthrough on them.
> 
> At least for some of the legacy SCSI drivers that is intentional, and
> the reason why dma_get_required_mask was originally added.  On actual
> PCI (and PCI-X, but not PCIe which everyone uses now) the 64-bit
> addressing even if supported is not very efficient as it needs two
> bus cycles.  So we prefer to just use the iommu.
> 
> > The etnaviv driver is doing something funky that I'm not sure about, but
> > I *think* that one might want the real physical range as well. The mmc
> > driver I think might be ok with the 32-bit range.
> 
> etnaviv is never used on systems with the intel iommu anyway.
> 
> > The vmd controller one not sure about.
> 
> That just passes through the dma ops to work around really stupid
> intel chipsets.
> 
> > In dma-mapping.h, the function is used in dma_addressing_limited, which
> > in turn is used in a couple of amd drm drivers, again not sure about
> > these.
> ---end quoted text---

Thanks for the detailed explanation!

That means your changes actually improve the situation for those scsi
drivers -- previously they would have been using 64-bit addressing if
physical RAM needed it, regardless of IOMMU availability/use, right?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-09  6:51                       ` Christoph Hellwig
@ 2019-10-10  1:24                         ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2019-10-10  1:24 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: baolu.lu, Arvind Sankar, linux-kernel

Hi Christoph,

On 10/9/19 2:51 PM, Christoph Hellwig wrote:
> On Wed, Oct 09, 2019 at 10:45:15AM +0800, Lu Baolu wrote:
>> Do you mind explaining why we always return 32 bit here?
> 
> See the comment in dma_get_required_mask().
> 

Got it. Thank you.

Best regards,
Baolu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-08 14:33                   ` [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU Arvind Sankar
  2019-10-09  2:45                     ` Lu Baolu
@ 2019-10-10  1:26                     ` Lu Baolu
  2019-10-16 19:15                       ` Arvind Sankar
  1 sibling, 1 reply; 30+ messages in thread
From: Lu Baolu @ 2019-10-10  1:26 UTC (permalink / raw)
  To: Arvind Sankar, linux-kernel; +Cc: baolu.lu, Christoph Hellwig

Hi,

On 10/8/19 10:33 PM, Arvind Sankar wrote:
> We must return a mask covering the full physical RAM when bypassing the
> IOMMU mapping. Also, in iommu_need_mapping, we need to check using
> dma_direct_get_required_mask to ensure that the device's dma_mask can
> cover physical RAM before deciding to bypass IOMMU mapping.
> 
> Fixes: 249baa547901 ("dma-mapping: provide a better default ->get_required_mask")
> Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> Originally-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>

This patch looks good to me.

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

> ---
>   drivers/iommu/intel-iommu.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 3f974919d3bd..79e35b3180ac 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -3471,7 +3471,7 @@ static bool iommu_need_mapping(struct device *dev)
>   		if (dev->coherent_dma_mask && dev->coherent_dma_mask < dma_mask)
>   			dma_mask = dev->coherent_dma_mask;
>   
> -		if (dma_mask >= dma_get_required_mask(dev))
> +		if (dma_mask >= dma_direct_get_required_mask(dev))
>   			return false;
>   
>   		/*
> @@ -3775,6 +3775,13 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
>   	return nelems;
>   }
>   
> +static u64 intel_get_required_mask(struct device *dev)
> +{
> +	if (!iommu_need_mapping(dev))
> +		return dma_direct_get_required_mask(dev);
> +	return DMA_BIT_MASK(32);
> +}
> +
>   static const struct dma_map_ops intel_dma_ops = {
>   	.alloc = intel_alloc_coherent,
>   	.free = intel_free_coherent,
> @@ -3787,6 +3794,7 @@ static const struct dma_map_ops intel_dma_ops = {
>   	.dma_supported = dma_direct_supported,
>   	.mmap = dma_common_mmap,
>   	.get_sgtable = dma_common_get_sgtable,
> +	.get_required_mask = intel_get_required_mask,
>   };
>   
>   static void
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-10  1:26                     ` Lu Baolu
@ 2019-10-16 19:15                       ` Arvind Sankar
  2019-10-17  7:08                         ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Arvind Sankar @ 2019-10-16 19:15 UTC (permalink / raw)
  To: Lu Baolu; +Cc: Arvind Sankar, linux-kernel, Christoph Hellwig

On Thu, Oct 10, 2019 at 09:26:37AM +0800, Lu Baolu wrote:
> Hi,
> 
> On 10/8/19 10:33 PM, Arvind Sankar wrote:
> > We must return a mask covering the full physical RAM when bypassing the
> > IOMMU mapping. Also, in iommu_need_mapping, we need to check using
> > dma_direct_get_required_mask to ensure that the device's dma_mask can
> > cover physical RAM before deciding to bypass IOMMU mapping.
> > 
> > Fixes: 249baa547901 ("dma-mapping: provide a better default ->get_required_mask")
> > Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> > Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> > Originally-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> 
> This patch looks good to me.
> 
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> 

Hi Christoph, will you be taking this through your dma-mapping branch?

Thanks.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-16 19:15                       ` Arvind Sankar
@ 2019-10-17  7:08                         ` Christoph Hellwig
  2019-10-17 15:55                             ` Arvind Sankar
  2019-10-18  9:50                           ` Joerg Roedel
  0 siblings, 2 replies; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-17  7:08 UTC (permalink / raw)
  To: Arvind Sankar; +Cc: Lu Baolu, linux-kernel, Christoph Hellwig, Joerg Roedel

On Wed, Oct 16, 2019 at 03:15:52PM -0400, Arvind Sankar wrote:
> > > Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > Originally-by: Christoph Hellwig <hch@lst.de>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > 
> > This patch looks good to me.
> > 
> > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > 
> 
> Hi Christoph, will you be taking this through your dma-mapping branch?

Given this is a patch to intel-iommu I expect Joerg to pick it up.
But if he is fine with that I can also queue it up instead.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-17  7:08                         ` Christoph Hellwig
@ 2019-10-17 15:55                             ` Arvind Sankar
  2019-10-18  9:50                           ` Joerg Roedel
  1 sibling, 0 replies; 30+ messages in thread
From: Arvind Sankar @ 2019-10-17 15:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Arvind Sankar, Lu Baolu, linux-kernel, Joerg Roedel,
	David Woodhouse, iommu

On Thu, Oct 17, 2019 at 09:08:47AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 16, 2019 at 03:15:52PM -0400, Arvind Sankar wrote:
> > > > Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Originally-by: Christoph Hellwig <hch@lst.de>
> > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > 
> > > This patch looks good to me.
> > > 
> > > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > > 
> > 
> > Hi Christoph, will you be taking this through your dma-mapping branch?
> 
> Given this is a patch to intel-iommu I expect Joerg to pick it up.
> But if he is fine with that I can also queue it up instead.

David Woodhouse is listed as maintainer for intel-iommu. Cc'ing him as
well.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
@ 2019-10-17 15:55                             ` Arvind Sankar
  0 siblings, 0 replies; 30+ messages in thread
From: Arvind Sankar @ 2019-10-17 15:55 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, iommu, Arvind Sankar, David Woodhouse

On Thu, Oct 17, 2019 at 09:08:47AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 16, 2019 at 03:15:52PM -0400, Arvind Sankar wrote:
> > > > Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Originally-by: Christoph Hellwig <hch@lst.de>
> > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > 
> > > This patch looks good to me.
> > > 
> > > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > > 
> > 
> > Hi Christoph, will you be taking this through your dma-mapping branch?
> 
> Given this is a patch to intel-iommu I expect Joerg to pick it up.
> But if he is fine with that I can also queue it up instead.

David Woodhouse is listed as maintainer for intel-iommu. Cc'ing him as
well.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-17  7:08                         ` Christoph Hellwig
  2019-10-17 15:55                             ` Arvind Sankar
@ 2019-10-18  9:50                           ` Joerg Roedel
  2019-10-18 15:14                             ` Christoph Hellwig
  1 sibling, 1 reply; 30+ messages in thread
From: Joerg Roedel @ 2019-10-18  9:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Arvind Sankar, Lu Baolu, linux-kernel

On Thu, Oct 17, 2019 at 09:08:47AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 16, 2019 at 03:15:52PM -0400, Arvind Sankar wrote:
> > > > Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Originally-by: Christoph Hellwig <hch@lst.de>
> > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > 
> > > This patch looks good to me.
> > > 
> > > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > > 
> > 
> > Hi Christoph, will you be taking this through your dma-mapping branch?
> 
> Given this is a patch to intel-iommu I expect Joerg to pick it up.
> But if he is fine with that I can also queue it up instead.

Fine with me.

Acked-by: Joerg Roedel <jroedel@suse.de>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-18  9:50                           ` Joerg Roedel
@ 2019-10-18 15:14                             ` Christoph Hellwig
  2019-10-18 15:21                               ` Joerg Roedel
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-18 15:14 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Christoph Hellwig, Arvind Sankar, Lu Baolu, linux-kernel

On Fri, Oct 18, 2019 at 11:50:37AM +0200, Joerg Roedel wrote:
> On Thu, Oct 17, 2019 at 09:08:47AM +0200, Christoph Hellwig wrote:
> > On Wed, Oct 16, 2019 at 03:15:52PM -0400, Arvind Sankar wrote:
> > > > > Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > Originally-by: Christoph Hellwig <hch@lst.de>
> > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > > Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > 
> > > > This patch looks good to me.
> > > > 
> > > > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > > > 
> > > 
> > > Hi Christoph, will you be taking this through your dma-mapping branch?
> > 
> > Given this is a patch to intel-iommu I expect Joerg to pick it up.
> > But if he is fine with that I can also queue it up instead.
> 
> Fine with me.
> 
> Acked-by: Joerg Roedel <jroedel@suse.de>

That means you want me to queue it up?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-18 15:14                             ` Christoph Hellwig
@ 2019-10-18 15:21                               ` Joerg Roedel
  2019-10-18 15:22                                 ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Joerg Roedel @ 2019-10-18 15:21 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Arvind Sankar, Lu Baolu, linux-kernel

On Fri, Oct 18, 2019 at 05:14:53PM +0200, Christoph Hellwig wrote:
> On Fri, Oct 18, 2019 at 11:50:37AM +0200, Joerg Roedel wrote:
> > On Thu, Oct 17, 2019 at 09:08:47AM +0200, Christoph Hellwig wrote:
> > > On Wed, Oct 16, 2019 at 03:15:52PM -0400, Arvind Sankar wrote:
> > > > > > Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > > Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > > Originally-by: Christoph Hellwig <hch@lst.de>
> > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > > > Fixed-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > 
> > > > > This patch looks good to me.
> > > > > 
> > > > > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > > > > 
> > > > 
> > > > Hi Christoph, will you be taking this through your dma-mapping branch?
> > > 
> > > Given this is a patch to intel-iommu I expect Joerg to pick it up.
> > > But if he is fine with that I can also queue it up instead.
> > 
> > Fine with me.
> > 
> > Acked-by: Joerg Roedel <jroedel@suse.de>
> 
> That means you want me to queue it up?

Yes, feel free to take it into your tree.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU
  2019-10-18 15:21                               ` Joerg Roedel
@ 2019-10-18 15:22                                 ` Christoph Hellwig
  0 siblings, 0 replies; 30+ messages in thread
From: Christoph Hellwig @ 2019-10-18 15:22 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Christoph Hellwig, Arvind Sankar, Lu Baolu, linux-kernel

On Fri, Oct 18, 2019 at 05:21:49PM +0200, Joerg Roedel wrote:
> > > Fine with me.
> > > 
> > > Acked-by: Joerg Roedel <jroedel@suse.de>
> > 
> > That means you want me to queue it up?
> 
> Yes, feel free to take it into your tree.

Done.

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2019-10-18 15:22 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07  2:24 ehci-pci breakage with dma-mapping changes in 5.4-rc2 Arvind Sankar
2019-10-07  7:34 ` Christoph Hellwig
2019-10-07 17:54   ` Arvind Sankar
2019-10-07 17:55     ` Christoph Hellwig
2019-10-07 17:56       ` Christoph Hellwig
2019-10-07 17:58         ` Arvind Sankar
2019-10-07 18:32           ` Arvind Sankar
2019-10-07 18:47             ` Christoph Hellwig
2019-10-07 22:10               ` Arvind Sankar
2019-10-07 23:54                 ` Arvind Sankar
2019-10-08  7:32                   ` Christoph Hellwig
2019-10-08 11:51                     ` Arvind Sankar
2019-10-08 12:50                       ` Christoph Hellwig
2019-10-08 15:47                       ` Arvind Sankar
2019-10-09  6:50                         ` Christoph Hellwig
2019-10-09 13:48                           ` Arvind Sankar
2019-10-08  7:29                 ` Christoph Hellwig
2019-10-08 14:33                   ` [PATCH] iommu/vt-d: Return the correct dma mask when we are bypassing the IOMMU Arvind Sankar
2019-10-09  2:45                     ` Lu Baolu
2019-10-09  6:51                       ` Christoph Hellwig
2019-10-10  1:24                         ` Lu Baolu
2019-10-10  1:26                     ` Lu Baolu
2019-10-16 19:15                       ` Arvind Sankar
2019-10-17  7:08                         ` Christoph Hellwig
2019-10-17 15:55                           ` Arvind Sankar
2019-10-17 15:55                             ` Arvind Sankar
2019-10-18  9:50                           ` Joerg Roedel
2019-10-18 15:14                             ` Christoph Hellwig
2019-10-18 15:21                               ` Joerg Roedel
2019-10-18 15:22                                 ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.