diff mbox series

[12/34] powerpc/cell: move dma direct window setup out of dma_configure

Message ID 20181114082314.8965-13-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [01/34] powerpc: use mm zones more sensibly | expand

Commit Message

Christoph Hellwig Nov. 14, 2018, 8:22 a.m. UTC
Configure the dma settings at device setup time, and stop playing games
with get_pci_dma_ops.  This prepares for using the common dma_configure
code later on.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/platforms/cell/iommu.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Comments

Michael Ellerman Dec. 9, 2018, 10:23 a.m. UTC | #1
Christoph Hellwig <hch@lst.de> writes:

> Configure the dma settings at device setup time, and stop playing games
> with get_pci_dma_ops.  This prepares for using the common dma_configure
> code later on.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/powerpc/platforms/cell/iommu.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)

This one's crashing, haven't dug into why yet:

  [    1.347085] Unable to handle kernel paging request for data at address 0x00000040
  [    1.391505] Faulting instruction address: 0xc0000000006b6e6c
  cpu 0x0: Vector: 380 (Data SLB Access) at [c0000007fc9032d0]
  pc: c0000000006b6e6c: .of_n_addr_cells+0x34/0xc0
  lr: c000000000070b30: .cell_iommu_get_fixed_address+0x58/0x2b0
  sp: c0000007fc903560
  msr: 9000000000009032
  dar: 40
  current = 0xc0000007fc8d0000
  paca    = 0xc000000000f60000	 irqmask: 0x03	 irq_happened: 0x01
  pid   = 1, comm = swapper/0
  Linux version 4.20.0-rc2-gcc7x-g1e32f48 (kerkins@p82) (gcc version 7.4.1 20181208 (Custom eb377405ab2d1900)) #1 SMP Sun Dec 9 12:16:48 AEDT 2018
  enter ? for help
  [c0000007fc9035f0] c000000000070b30 .cell_iommu_get_fixed_address+0x58/0x2b0
  [c0000007fc9036c0] c0000000000711ac .cell_dma_dev_setup.part.1+0x24/0x118
  [c0000007fc903740] c000000000071374 .cell_of_bus_notify+0x6c/0xbc
  [c0000007fc9037c0] c0000000000e7ef0 .notifier_call_chain+0x90/0xf8
  [c0000007fc903860] c0000000000e8c2c .blocking_notifier_call_chain+0x84/0xb8
  [c0000007fc9038f0] c000000000597544 .device_add+0x584/0x7b8
  [c0000007fc9039c0] c0000000005a0308 .platform_device_add+0x148/0x2f0
  [c0000007fc903a60] c0000000005a1508 .platform_device_register_full+0x148/0x168
  [c0000007fc903ae0] c000000000a9a8a0 .__machine_initcall_cell_cell_publish_devices+0x1bc/0x210
  [c0000007fc903be0] c00000000000eca4 .do_one_initcall+0x64/0x2d8
  [c0000007fc903cc0] c000000000a844ec .kernel_init_freeable+0x3dc/0x4e4
  [c0000007fc903da0] c00000000000f06c .kernel_init+0x24/0x150
  [c0000007fc903e20] c00000000000a9c0 .ret_from_kernel_thread+0x58/0x78

cheers

> diff --git a/arch/powerpc/platforms/cell/iommu.c b/arch/powerpc/platforms/cell/iommu.c
> index 12352a58072a..cce5bf9515e5 100644
> --- a/arch/powerpc/platforms/cell/iommu.c
> +++ b/arch/powerpc/platforms/cell/iommu.c
> @@ -657,14 +657,21 @@ static const struct dma_map_ops dma_iommu_fixed_ops = {
>  	.mapping_error	= dma_iommu_mapping_error,
>  };
>  
> +static u64 cell_iommu_get_fixed_address(struct device *dev);
> +
>  static void cell_dma_dev_setup(struct device *dev)
>  {
> -	if (get_pci_dma_ops() == &dma_iommu_ops)
> +	if (get_pci_dma_ops() == &dma_iommu_ops) {
> +		u64 addr = cell_iommu_get_fixed_address(dev);
> +
> +		if (addr != OF_BAD_ADDR)
> +			set_dma_offset(dev, addr + dma_iommu_fixed_base);
>  		set_iommu_table_base(dev, cell_get_iommu_table(dev));
> -	else if (get_pci_dma_ops() == &dma_nommu_ops)
> +	} else if (get_pci_dma_ops() == &dma_nommu_ops) {
>  		set_dma_offset(dev, cell_dma_nommu_offset);
> -	else
> +	} else {
>  		BUG();
> +	}
>  }
>  
>  static void cell_pci_dma_dev_setup(struct pci_dev *dev)
> @@ -950,19 +957,14 @@ static int dma_suported_and_switch(struct device *dev, u64 dma_mask)
>  {
>  	if (dma_mask == DMA_BIT_MASK(64) &&
>  	    cell_iommu_get_fixed_address(dev) != OF_BAD_ADDR) {
> -		u64 addr = cell_iommu_get_fixed_address(dev) +
> -			dma_iommu_fixed_base;
>  		dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n");
> -		dev_dbg(dev, "iommu: fixed addr = %llx\n", addr);
>  		set_dma_ops(dev, &dma_iommu_fixed_ops);
> -		set_dma_offset(dev, addr);
>  		return 1;
>  	}
>  
>  	if (dma_iommu_dma_supported(dev, dma_mask)) {
>  		dev_dbg(dev, "iommu: not 64-bit, using default ops\n");
> -		set_dma_ops(dev, get_pci_dma_ops());
> -		cell_dma_dev_setup(dev);
> +		set_dma_ops(dev, &dma_iommu_ops);
>  		return 1;
>  	}
>  
> -- 
> 2.19.1
Christoph Hellwig Dec. 12, 2018, 2:36 p.m. UTC | #2
On Sun, Dec 09, 2018 at 09:23:39PM +1100, Michael Ellerman wrote:
> Christoph Hellwig <hch@lst.de> writes:
> 
> > Configure the dma settings at device setup time, and stop playing games
> > with get_pci_dma_ops.  This prepares for using the common dma_configure
> > code later on.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  arch/powerpc/platforms/cell/iommu.c | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> This one's crashing, haven't dug into why yet:

Can you provide a gdb assembly of the exact crash site?  This looks
like for some odd reason the DT structures aren't fully setup by the
time we are probing the device, which seems odd.

Either way, something like the patch below would ensure we call
cell_iommu_get_fixed_address from a similar context as before, can you
check if that fixes the issue?

diff --git a/arch/powerpc/platforms/cell/iommu.c b/arch/powerpc/platforms/cell/iommu.c
index 93c7e4aef571..4891b338bf9f 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -569,19 +569,12 @@ static struct iommu_table *cell_get_iommu_table(struct device *dev)
 	return &window->table;
 }
 
