[PATCH 1/1 (resend)] mthca: Modify to support embedded PowerPC platforms

[PATCH 1/1 (resend)] mthca: Modify to support embedded PowerPC platforms

[J.L. Burr]

Resending to fix mangling of patch.
---
Here's the patch described in part 0.

Use phys_addr_t, rather than unsigned long, for physical addresses to
accommodate embedded PowerPC processors which use a 36-bit physical address
on a 32-bit CPU.  phys_addr_t will be 64-bits on these platforms.

This patch is for Linux kernel 2.6.26.
---
diff -rup a/drivers/infiniband/hw/mthca/mthca_catas.c b/drivers/infiniband/hw/mthca/mthca_catas.c
--- a/drivers/infiniband/hw/mthca/mthca_catas.c
+++ b/drivers/infiniband/hw/mthca/mthca_catas.c
@@ -148,7 +148,7 @@ static void poll_catas(unsigned long dev

 void mthca_start_catas_poll(struct mthca_dev *dev)
 {
- unsigned long addr;
+ phys_addr_t addr;

  init_timer(&dev->catas_err.timer);
  dev->catas_err.stop = 0;
@@ -161,14 +161,14 @@ void mthca_start_catas_poll(struct mthca
  if (!request_mem_region(addr, dev->catas_err.size * 4,
     DRV_NAME)) {
   mthca_warn(dev, "couldn't request catastrophic error region "
-      "at 0x%lx/0x%x\n", addr, dev->catas_err.size * 4);
+      "at 0x%llx/0x%x\n", (unsigned long long)addr, dev->catas_err.size * 4);
   return;
  }

  dev->catas_err.map = ioremap(addr, dev->catas_err.size * 4);
  if (!dev->catas_err.map) {
   mthca_warn(dev, "couldn't map catastrophic error region "
-      "at 0x%lx/0x%x\n", addr, dev->catas_err.size * 4);
+      "at 0x%llx/0x%x\n", (unsigned long long)addr, dev->catas_err.size * 4);
   release_mem_region(addr, dev->catas_err.size * 4);
   return;
  }
diff -rup a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -712,7 +712,7 @@ int mthca_RUN_FW(struct mthca_dev *dev,

 static void mthca_setup_cmd_doorbells(struct mthca_dev *dev, u64 base)
 {
- unsigned long addr;
+ phys_addr_t addr;
  u16 max_off = 0;
  int i;

diff -rup a/drivers/infiniband/hw/mthca/mthca_eq.c b/drivers/infiniband/hw/mthca/mthca_eq.c
--- a/drivers/infiniband/hw/mthca/mthca_eq.c
+++ b/drivers/infiniband/hw/mthca/mthca_eq.c
@@ -652,7 +652,7 @@ static int mthca_map_reg(struct mthca_de
     unsigned long offset, unsigned long size,
     void __iomem **map)
 {
- unsigned long base = pci_resource_start(dev->pdev, 0);
+ phys_addr_t base = pci_resource_start(dev->pdev, 0);

  if (!request_mem_region(base + offset, size, DRV_NAME))
   return -EBUSY;
@@ -669,7 +669,7 @@ static int mthca_map_reg(struct mthca_de
 static void mthca_unmap_reg(struct mthca_dev *dev, unsigned long offset,
        unsigned long size, void __iomem *map)
 {
- unsigned long base = pci_resource_start(dev->pdev, 0);
+ phys_addr_t base = pci_resource_start(dev->pdev, 0);

  release_mem_region(base + offset, size);
  iounmap(map);
diff -rup a/drivers/infiniband/hw/mthca/mthca_main.c b/drivers/infiniband/hw/mthca/mthca_main.c
--- a/drivers/infiniband/hw/mthca/mthca_main.c
+++ b/drivers/infiniband/hw/mthca/mthca_main.c
@@ -786,7 +786,7 @@ static int mthca_setup_hca(struct mthca_
   goto err_uar_table_free;
  }

- dev->kar = ioremap(dev->driver_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
+ dev->kar = ioremap((phys_addr_t)(dev->driver_uar.pfn) << PAGE_SHIFT, PAGE_SIZE);
  if (!dev->kar) {
   mthca_err(dev, "Couldn't map kernel access region, "
      "aborting.\n");
diff -rup a/drivers/infiniband/hw/mthca/mthca_mr.c b/drivers/infiniband/hw/mthca/mthca_mr.c
--- a/drivers/infiniband/hw/mthca/mthca_mr.c
+++ b/drivers/infiniband/hw/mthca/mthca_mr.c
@@ -838,7 +838,7 @@ void mthca_arbel_fmr_unmap(struct mthca_

 int mthca_init_mr_table(struct mthca_dev *dev)
 {
- unsigned long addr;
+ phys_addr_t addr;
  int mpts, mtts, err, i;

  err = mthca_alloc_init(&dev->mr_table.mpt_alloc,



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1 (resend)] mthca: Modify to support embedded PowerPC platforms

[Roland Dreier]

 > Resending to fix mangling of patch.

Still mangled unfortunately.  Go to https://patchwork.kernel.org/patch/462311/
and use the "patch" link from the "Download" line and you'll see that
your patch no longer applies.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1 (resend)] mthca: Modify to support embedded PowerPC platforms

[Roland Dreier]

So I redid your patch by hand (and also fixed it up to apply to the
current driver -- there were some changes to mthca_eq.c that I think I
took into account).  See below... can I get a "Signed-off-by:" line from
you for this?

8<--------------------------------------------------

IB/mthca: Fix driver when sizeof (phys_addr_t) > sizeof (long)

Some systems have PCI addresses that don't fit in unsigned long (eg some
32-bit PowerPC 440 systems have 36-bit bus addresses).  Fix up the driver
by using phys_addr_t where appropriate, so we don't truncate any PCI
resource addresses before ioremapping them.

[ Update to apply to current driver source.  - Roland ]

Signed-off-by: Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/mthca/mthca_catas.c |    5 +++--
 drivers/infiniband/hw/mthca/mthca_cmd.c   |    2 +-
 drivers/infiniband/hw/mthca/mthca_eq.c    |    2 +-
 drivers/infiniband/hw/mthca/mthca_main.c  |    2 +-
 drivers/infiniband/hw/mthca/mthca_mr.c    |    2 +-
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_catas.c b/drivers/infiniband/hw/mthca/mthca_catas.c
index 0aa0110..e4a08c2 100644
--- a/drivers/infiniband/hw/mthca/mthca_catas.c
+++ b/drivers/infiniband/hw/mthca/mthca_catas.c
@@ -146,7 +146,7 @@ static void poll_catas(unsigned long dev_ptr)
 
 void mthca_start_catas_poll(struct mthca_dev *dev)
 {
-	unsigned long addr;
+	phys_addr_t addr;
 
 	init_timer(&dev->catas_err.timer);
 	dev->catas_err.map  = NULL;
@@ -158,7 +158,8 @@ void mthca_start_catas_poll(struct mthca_dev *dev)
 	dev->catas_err.map = ioremap(addr, dev->catas_err.size * 4);
 	if (!dev->catas_err.map) {
 		mthca_warn(dev, "couldn't map catastrophic error region "
-			   "at 0x%lx/0x%x\n", addr, dev->catas_err.size * 4);
+			   "at 0x%llx/0x%x\n", (unsigned long long) addr,
+			   dev->catas_err.size * 4);
 		return;
 	}
 
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index f4ceecd..7bfa2a1 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -713,7 +713,7 @@ int mthca_RUN_FW(struct mthca_dev *dev, u8 *status)
 
 static void mthca_setup_cmd_doorbells(struct mthca_dev *dev, u64 base)
 {
-	unsigned long addr;
+	phys_addr_t addr;
 	u16 max_off = 0;
 	int i;
 
diff --git a/drivers/infiniband/hw/mthca/mthca_eq.c b/drivers/infiniband/hw/mthca/mthca_eq.c
index 8e8c728..76785c6 100644
--- a/drivers/infiniband/hw/mthca/mthca_eq.c
+++ b/drivers/infiniband/hw/mthca/mthca_eq.c
@@ -653,7 +653,7 @@ static int mthca_map_reg(struct mthca_dev *dev,
 			 unsigned long offset, unsigned long size,
 			 void __iomem **map)
 {
-	unsigned long base = pci_resource_start(dev->pdev, 0);
+	phys_addr_t base = pci_resource_start(dev->pdev, 0);
 
 	*map = ioremap(base + offset, size);
 	if (!*map)
diff --git a/drivers/infiniband/hw/mthca/mthca_main.c b/drivers/infiniband/hw/mthca/mthca_main.c
index 5eee666..8a40cd5 100644
--- a/drivers/infiniband/hw/mthca/mthca_main.c
+++ b/drivers/infiniband/hw/mthca/mthca_main.c
@@ -790,7 +790,7 @@ static int mthca_setup_hca(struct mthca_dev *dev)
 		goto err_uar_table_free;
 	}
 
-	dev->kar = ioremap(dev->driver_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
+	dev->kar = ioremap((phys_addr_t) dev->driver_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
 	if (!dev->kar) {
 		mthca_err(dev, "Couldn't map kernel access region, "
 			  "aborting.\n");
diff --git a/drivers/infiniband/hw/mthca/mthca_mr.c b/drivers/infiniband/hw/mthca/mthca_mr.c
index 065b208..44045c8 100644
--- a/drivers/infiniband/hw/mthca/mthca_mr.c
+++ b/drivers/infiniband/hw/mthca/mthca_mr.c
@@ -853,7 +853,7 @@ void mthca_arbel_fmr_unmap(struct mthca_dev *dev, struct mthca_fmr *fmr)
 
 int mthca_init_mr_table(struct mthca_dev *dev)
 {
-	unsigned long addr;
+	phys_addr_t addr;
 	int mpts, mtts, err, i;
 
 	err = mthca_alloc_init(&dev->mr_table.mpt_alloc,
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1 (resend)] mthca: Modify to support embedded PowerPC platforms

[J.L. Burr]

On 1/10/2011 11:12 PM, Roland Dreier wrote:
> So I redid your patch by hand (and also fixed it up to apply to the
> current driver -- there were some changes to mthca_eq.c that I think I
> took into account).  See below... can I get a "Signed-off-by:" line from
> you for this?

I reviewed your change against the latest tree and it looks fine to me.
I've tried to get my e-mail / news client set-up properly and hope that I'm
now maintaining tab characters!  Thanks for shepherding this through for me.

8<--------------------------------------------------

IB/mthca: Fix driver when sizeof (phys_addr_t) > sizeof (long)

Some systems have PCI addresses that don't fit in unsigned long (eg some
32-bit PowerPC 440 systems have 36-bit bus addresses).  Fix up the driver
by using phys_addr_t where appropriate, so we don't truncate any PCI
resource addresses before ioremapping them.

[ Update to apply to current driver source.  - Roland ]

Signed-off-by: Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
Signed-off-by: John L. Burr <jlburr-vna1KIf7WgpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/infiniband/hw/mthca/mthca_catas.c |    5 +++--
 drivers/infiniband/hw/mthca/mthca_cmd.c   |    2 +-
 drivers/infiniband/hw/mthca/mthca_eq.c    |    2 +-
 drivers/infiniband/hw/mthca/mthca_main.c  |    2 +-
 drivers/infiniband/hw/mthca/mthca_mr.c    |    2 +-
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_catas.c b/drivers/infiniband/hw/mthca/mthca_catas.c
index 0aa0110..e4a08c2 100644
--- a/drivers/infiniband/hw/mthca/mthca_catas.c
+++ b/drivers/infiniband/hw/mthca/mthca_catas.c
@@ -146,7 +146,7 @@ static void poll_catas(unsigned long dev_ptr)

 void mthca_start_catas_poll(struct mthca_dev *dev)
 {
-	unsigned long addr;
+	phys_addr_t addr;

 	init_timer(&dev->catas_err.timer);
 	dev->catas_err.map  = NULL;
@@ -158,7 +158,8 @@ void mthca_start_catas_poll(struct mthca_dev *dev)
 	dev->catas_err.map = ioremap(addr, dev->catas_err.size * 4);
 	if (!dev->catas_err.map) {
 		mthca_warn(dev, "couldn't map catastrophic error region "
-			   "at 0x%lx/0x%x\n", addr, dev->catas_err.size * 4);
+			   "at 0x%llx/0x%x\n", (unsigned long long) addr,
+			   dev->catas_err.size * 4);
 		return;
 	}

diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index f4ceecd..7bfa2a1 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -713,7 +713,7 @@ int mthca_RUN_FW(struct mthca_dev *dev, u8 *status)

 static void mthca_setup_cmd_doorbells(struct mthca_dev *dev, u64 base)
 {
-	unsigned long addr;
+	phys_addr_t addr;
 	u16 max_off = 0;
 	int i;

diff --git a/drivers/infiniband/hw/mthca/mthca_eq.c b/drivers/infiniband/hw/mthca/mthca_eq.c
index 8e8c728..76785c6 100644
--- a/drivers/infiniband/hw/mthca/mthca_eq.c
+++ b/drivers/infiniband/hw/mthca/mthca_eq.c
@@ -653,7 +653,7 @@ static int mthca_map_reg(struct mthca_dev *dev,
 			 unsigned long offset, unsigned long size,
 			 void __iomem **map)
 {
-	unsigned long base = pci_resource_start(dev->pdev, 0);
+	phys_addr_t base = pci_resource_start(dev->pdev, 0);

 	*map = ioremap(base + offset, size);
 	if (!*map)
diff --git a/drivers/infiniband/hw/mthca/mthca_main.c b/drivers/infiniband/hw/mthca/mthca_main.c
index 5eee666..8a40cd5 100644
--- a/drivers/infiniband/hw/mthca/mthca_main.c
+++ b/drivers/infiniband/hw/mthca/mthca_main.c
@@ -790,7 +790,7 @@ static int mthca_setup_hca(struct mthca_dev *dev)
 		goto err_uar_table_free;
 	}

-	dev->kar = ioremap(dev->driver_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
+	dev->kar = ioremap((phys_addr_t) dev->driver_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
 	if (!dev->kar) {
 		mthca_err(dev, "Couldn't map kernel access region, "
 			  "aborting.\n");
diff --git a/drivers/infiniband/hw/mthca/mthca_mr.c b/drivers/infiniband/hw/mthca/mthca_mr.c
index 065b208..44045c8 100644
--- a/drivers/infiniband/hw/mthca/mthca_mr.c
+++ b/drivers/infiniband/hw/mthca/mthca_mr.c
@@ -853,7 +853,7 @@ void mthca_arbel_fmr_unmap(struct mthca_dev *dev, struct mthca_fmr *fmr)

 int mthca_init_mr_table(struct mthca_dev *dev)
 {
-	unsigned long addr;
+	phys_addr_t addr;
 	int mpts, mtts, err, i;

 	err = mthca_alloc_init(&dev->mr_table.mpt_alloc,

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

mlx4 fix for 36-bit bus addresses on 32-bit arch (was: [PATCH 1/1 (resend)] mthca: Modify to support embedded PowerPC platforms)

[Roland Dreier]

 > I reviewed your change against the latest tree and it looks fine to me.
 > I've tried to get my e-mail / news client set-up properly and hope that I'm
 > now maintaining tab characters!  Thanks for shepherding this through for me.

Thanks, I'll get the mthca patch upstream.  I think the following is
also required -- any review is appreciated.

 - R.

8<--------------------------------------------------

mlx4_{core, ib, en}: Fix driver when sizeof (phys_addr_t) > sizeof (long)

Some systems have PCI addresses that don't fit in unsigned long (eg some
32-bit PowerPC 440 systems have 36-bit bus addresses).  Fix up mlx4 drivers
by using phys_addr_t where appropriate, so we don't truncate any PCI
resource addresses before ioremapping them.

Signed-off-by: Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/main.c |    3 ++-
 drivers/net/mlx4/catas.c          |    6 +++---
 drivers/net/mlx4/en_main.c        |    3 ++-
 drivers/net/mlx4/main.c           |    2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 4c85224..d68d849 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -1005,7 +1005,8 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
 	if (mlx4_uar_alloc(dev, &ibdev->priv_uar))
 		goto err_pd;
 
-	ibdev->uar_map = ioremap(ibdev->priv_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
+	ibdev->uar_map = ioremap((phys_addr_t) ibdev->priv_uar.pfn << PAGE_SHIFT,
+				 PAGE_SIZE);
 	if (!ibdev->uar_map)
 		goto err_uar;
 	MLX4_INIT_DOORBELL_LOCK(&ibdev->uar_lock);
diff --git a/drivers/net/mlx4/catas.c b/drivers/net/mlx4/catas.c
index 68aaa42..32f9471 100644
--- a/drivers/net/mlx4/catas.c
+++ b/drivers/net/mlx4/catas.c
@@ -113,7 +113,7 @@ static void catas_reset(struct work_struct *work)
 void mlx4_start_catas_poll(struct mlx4_dev *dev)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
-	unsigned long addr;
+	phys_addr_t addr;
 
 	INIT_LIST_HEAD(&priv->catas_err.list);
 	init_timer(&priv->catas_err.timer);
@@ -124,8 +124,8 @@ void mlx4_start_catas_poll(struct mlx4_dev *dev)
 
 	priv->catas_err.map = ioremap(addr, priv->fw.catas_size * 4);
 	if (!priv->catas_err.map) {
-		mlx4_warn(dev, "Failed to map internal error buffer at 0x%lx\n",
-			  addr);
+		mlx4_warn(dev, "Failed to map internal error buffer at 0x%llx\n",
+			  (unsigned long long) addr);
 		return;
 	}
 
diff --git a/drivers/net/mlx4/en_main.c b/drivers/net/mlx4/en_main.c
index f6e0d40..1ff6ca6 100644
--- a/drivers/net/mlx4/en_main.c
+++ b/drivers/net/mlx4/en_main.c
@@ -202,7 +202,8 @@ static void *mlx4_en_add(struct mlx4_dev *dev)
 	if (mlx4_uar_alloc(dev, &mdev->priv_uar))
 		goto err_pd;
 
-	mdev->uar_map = ioremap(mdev->priv_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
+	mdev->uar_map = ioremap((phys_addr_t) mdev->priv_uar.pfn << PAGE_SHIFT,
+				PAGE_SIZE);
 	if (!mdev->uar_map)
 		goto err_uar;
 	spin_lock_init(&mdev->uar_lock);
diff --git a/drivers/net/mlx4/main.c b/drivers/net/mlx4/main.c
index 782f11d..4ffdc18 100644
--- a/drivers/net/mlx4/main.c
+++ b/drivers/net/mlx4/main.c
@@ -829,7 +829,7 @@ static int mlx4_setup_hca(struct mlx4_dev *dev)
 		goto err_uar_table_free;
 	}
 
-	priv->kar = ioremap(priv->driver_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
+	priv->kar = ioremap((phys_addr_t) priv->driver_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
 	if (!priv->kar) {
 		mlx4_err(dev, "Couldn't map kernel access region, "
 			 "aborting.\n");
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch

[J.L. Burr]

> Thanks, I'll get the mthca patch upstream.  I think the following is
> also required -- any review is appreciated.

I hadn't realized the newer Mellanox driver had some of its code in drivers/net.  
Yesterday, I had looked at its drivers/infiniband portion and saw that your 
proposed change to hw/mlx4/main.c looked good.

I agree with the changes you've made.    

However, I had a new concern about both the mthca and mlx4 changes so far:  
Are we sure using the phys_addr_t type is the correct thing to do?

I know it is for the PowerPC platform.  When I checked my Linux tree for
phys_addr_t it was only present for PowerPC and Intel architectures.  So,
I was worried that it might not be appropriate for other architectures.

But I now see that the newer trees do define phys_addr_t for all platforms
(defined in linux/types.h).  And, resource_size_t is now defined using
phys_addr_t.  That is the type returned from pci_resource_start(), which 
is where we normally acquire the value to pass to ioremap().

This all happened at 2.6.28, so I'm a victim of running such an old level 
(I'm still at 2.6.26).

I think therefore that all is well with this change.

By the way, the only reason I'm using mthca is because my hardware
has a 4-lane slot.  If I had 8-lanes, I would be trying to use
mlx4.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch

[Jason Gunthorpe]

On Thu, Jan 13, 2011 at 12:06:19PM -0500, J.L. Burr wrote:

> However, I had a new concern about both the mthca and mlx4 changes so far:  
> Are we sure using the phys_addr_t type is the correct thing to do?

I'd say this is right. It looks to me like the choice has been made to
just drop BAR's that have 64 bit addresses if the system is 32
bit. 

Although, while looking into this I wonder J.L., how does your PCI
device even work? Look at, eg pci_read_bridge_mmio_ref, it seems to me
it will bail if a bridge is seen with a 64 bit setting but the system
isn't 64 bit? IMHO, this is a bug, it should be testing for
sizeof(resource_size_t)

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch

[Roland Dreier]

 > Although, while looking into this I wonder J.L., how does your PCI
 > device even work? Look at, eg pci_read_bridge_mmio_ref, it seems to me
 > it will bail if a bridge is seen with a 64 bit setting but the system
 > isn't 64 bit? IMHO, this is a bug, it should be testing for
 > sizeof(resource_size_t)

I don't see that function... where are you looking?

$ git grep pci_read_bridge_mmio_ref
$ git describe
v2.6.37-6858-g581548d

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch

[Jason Gunthorpe]

On Thu, Jan 13, 2011 at 02:10:05PM -0800, Roland Dreier wrote:
>  > Although, while looking into this I wonder J.L., how does your PCI
>  > device even work? Look at, eg pci_read_bridge_mmio_ref, it seems to me
>  > it will bail if a bridge is seen with a 64 bit setting but the system
>  > isn't 64 bit? IMHO, this is a bug, it should be testing for
>  > sizeof(resource_size_t)
> 
> I don't see that function... where are you looking?

Sorry, I typo'd it

static void __devinit pci_read_bridge_mmio_pref(struct pci_bus *child)

drivers/pci/probe.c

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch

[J.L. Burr]

>>  > Although, while looking into this I wonder J.L., how does your PCI
>>  > device even work? Look at, eg pci_read_bridge_mmio_ref, it seems to me
>>  > it will bail if a bridge is seen with a 64 bit setting but the system
>>  > isn't 64 bit? IMHO, this is a bug, it should be testing for
>>  > sizeof(resource_size_t)
>>
>> I don't see that function... where are you looking?
>
> Sorry, I typo'd it
>
> static void __devinit pci_read_bridge_mmio_pref(struct pci_bus *child)
>
> drivers/pci/probe.c

Yes, on my embedded system, Linux does "bail".  That is, during enumeration,
I get the message:

PCI: Unable to handle 64-bit BAR for device 0000:13:00.0

At 2.6.26, pci_read_bridge_mmio_pref was not present, but the same basic
code is (within pci_read_bases() of probe.c).

When first working on getting the petabyte BAR device to work, I did add
some logic to this code so it was not wholly dependent on BITS_PER_LONG
being 64.  I used CONFIG_RESOURCES_64BIT in an "else" clause (although
using your suggestion of sizeof(resource_size_t) should be equivalent).
Starting with 2.6.28, CONFIG_RESOURCES_64BIT has been replaced by
CONFIG_PHYS_ADDR_T_64BIT.

However, I had to comment out my change in this area (i.e. go back to
the original behavior) as there was no way to allocate a petabyte area
within the 440's mappable space.  I believe there is 2GB of PCI mappable
space in the normal 440 setup under Linux.

My scheme (only viable in the controlled environment of an embedded system)
is to allow Linux enumeration to fail for the petabyte BAR device and
use a quirk routine to manually assign both the BAR value for the device
and to adjust the parent PCIe bridges' prefetchable ranges.

2+ years later, that's where I got in trouble when now adding the
Infiniband card to my "controlled environment".  My quirk routine is not
sensitive to the possibility that Linux enumeration may have established
proper 32-bit prefetchable ranges in the bridges.  I'm going to look at
reworking enumeration for my system to assign prefetchable memory at the
end of the 440 mappable area.  That way, the quirk routine should be able
to simply extend the size of any existing prefetchable ranges to include
the large petabyte area.

But back to your point, yes, my petabyte device is not handled by the
2.6.26 Linux enumeration but that's unavoidable for the PowerPC 440.
It means that the Linux kernel does not have a resource object for the
petabyte area, etc.  But that's OK because the 440 CPU can not access
it.  Our device has a smaller "baby BAR" for 440 CPU access to part of
its resources.  The petabyte BAR is used for peer accesses from other
PCIe devices.

I agree with you:  the use of BITS_PER_LONG looks wrong in probe.c for
environments like the PowerPC 440.  You may want to pose your question
in the linux-pci discussion group.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch (was: [PATCH 1/1 (resend)] mthca: Modify to support embedded PowerPC platforms)

[Eli Cohen]

On Wed, Jan 12, 2011 at 09:53:00AM -0800, Roland Dreier wrote:
>  
> -	ibdev->uar_map = ioremap(ibdev->priv_uar.pfn << PAGE_SHIFT, PAGE_SIZE);
> +	ibdev->uar_map = ioremap((phys_addr_t) ibdev->priv_uar.pfn << PAGE_SHIFT,
> +				 PAGE_SIZE);
>  	if (!ibdev->uar_map)
>  		goto err_uar;

CX devices are 64 bit devices with respect to the PCI bus. As far as I
know, a 64 bit PCI device memory space, could be placed anywhere in
that space regardless of the CPU architecture. So I think it would be
more appropriate to to use u64 casting. I know that ioremap accepts
phys_addr_t as its first argument but I wonder if ioremap shouldn't be
changed to accept u64 for that argument.

Regardless of whether what I say above is correct or not, wouldn't it
be nicer to to define pfn as either u64 or phys_addr_t and avoid the
casting?

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch (was: [PATCH 1/1 (resend)] mthca: Modify to support embedded PowerPC platforms)

[Jason Gunthorpe]

On Fri, Jan 14, 2011 at 03:09:03PM +0200, Eli Cohen wrote:

> CX devices are 64 bit devices with respect to the PCI bus. As far as I
> know, a 64 bit PCI device memory space, could be placed anywhere in
> that space regardless of the CPU architecture. So I think it would
> be

Linux limits the addresses of PCI devices to phys_addr_t, so if your
device is placed outside phys_addr_t by the BIOS then it will be
discarded during discovery. Thus phys_addr_t/resource_size_t is
appropriate for storing BAR addresses from PCI devices.

> Regardless of whether what I say above is correct or not, wouldn't it
> be nicer to to define pfn as either u64 or phys_addr_t and avoid the
> casting?

Good point, the pfn must be stored as phys_addr_t too, otherwise you
only support a 44 bit physical address space before truncatation
occures.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch

[Roland Dreier]

 > > Regardless of whether what I say above is correct or not, wouldn't it
 > > be nicer to to define pfn as either u64 or phys_addr_t and avoid the
 > > casting?
 > 
 > Good point, the pfn must be stored as phys_addr_t too, otherwise you
 > only support a 44 bit physical address space before truncatation
 > occures.

Not sure I agree at the moment... eg remap_pfn_range takes unsigned
long.  So I don't think things will really work on a 32-bit arch with >
44 bit bus addresses for lots of other reasons.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mlx4 fix for 36-bit bus addresses on 32-bit arch

[Jason Gunthorpe]

On Fri, Jan 14, 2011 at 01:26:33PM -0800, Roland Dreier wrote:
>  > > Regardless of whether what I say above is correct or not, wouldn't it
>  > > be nicer to to define pfn as either u64 or phys_addr_t and avoid the
>  > > casting?
>  > 
>  > Good point, the pfn must be stored as phys_addr_t too, otherwise you
>  > only support a 44 bit physical address space before truncatation
>  > occures.
> 
> Not sure I agree at the moment... eg remap_pfn_range takes unsigned
> long.  So I don't think things will really work on a 32-bit arch with >
> 44 bit bus addresses for lots of other reasons.

Hmm, fair point. It does look to me like pfns are generally (always?)
stored as unsigned long, so there is no attempt to support more
physical bits than about 44 or so on 32 bit. But I also don't know of
any places that actually check for this when processing BARs... :)

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html