[2/2] cnic,bnx2,bnx2x: use UIO_MEM_DMA_COHERENT
Commit Message
Use the UIO_MEM_DMA_COHERENT type to properly handle mmap for
dma_alloc_coherent buffers.
The cnic l2_ring and l2_buf mmaps have caused page refcount issues as
the dma_alloc_coherent no longer provide __GFP_COMP allocation as per
commit "dma-mapping: reject __GFP_COMP in dma_alloc_attrs".
Fix this by having the uio device use dma_mmap_coherent.
The bnx2 and bnx2x status block allocations are also dma_alloc_coherent,
and should use dma_mmap_coherent. They don't allocate multiple pages,
but this interface does not work correctly with an iommu enabled unless
dma_mmap_coherent is used.
Fixes: bb73955c0b1d ("cnic: don't pass bogus GFP_ flags to dma_alloc_coherent")
Signed-off-by: Nilesh Javali <njavali@marvell.com>
Signed-off-by: Chris Leech <cleech@redhat.com>
---
drivers/net/ethernet/broadcom/bnx2.c | 1 +
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 2 ++
drivers/net/ethernet/broadcom/cnic.c | 15 ++++++++++++---
drivers/net/ethernet/broadcom/cnic.h | 1 +
drivers/net/ethernet/broadcom/cnic_if.h | 1 +
5 files changed, 17 insertions(+), 3 deletions(-)
Comments
On Wed, Jan 31, 2024 at 11:17:32AM -0800, Chris Leech wrote:
> Use the UIO_MEM_DMA_COHERENT type to properly handle mmap for
> dma_alloc_coherent buffers.
>
> The cnic l2_ring and l2_buf mmaps have caused page refcount issues as
> the dma_alloc_coherent no longer provide __GFP_COMP allocation as per
> commit "dma-mapping: reject __GFP_COMP in dma_alloc_attrs".
>
> Fix this by having the uio device use dma_mmap_coherent.
>
> The bnx2 and bnx2x status block allocations are also dma_alloc_coherent,
> and should use dma_mmap_coherent. They don't allocate multiple pages,
> but this interface does not work correctly with an iommu enabled unless
> dma_mmap_coherent is used.
>
> Fixes: bb73955c0b1d ("cnic: don't pass bogus GFP_ flags to dma_alloc_coherent")
This is really the commit that broke things? By adding this, are you
expecting anyone to backport this change to older kernels?
thanks,
greg k-h
On Wed, Jan 31, 2024 at 1:30 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 31, 2024 at 11:17:32AM -0800, Chris Leech wrote:
> > Use the UIO_MEM_DMA_COHERENT type to properly handle mmap for
> > dma_alloc_coherent buffers.
> >
> > The cnic l2_ring and l2_buf mmaps have caused page refcount issues as
> > the dma_alloc_coherent no longer provide __GFP_COMP allocation as per
> > commit "dma-mapping: reject __GFP_COMP in dma_alloc_attrs".
> >
> > Fix this by having the uio device use dma_mmap_coherent.
> >
> > The bnx2 and bnx2x status block allocations are also dma_alloc_coherent,
> > and should use dma_mmap_coherent. They don't allocate multiple pages,
> > but this interface does not work correctly with an iommu enabled unless
> > dma_mmap_coherent is used.
> >
> > Fixes: bb73955c0b1d ("cnic: don't pass bogus GFP_ flags to dma_alloc_coherent")
>
> This is really the commit that broke things? By adding this, are you
> expecting anyone to backport this change to older kernels?
That's certainly where things stopped working altogether, iommu issues
go back further.
- Chris
On Wed, Jan 31, 2024 at 01:29:56PM -0800, Greg Kroah-Hartman wrote:
> On Wed, Jan 31, 2024 at 11:17:32AM -0800, Chris Leech wrote:
> > Use the UIO_MEM_DMA_COHERENT type to properly handle mmap for
> > dma_alloc_coherent buffers.
> >
> > The cnic l2_ring and l2_buf mmaps have caused page refcount issues as
> > the dma_alloc_coherent no longer provide __GFP_COMP allocation as per
> > commit "dma-mapping: reject __GFP_COMP in dma_alloc_attrs".
> >
> > Fix this by having the uio device use dma_mmap_coherent.
> >
> > The bnx2 and bnx2x status block allocations are also dma_alloc_coherent,
> > and should use dma_mmap_coherent. They don't allocate multiple pages,
> > but this interface does not work correctly with an iommu enabled unless
> > dma_mmap_coherent is used.
> >
> > Fixes: bb73955c0b1d ("cnic: don't pass bogus GFP_ flags to dma_alloc_coherent")
>
> This is really the commit that broke things? By adding this, are you
> expecting anyone to backport this change to older kernels?
Well, the driver has literally been broken since day 1. The above
commit is what made people finally care as it also broke on more
common setups. So I'm not sure the fixes tag is correct.
@@ -367,6 +367,7 @@ static void bnx2_setup_cnic_irq_info(struct bnx2 *bp)
cp->irq_arr[0].status_blk = (void *)
((unsigned long) bnapi->status_blk.msi +
(BNX2_SBLK_MSIX_ALIGN_SIZE * sb_id));
+ cp->irq_arr[0].status_blk_map = bp->status_blk_mapping;
cp->irq_arr[0].status_blk_num = sb_id;
cp->num_irq = 1;
}
@@ -14912,9 +14912,11 @@ void bnx2x_setup_cnic_irq_info(struct bnx2x *bp)
else
cp->irq_arr[0].status_blk = (void *)bp->cnic_sb.e1x_sb;
+ cp->irq_arr[0].status_blk_map = bp->cnic_sb_mapping;
cp->irq_arr[0].status_blk_num = bnx2x_cnic_fw_sb_id(bp);
cp->irq_arr[0].status_blk_num2 = bnx2x_cnic_igu_sb_id(bp);
cp->irq_arr[1].status_blk = bp->def_status_blk;
+ cp->irq_arr[1].status_blk_map = bp->def_status_blk_mapping;
cp->irq_arr[1].status_blk_num = DEF_SB_ID;
cp->irq_arr[1].status_blk_num2 = DEF_SB_IGU_ID;
@@ -1107,6 +1107,7 @@ static int cnic_init_uio(struct cnic_dev *dev)
TX_MAX_TSS_RINGS + 1);
uinfo->mem[1].addr = (unsigned long) cp->status_blk.gen &
CNIC_PAGE_MASK;
+ uinfo->mem[1].dma_addr = cp->status_blk_map;
if (cp->ethdev->drv_state & CNIC_DRV_STATE_USING_MSIX)
uinfo->mem[1].size = BNX2_SBLK_MSIX_ALIGN_SIZE * 9;
else
@@ -1118,20 +1119,26 @@ static int cnic_init_uio(struct cnic_dev *dev)
uinfo->mem[1].addr = (unsigned long) cp->bnx2x_def_status_blk &
CNIC_PAGE_MASK;
+ uinfo->mem[1].dma_addr = cp->status_blk_map;
uinfo->mem[1].size = sizeof(*cp->bnx2x_def_status_blk);
uinfo->name = "bnx2x_cnic";
}
- uinfo->mem[1].memtype = UIO_MEM_LOGICAL;
+ uinfo->mem[1].dma_device = &dev->pcidev->dev;
+ uinfo->mem[1].memtype = UIO_MEM_DMA_COHERENT;
uinfo->mem[2].addr = (unsigned long) udev->l2_ring;
+ uinfo->mem[2].dma_addr = udev->l2_ring_map;
uinfo->mem[2].size = udev->l2_ring_size;
- uinfo->mem[2].memtype = UIO_MEM_LOGICAL;
+ uinfo->mem[2].dma_device = &dev->pcidev->dev;
+ uinfo->mem[2].memtype = UIO_MEM_DMA_COHERENT;
uinfo->mem[3].addr = (unsigned long) udev->l2_buf;
+ uinfo->mem[3].dma_addr = udev->l2_buf_map;
uinfo->mem[3].size = udev->l2_buf_size;
- uinfo->mem[3].memtype = UIO_MEM_LOGICAL;
+ uinfo->mem[3].dma_device = &dev->pcidev->dev;
+ uinfo->mem[3].memtype = UIO_MEM_DMA_COHERENT;
uinfo->version = CNIC_MODULE_VERSION;
uinfo->irq = UIO_IRQ_CUSTOM;
@@ -1313,6 +1320,7 @@ static int cnic_alloc_bnx2x_resc(struct cnic_dev *dev)
return 0;
cp->bnx2x_def_status_blk = cp->ethdev->irq_arr[1].status_blk;
+ cp->status_blk_map = cp->ethdev->irq_arr[1].status_blk_map;
cp->l2_rx_ring_size = 15;
@@ -5323,6 +5331,7 @@ static int cnic_start_hw(struct cnic_dev *dev)
pci_dev_get(dev->pcidev);
cp->func = PCI_FUNC(dev->pcidev->devfn);
cp->status_blk.gen = ethdev->irq_arr[0].status_blk;
+ cp->status_blk_map = ethdev->irq_arr[0].status_blk_map;
cp->status_blk_num = ethdev->irq_arr[0].status_blk_num;
err = cp->alloc_resc(dev);
@@ -260,6 +260,7 @@ struct cnic_local {
#define SM_RX_ID 0
#define SM_TX_ID 1
} status_blk;
+ dma_addr_t status_blk_map;
struct host_sp_status_block *bnx2x_def_status_blk;
@@ -190,6 +190,7 @@ struct cnic_ops {
struct cnic_irq {
unsigned int vector;
void *status_blk;
+ dma_addr_t status_blk_map;
u32 status_blk_num;
u32 status_blk_num2;
u32 irq_flags;