-static u64 cell_iommu_get_fixed_address(struct device *dev);
-
 static void cell_dma_dev_setup(struct device *dev)
 {
-	if (cell_iommu_enabled) {
-		u64 addr = cell_iommu_get_fixed_address(dev);
-
-		if (addr != OF_BAD_ADDR)
-			set_dma_offset(dev, addr + dma_iommu_fixed_base);
+	if (cell_iommu_enabled)
 		set_iommu_table_base(dev, cell_get_iommu_table(dev));
-	} else {
+	else
 		set_dma_offset(dev, cell_dma_nommu_offset);
-	}
 }
 
 static void cell_pci_dma_dev_setup(struct pci_dev *dev)
@@ -865,8 +858,16 @@ static u64 cell_iommu_get_fixed_address(struct device *dev)
 
 static bool cell_pci_iommu_bypass_supported(struct pci_dev *pdev, u64 mask)
 {
-	return mask == DMA_BIT_MASK(64) &&
-		cell_iommu_get_fixed_address(&pdev->dev) != OF_BAD_ADDR;
+	if (mask == DMA_BIT_MASK(64)) {
+		u64 addr = cell_iommu_get_fixed_address(&pdev->dev);
+
+		if (addr != OF_BAD_ADDR) {
+			set_dma_offset(&pdev->dev, dma_iommu_fixed_base + addr);
+			return true;
+		}
+	}
+
+	return true;
 }
 
 static void insert_16M_pte(unsigned long addr, unsigned long *ptab,
Christoph Hellwig Dec. 14, 2018, 4:42 p.m. UTC | #3
On Sat, Dec 15, 2018 at 12:29:11AM +1100, Michael Ellerman wrote:
> I think the problem is that we don't want to set iommu_bypass_supported
> unless cell_iommu_fixed_mapping_init() succeeds.
> 
> Yep. This makes it work for me on cell on top of your v5.

Thanks, this looks good.  I've folded it with the slight change of moving
the iommu_bypass_supported setup into cell_iommu_fixed_mapping_init.
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/cell/iommu.c b/arch/powerpc/platforms/cell/iommu.c
index 12352a58072a..cce5bf9515e5 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -657,14 +657,21 @@  static const struct dma_map_ops dma_iommu_fixed_ops = {
 	.mapping_error	= dma_iommu_mapping_error,
 };
 
+static u64 cell_iommu_get_fixed_address(struct device *dev);
+
 static void cell_dma_dev_setup(struct device *dev)
 {
-	if (get_pci_dma_ops() == &dma_iommu_ops)
+	if (get_pci_dma_ops() == &dma_iommu_ops) {
+		u64 addr = cell_iommu_get_fixed_address(dev);
+
+		if (addr != OF_BAD_ADDR)
+			set_dma_offset(dev, addr + dma_iommu_fixed_base);
 		set_iommu_table_base(dev, cell_get_iommu_table(dev));
-	else if (get_pci_dma_ops() == &dma_nommu_ops)
+	} else if (get_pci_dma_ops() == &dma_nommu_ops) {
 		set_dma_offset(dev, cell_dma_nommu_offset);
-	else
+	} else {
 		BUG();
+	}
 }
 
 static void cell_pci_dma_dev_setup(struct pci_dev *dev)
@@ -950,19 +957,14 @@  static int dma_suported_and_switch(struct device *dev, u64 dma_mask)
 {
 	if (dma_mask == DMA_BIT_MASK(64) &&
 	    cell_iommu_get_fixed_address(dev) != OF_BAD_ADDR) {
-		u64 addr = cell_iommu_get_fixed_address(dev) +
-			dma_iommu_fixed_base;
 		dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n");
-		dev_dbg(dev, "iommu: fixed addr = %llx\n", addr);
 		set_dma_ops(dev, &dma_iommu_fixed_ops);
-		set_dma_offset(dev, addr);
 		return 1;
 	}
 
 	if (dma_iommu_dma_supported(dev, dma_mask)) {
 		dev_dbg(dev, "iommu: not 64-bit, using default ops\n");
-		set_dma_ops(dev, get_pci_dma_ops());
-		cell_dma_dev_setup(dev);
+		set_dma_ops(dev, &dma_iommu_ops);
 		return 1;
 	}