| Message ID | 20230929-mlx5_init_fix-v2-1-51ed2094c9d8@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net/mlx5: fix calling mlx5_cmd_init() before DMA mask is set | expand |
On 9/29/2023 5:15 AM, Niklas Schnelle wrote: > Since commit 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and > reload routines") mlx5_cmd_init() is called in mlx5_mdev_init() which is > called in probe_one() before mlx5_pci_init(). This is a problem because > mlx5_pci_init() is where the DMA and coherent mask is set but > mlx5_cmd_init() already does a dma_alloc_coherent(). Thus a DMA > allocation is done during probe before the correct mask is set. This > causes probe to fail initialization of the cmdif SW structs on s390x > after that is converted to the common dma-iommu code. This is because on > s390x DMA addresses below 4 GiB are reserved on current machines and > unlike the old s390x specific DMA API implementation common code > enforces DMA masks. > > Fix this by moving set_dma_caps() out of mlx5_pci_init() and into > probe_one() before mlx5_mdev_init(). To match the overall naming scheme > rename it to mlx5_dma_init(). > > Link: https://lore.kernel.org/linux-iommu/cfc9e9128ed5571d2e36421e347301057662a09e.camel@linux.ibm.com/ > Fixes: 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and reload routines") > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > --- > Note: I ran into this while testing the linked series for converting > s390x to use dma-iommu. The existing s390x specific DMA API > implementation doesn't respect DMA masks and is thus not affected > despite of course also only supporting DMA addresses above 4 GiB. > --- Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
On Fri, Sep 29, 2023 at 02:15:49PM +0200, Niklas Schnelle wrote: > Since commit 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and > reload routines") mlx5_cmd_init() is called in mlx5_mdev_init() which is > called in probe_one() before mlx5_pci_init(). This is a problem because > mlx5_pci_init() is where the DMA and coherent mask is set but > mlx5_cmd_init() already does a dma_alloc_coherent(). Thus a DMA > allocation is done during probe before the correct mask is set. This > causes probe to fail initialization of the cmdif SW structs on s390x > after that is converted to the common dma-iommu code. This is because on > s390x DMA addresses below 4 GiB are reserved on current machines and > unlike the old s390x specific DMA API implementation common code > enforces DMA masks. > > Fix this by moving set_dma_caps() out of mlx5_pci_init() and into > probe_one() before mlx5_mdev_init(). To match the overall naming scheme > rename it to mlx5_dma_init(). > > Link: https://lore.kernel.org/linux-iommu/cfc9e9128ed5571d2e36421e347301057662a09e.camel@linux.ibm.com/ > Fixes: 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and reload routines") > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > --- > Note: I ran into this while testing the linked series for converting > s390x to use dma-iommu. The existing s390x specific DMA API > implementation doesn't respect DMA masks and is thus not affected > despite of course also only supporting DMA addresses above 4 GiB. > --- > Changes in v2: > - Instead of moving the whole mlx5_pci_init() only move the > set_dma_caps() call so as to keep pci_enable_device() after the FW > command interface initialization (Leon) > - Link to v1: https://lore.kernel.org/r/20230928-mlx5_init_fix-v1-1-79749d45ce60@linux.ibm.com > --- > drivers/net/ethernet/mellanox/mlx5/core/main.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > Thanks, Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
On Sat, 2023-09-30 at 10:36 +0300, Leon Romanovsky wrote: > On Fri, Sep 29, 2023 at 02:15:49PM +0200, Niklas Schnelle wrote: > > Since commit 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and > > reload routines") mlx5_cmd_init() is called in mlx5_mdev_init() which is > > called in probe_one() before mlx5_pci_init(). This is a problem because > > mlx5_pci_init() is where the DMA and coherent mask is set but > > mlx5_cmd_init() already does a dma_alloc_coherent(). Thus a DMA > > allocation is done during probe before the correct mask is set. This > > causes probe to fail initialization of the cmdif SW structs on s390x > > after that is converted to the common dma-iommu code. This is because on > > s390x DMA addresses below 4 GiB are reserved on current machines and > > unlike the old s390x specific DMA API implementation common code > > enforces DMA masks. > > > > Fix this by moving set_dma_caps() out of mlx5_pci_init() and into > > probe_one() before mlx5_mdev_init(). To match the overall naming scheme > > rename it to mlx5_dma_init(). > > > > Link: https://lore.kernel.org/linux-iommu/cfc9e9128ed5571d2e36421e347301057662a09e.camel@linux.ibm.com/ > > Fixes: 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and reload routines") > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > > --- > > Note: I ran into this while testing the linked series for converting > > s390x to use dma-iommu. The existing s390x specific DMA API > > implementation doesn't respect DMA masks and is thus not affected > > despite of course also only supporting DMA addresses above 4 GiB. > > --- > > Changes in v2: > > - Instead of moving the whole mlx5_pci_init() only move the > > set_dma_caps() call so as to keep pci_enable_device() after the FW > > command interface initialization (Leon) > > - Link to v1: https://lore.kernel.org/r/20230928-mlx5_init_fix-v1-1-79749d45ce60@linux.ibm.com > > --- > > drivers/net/ethernet/mellanox/mlx5/core/main.c | 18 +++++++++--------- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > Thanks, > Reviewed-by: Leon Romanovsky <leonro@nvidia.com> Thank you for the review. Assuming the mlx5 tree is included in linux- next I think it would be easiest if this goes via that tree thereby unbreaking linux-next for s390. Or do you prefer Joerg to take this via the IOMMU tree or even some other tree? Thanks, Niklas
On Wed, Oct 04, 2023 at 02:40:49PM +0200, Niklas Schnelle wrote: > On Sat, 2023-09-30 at 10:36 +0300, Leon Romanovsky wrote: > > On Fri, Sep 29, 2023 at 02:15:49PM +0200, Niklas Schnelle wrote: > > > Since commit 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and > > > reload routines") mlx5_cmd_init() is called in mlx5_mdev_init() which is > > > called in probe_one() before mlx5_pci_init(). This is a problem because > > > mlx5_pci_init() is where the DMA and coherent mask is set but > > > mlx5_cmd_init() already does a dma_alloc_coherent(). Thus a DMA > > > allocation is done during probe before the correct mask is set. This > > > causes probe to fail initialization of the cmdif SW structs on s390x > > > after that is converted to the common dma-iommu code. This is because on > > > s390x DMA addresses below 4 GiB are reserved on current machines and > > > unlike the old s390x specific DMA API implementation common code > > > enforces DMA masks. > > > > > > Fix this by moving set_dma_caps() out of mlx5_pci_init() and into > > > probe_one() before mlx5_mdev_init(). To match the overall naming scheme > > > rename it to mlx5_dma_init(). > > > > > > Link: https://lore.kernel.org/linux-iommu/cfc9e9128ed5571d2e36421e347301057662a09e.camel@linux.ibm.com/ > > > Fixes: 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and reload routines") > > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > > > --- > > > Note: I ran into this while testing the linked series for converting > > > s390x to use dma-iommu. The existing s390x specific DMA API > > > implementation doesn't respect DMA masks and is thus not affected > > > despite of course also only supporting DMA addresses above 4 GiB. > > > --- > > > Changes in v2: > > > - Instead of moving the whole mlx5_pci_init() only move the > > > set_dma_caps() call so as to keep pci_enable_device() after the FW > > > command interface initialization (Leon) > > > - Link to v1: https://lore.kernel.org/r/20230928-mlx5_init_fix-v1-1-79749d45ce60@linux.ibm.com > > > --- > > > drivers/net/ethernet/mellanox/mlx5/core/main.c | 18 +++++++++--------- > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > Thanks, > > Reviewed-by: Leon Romanovsky <leonro@nvidia.com> > > Thank you for the review. Assuming the mlx5 tree is included in linux- > next I think it would be easiest if this goes via that tree thereby > unbreaking linux-next for s390. Or do you prefer Joerg to take this via > the IOMMU tree or even some other tree? Strictly speaking this is net patch, netdev maintainers should pick it. We use mlx5 tree [1] for *-next material. Thanks [1] https://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git/ > > Thanks, > Niklas
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c index 15561965d2af..f251d233a16c 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c @@ -250,7 +250,7 @@ static void mlx5_set_driver_version(struct mlx5_core_dev *dev) mlx5_cmd_exec_in(dev, set_driver_version, in); } -static int set_dma_caps(struct pci_dev *pdev) +static int mlx5_dma_init(struct pci_dev *pdev) { int err; @@ -905,12 +905,6 @@ static int mlx5_pci_init(struct mlx5_core_dev *dev, struct pci_dev *pdev, pci_set_master(pdev); - err = set_dma_caps(pdev); - if (err) { - mlx5_core_err(dev, "Failed setting DMA capabilities mask, aborting\n"); - goto err_clr_master; - } - if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32) && pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP64) && pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP128)) @@ -1908,9 +1902,15 @@ static int probe_one(struct pci_dev *pdev, const struct pci_device_id *id) goto adev_init_err; } + err = mlx5_dma_init(pdev); + if (err) { + mlx5_core_err(dev, "Failed setting DMA capabilities mask, aborting\n"); + goto dma_init_err; + } + err = mlx5_mdev_init(dev, prof_sel); if (err) - goto mdev_init_err; + goto dma_init_err; err = mlx5_pci_init(dev, pdev, id); if (err) { @@ -1942,7 +1942,7 @@ static int probe_one(struct pci_dev *pdev, const struct pci_device_id *id) mlx5_pci_close(dev); pci_init_err: mlx5_mdev_uninit(dev); -mdev_init_err: +dma_init_err: mlx5_adev_idx_free(dev->priv.adev_idx); adev_init_err: mlx5_devlink_free(devlink);
Since commit 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and reload routines") mlx5_cmd_init() is called in mlx5_mdev_init() which is called in probe_one() before mlx5_pci_init(). This is a problem because mlx5_pci_init() is where the DMA and coherent mask is set but mlx5_cmd_init() already does a dma_alloc_coherent(). Thus a DMA allocation is done during probe before the correct mask is set. This causes probe to fail initialization of the cmdif SW structs on s390x after that is converted to the common dma-iommu code. This is because on s390x DMA addresses below 4 GiB are reserved on current machines and unlike the old s390x specific DMA API implementation common code enforces DMA masks. Fix this by moving set_dma_caps() out of mlx5_pci_init() and into probe_one() before mlx5_mdev_init(). To match the overall naming scheme rename it to mlx5_dma_init(). Link: https://lore.kernel.org/linux-iommu/cfc9e9128ed5571d2e36421e347301057662a09e.camel@linux.ibm.com/ Fixes: 06cd555f73ca ("net/mlx5: split mlx5_cmd_init() to probe and reload routines") Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> --- Note: I ran into this while testing the linked series for converting s390x to use dma-iommu. The existing s390x specific DMA API implementation doesn't respect DMA masks and is thus not affected despite of course also only supporting DMA addresses above 4 GiB. --- Changes in v2: - Instead of moving the whole mlx5_pci_init() only move the set_dma_caps() call so as to keep pci_enable_device() after the FW command interface initialization (Leon) - Link to v1: https://lore.kernel.org/r/20230928-mlx5_init_fix-v1-1-79749d45ce60@linux.ibm.com --- drivers/net/ethernet/mellanox/mlx5/core/main.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) --- base-commit: 6465e260f48790807eef06b583b38ca9789b6072 change-id: 20230928-mlx5_init_fix-c465b5cda327 Best regards,