Skip to contentSkip to conversation

NTB: Set dma mask and dma coherent mask to NTB devices

PATCH In review Serge Semin 2 participants 2 messages Nov 30, 2017

Subsystems (3)
NTB driver core NTB Intel driver NTB AMD driver
lore
Share & tools
lore

Serge Semin

author
#0 lore raw

PATCH 1/1 NTB: Set dma mask and dma coherent mask to NTB devices

The dma_mask and dma_coherent_mask fields of the NTB struct device weren't initialized in hardware drivers. In fact it should be done instead of PCIe interface usage, since NTB clients are supposed to use NTB API and left unaware of real hardware implementation. In addition to that ntb_device_register() method shouldn't clear the passed ntb_dev structure, since it dma_mask is initialized by hardware drivers.

Signed-off-by Serge Semin <[email protected]>
---
 drivers/ntb/hw/amd/ntb_hw_amd.c     | 4 ++++
 drivers/ntb/hw/idt/ntb_hw_idt.c     | 8 +++++++-
 drivers/ntb/hw/intel/ntb_hw_intel.c | 4 ++++
 drivers/ntb/ntb.c                   | 1 -
 4 files changed, 15 insertions(+), 2 deletions(-)
drivers/ntb/hw/amd/ntb_hw_amd.c +4 -0
index f0788aae05c9..3cfa46876239 100644
@@ -1020,6 +1020,10 @@ static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
 			goto err_dma_mask;
 		dev_warn(&pdev->dev, "Cannot DMA consistent highmem\n");
 	}
+	rc = dma_coerce_mask_and_coherent(&ndev->ntb.dev,
+					  dma_get_mask(&pdev->dev));
+	if (rc)
+		goto err_dma_mask;
 
 	ndev->self_mmio = pci_iomap(pdev, 0, 0);
 	if (!ndev->self_mmio) {
drivers/ntb/hw/idt/ntb_hw_idt.c +7 -1
index 6fb87c0f0d97..d87dbd4bc82c 100644
@@ -2426,7 +2426,7 @@ static int idt_init_pci(struct idt_ntb_dev *ndev)
 	struct pci_dev *pdev = ndev->ntb.pdev;
 	int ret;
 
-	/* Initialize the bit mask of DMA */
+	/* Initialize the bit mask of PCI/NTB DMA */
 	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
 	if (ret != 0) {
 		ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));

@@ -2447,6 +2447,12 @@ static int idt_init_pci(struct idt_ntb_dev *ndev)
 		dev_warn(&pdev->dev,
 			"Cannot set consistent DMA highmem bit mask\n");
 	}
+	ret = dma_coerce_mask_and_coherent(&ndev->ntb.dev,
+					   dma_get_mask(&pdev->dev));
+	if (ret != 0) {
+		dev_err(&pdev->dev, "Failed to set NTB device DMA bit mask\n");
+		return ret;
+	}
 
 	/*
 	 * Enable the device advanced error reporting. It's not critical to
drivers/ntb/hw/intel/ntb_hw_intel.c +4 -0
index 4de074a86073..ea6c82f8655f 100644
@@ -2586,6 +2586,10 @@ static int intel_ntb_init_pci(struct intel_ntb_dev *ndev, struct pci_dev *pdev)
 			goto err_dma_mask;
 		dev_warn(&pdev->dev, "Cannot DMA consistent highmem\n");
 	}
+	rc = dma_coerce_mask_and_coherent(&ndev->ntb.dev,
+					  dma_get_mask(&pdev->dev));
+	if (rc)
+		goto err_dma_mask;
 
 	ndev->self_mmio = pci_iomap(pdev, 0, 0);
 	if (!ndev->self_mmio) {
drivers/ntb/ntb.c +0 -1
index 03b80d89b980..79e50a5f8902 100644
@@ -112,7 +112,6 @@ int ntb_register_device(struct ntb_dev *ntb)
 
 	init_completion(&ntb->released);
 
-	memset(&ntb->dev, 0, sizeof(ntb->dev));
 	ntb->dev.bus = &ntb_bus;
 	ntb->dev.parent = &ntb->pdev->dev;
 	ntb->dev.release = ntb_dev_release;
Signature · 1 line
2.12.0
#1 lore raw

drivers/ntb/hw/amd/ntb_hw_amd.c static int amd_ntb_init_pci(struct amd_ntb_dev *ndev, · line 1026

Quoted 12 lines · Serge Semin
drivers/ntb/hw/amd/ntb_hw_amd.c +4 -0
index f0788aae05c9..3cfa46876239 100644
@@ -1020,6 +1020,10 @@ static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
  			goto err_dma_mask;
  		dev_warn(&pdev->dev, "Cannot DMA consistent highmem\n");
  	}
+	rc = dma_coerce_mask_and_coherent(&ndev->ntb.dev,
+					  dma_get_mask(&pdev->dev));
+	if (rc)
+		goto err_dma_mask;

I can't say I understand fully what this is solving. Perhaps you could explain it a little further in the commit message.

In any case, to me, it looks like it would be better in ntb_register_device() instead of every single driver.

drivers/ntb/ntb.c int ntb_register_device(struct ntb_dev *ntb) · line 117

Quoted 13 lines
drivers/ntb/ntb.c +0 -1
index 03b80d89b980..79e50a5f8902 100644
@@ -112,7 +112,6 @@ int ntb_register_device(struct ntb_dev *ntb)
 
  	init_completion(&ntb->released);
 
-	memset(&ntb->dev, 0, sizeof(ntb->dev));
  	ntb->dev.bus = &ntb_bus;
  	ntb->dev.parent = &ntb->pdev->dev;
  	ntb->dev.release = ntb_dev_release;

This is unrelated to above. I agree it should probably be removed but I'd suggest it should be in its own commit with its own justification.

Thanks,

Logan