| Message ID | 20240131191732.3247996-3-cleech@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | UIO_MEM_DMA_COHERENT for cnic/bnx2/bnx2x | expand |
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 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.
diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c index 0d917a9699c58..b65b8592ad759 100644 --- a/drivers/net/ethernet/broadcom/bnx2.c +++ b/drivers/net/ethernet/broadcom/bnx2.c @@ -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; } diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index 0d8e61c63c7c6..678829646cec3 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c @@ -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; diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c index 7926aaef8f0c5..cca1e94fc35dc 100644 --- a/drivers/net/ethernet/broadcom/cnic.c +++ b/drivers/net/ethernet/broadcom/cnic.c @@ -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); diff --git a/drivers/net/ethernet/broadcom/cnic.h b/drivers/net/ethernet/broadcom/cnic.h index 4baea81bae7a3..fedc84ada937d 100644 --- a/drivers/net/ethernet/broadcom/cnic.h +++ b/drivers/net/ethernet/broadcom/cnic.h @@ -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; diff --git a/drivers/net/ethernet/broadcom/cnic_if.h b/drivers/net/ethernet/broadcom/cnic_if.h index 789e5c7e93116..49a11ec80b364 100644 --- a/drivers/net/ethernet/broadcom/cnic_if.h +++ b/drivers/net/ethernet/broadcom/cnic_if.h @@ -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;
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(-)