linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses
@ 2022-08-12  0:03 Will McVicker
  2022-08-12  0:03 ` [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32 Will McVicker
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Will McVicker @ 2022-08-12  0:03 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, Will McVicker
  Cc: kernel-team, Vidya Sagar, Christoph Hellwig, linux-pci, linux-kernel

Hi All,

I've updated the series to address the review comments. Refer to the v4
history below for the changes. Please take a look and thanks again for the
reviews!

Regards,
Will

Will McVicker (2):
  PCI: dwc: Drop dependency on ZONE_DMA32
  PCI: dwc: Add support for 64-bit MSI target address

v4:
 * Updated commit descriptions.
 * Renamed msi_64b -> msi_64bit.
 * Dropped msi_64bit ternary use.
 * Dropped export of dw_pcie_msi_capabilities.

v3:
  * Switched to a managed DMA allocation.
  * Simplified the DMA allocation cleanup.
  * Dropped msi_page from struct dw_pcie_rp.
  * Allocating a u64 instead of a full page.

v2:
  * Fixed build error caught by kernel test robot
  * Fixed error handling reported by Isaac Manjarres

 .../pci/controller/dwc/pcie-designware-host.c | 42 +++++++++----------
 drivers/pci/controller/dwc/pcie-designware.c  |  8 ++++
 drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
 3 files changed, 28 insertions(+), 24 deletions(-)


base-commit: 2ae08b36c06ea8df73a79f6b80ff7964e006e9e3
-- 
2.37.1.559.g78731f0fdb-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32
  2022-08-12  0:03 [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses Will McVicker
@ 2022-08-12  0:03 ` Will McVicker
  2022-08-12  2:47   ` Han Jingoo
  2022-08-12 18:21   ` Rob Herring
  2022-08-12  0:03 ` [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address Will McVicker
  2022-08-23 10:01 ` [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses Lorenzo Pieralisi
  2 siblings, 2 replies; 9+ messages in thread
From: Will McVicker @ 2022-08-12  0:03 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, Will McVicker
  Cc: kernel-team, Vidya Sagar, Christoph Hellwig, linux-pci,
	linux-kernel, Isaac J . Manjarres

Re-work the msi_msg DMA allocation logic to use dmam_alloc_coherent() which
uses the coherent DMA mask to try to return an allocation within the DMA
mask limits. With that, we now can drop the msi_page parameter in struct
dw_pcie_rp. This allows kernel configurations that disable ZONE_DMA32 to
continue supporting a 32-bit DMA mask. Without this patch, the PCIe host
device will fail to probe when ZONE_DMA32 is disabled.

Fixes: 35797e672ff0 ("PCI: dwc: Fix MSI msi_msg DMA mapping")
Reported-by: Isaac J. Manjarres <isaacmanjarres@google.com>
Signed-off-by: Will McVicker <willmcvicker@google.com>
---
 .../pci/controller/dwc/pcie-designware-host.c | 28 +++++--------------
 drivers/pci/controller/dwc/pcie-designware.h  |  1 -
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 7746f94a715f..39f3b37d4033 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -267,15 +267,6 @@ static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
 
 	irq_domain_remove(pp->msi_domain);
 	irq_domain_remove(pp->irq_domain);
-
-	if (pp->msi_data) {
-		struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
-		struct device *dev = pci->dev;
-
-		dma_unmap_page(dev, pp->msi_data, PAGE_SIZE, DMA_FROM_DEVICE);
-		if (pp->msi_page)
-			__free_page(pp->msi_page);
-	}
 }
 
 static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
@@ -336,6 +327,7 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct device *dev = pci->dev;
 	struct platform_device *pdev = to_platform_device(dev);
+	u64 *msi_vaddr;
 	int ret;
 	u32 ctrl, num_ctrls;
 
@@ -375,22 +367,16 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
 						    dw_chained_msi_isr, pp);
 	}
 
-	ret = dma_set_mask(dev, DMA_BIT_MASK(32));
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
 	if (ret)
 		dev_warn(dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
 
-	pp->msi_page = alloc_page(GFP_DMA32);
-	pp->msi_data = dma_map_page(dev, pp->msi_page, 0,
-				    PAGE_SIZE, DMA_FROM_DEVICE);
-	ret = dma_mapping_error(dev, pp->msi_data);
-	if (ret) {
-		dev_err(pci->dev, "Failed to map MSI data\n");
-		__free_page(pp->msi_page);
-		pp->msi_page = NULL;
-		pp->msi_data = 0;
+	msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
+					GFP_KERNEL);
+	if (!msi_vaddr) {
+		dev_err(dev, "Failed to alloc and map MSI data\n");
 		dw_pcie_free_msi(pp);
-
-		return ret;
+		return -ENOMEM;
 	}
 
 	return 0;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 09b887093a84..a871ae7eb59e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -243,7 +243,6 @@ struct dw_pcie_rp {
 	struct irq_domain	*irq_domain;
 	struct irq_domain	*msi_domain;
 	dma_addr_t		msi_data;
-	struct page		*msi_page;
 	struct irq_chip		*msi_irq_chip;
 	u32			num_vectors;
 	u32			irq_mask[MAX_MSI_CTRLS];
-- 
2.37.1.559.g78731f0fdb-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address
  2022-08-12  0:03 [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses Will McVicker
  2022-08-12  0:03 ` [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32 Will McVicker
@ 2022-08-12  0:03 ` Will McVicker
  2022-08-12  2:46   ` Han Jingoo
  2022-08-23 11:27   ` Robin Murphy
  2022-08-23 10:01 ` [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses Lorenzo Pieralisi
  2 siblings, 2 replies; 9+ messages in thread
From: Will McVicker @ 2022-08-12  0:03 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, Will McVicker
  Cc: kernel-team, Vidya Sagar, Christoph Hellwig, linux-pci,
	linux-kernel, kernel test robot

Since not all devices require a 32-bit MSI address, add support to the
PCIe host driver to allow setting the DMA mask to 64-bits. This allows
kernels to disable ZONE_DMA32 and bounce buffering (swiotlb) without
risking not being able to get a 32-bit address during DMA allocation.
Basically, in the slim chance that there are no 32-bit allocations
available, the current PCIe host driver will fail to allocate the
msi_msg page due to a DMA address overflow (seen in [1]). With this
patch, the PCIe device can advertise 64-bit support via its MSI
capabilities to hint to the PCIe host driver to set the DMA mask to
64-bits.

[1] https://lore.kernel.org/all/Yo0soniFborDl7+C@google.com/

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Will McVicker <willmcvicker@google.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 14 ++++++++++++--
 drivers/pci/controller/dwc/pcie-designware.c      |  8 ++++++++
 drivers/pci/controller/dwc/pcie-designware.h      |  1 +
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 39f3b37d4033..d7831040791b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -330,6 +330,8 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
 	u64 *msi_vaddr;
 	int ret;
 	u32 ctrl, num_ctrls;
+	bool msi_64bit = false;
+	u16 msi_capabilities;
 
 	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
 		pp->irq_mask[ctrl] = ~0;
@@ -367,9 +369,17 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
 						    dw_chained_msi_isr, pp);
 	}
 
-	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	msi_capabilities = dw_pcie_msi_capabilities(pci);
+	if (msi_capabilities & PCI_MSI_FLAGS_ENABLE)
+		msi_64bit = msi_capabilities & PCI_MSI_FLAGS_64BIT;
+
+	dev_dbg(dev, "Setting MSI DMA mask to %s-bit.\n",
+		msi_64bit ? "64" : "32");
+	ret = dma_set_mask_and_coherent(dev, msi_64bit ?
+					DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
 	if (ret)
-		dev_warn(dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
+		dev_warn(dev, "Failed to set DMA mask to %s-bit.\n",
+			 msi_64bit ? "64" : "32");
 
 	msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
 					GFP_KERNEL);
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c6725c519a47..650a7f22f9d0 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -82,6 +82,14 @@ u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap)
 }
 EXPORT_SYMBOL_GPL(dw_pcie_find_capability);
 
+u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
+{
+	u8 offset;
+
+	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
+	return dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
+}
+
 static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
 					    u8 cap)
 {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index a871ae7eb59e..45fcdfc8c035 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -332,6 +332,7 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
 
 u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
 u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);
+u16 dw_pcie_msi_capabilities(struct dw_pcie *pci);
 
 int dw_pcie_read(void __iomem *addr, int size, u32 *val);
 int dw_pcie_write(void __iomem *addr, int size, u32 val);
-- 
2.37.1.559.g78731f0fdb-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address
  2022-08-12  0:03 ` [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address Will McVicker
@ 2022-08-12  2:46   ` Han Jingoo
  2022-08-23 11:27   ` Robin Murphy
  1 sibling, 0 replies; 9+ messages in thread
From: Han Jingoo @ 2022-08-12  2:46 UTC (permalink / raw)
  To: Will McVicker
  Cc: Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, kernel-team,
	Vidya Sagar, Christoph Hellwig, <linux-pci@vger.kernel.org>,
	Linux Kernel Mailing List, kernel test robot

On 08/11/2022, Will McVicker wrote:
> Since not all devices require a 32-bit MSI address, add support to the
> PCIe host driver to allow setting the DMA mask to 64-bits. This allows
> kernels to disable ZONE_DMA32 and bounce buffering (swiotlb) without
> risking not being able to get a 32-bit address during DMA allocation.
> Basically, in the slim chance that there are no 32-bit allocations
> available, the current PCIe host driver will fail to allocate the
> msi_msg page due to a DMA address overflow (seen in [1]). With this
> patch, the PCIe device can advertise 64-bit support via its MSI
> capabilities to hint to the PCIe host driver to set the DMA mask to
> 64-bits.
>
> [1] https://lore.kernel.org/all/Yo0soniFborDl7+C@google.com/
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Will McVicker <willmcvicker@google.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Jingoo Han <jingoohan1@gmail.com>

> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 14 ++++++++++++--
> drivers/pci/controller/dwc/pcie-designware.c      |  8 ++++++++
> drivers/pci/controller/dwc/pcie-designware.h      |  1 +
> 3 files changed, 21 insertions(+), 2 deletions(-)
…..

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32
  2022-08-12  0:03 ` [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32 Will McVicker
@ 2022-08-12  2:47   ` Han Jingoo
  2022-08-12 18:21   ` Rob Herring
  1 sibling, 0 replies; 9+ messages in thread
From: Han Jingoo @ 2022-08-12  2:47 UTC (permalink / raw)
  To: Will McVicker
  Cc: Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, kernel-team,
	Vidya Sagar, Christoph Hellwig, <linux-pci@vger.kernel.org>,
	Linux Kernel Mailing List, Isaac J . Manjarres

On 08/11/2022, Will McVicker wrote:
> Re-work the msi_msg DMA allocation logic to use dmam_alloc_coherent() which
> uses the coherent DMA mask to try to return an allocation within the DMA
> mask limits. With that, we now can drop the msi_page parameter in struct
> dw_pcie_rp. This allows kernel configurations that disable ZONE_DMA32 to
> continue supporting a 32-bit DMA mask. Without this patch, the PCIe host
> device will fail to probe when ZONE_DMA32 is disabled.
>
> Fixes: 35797e672ff0 ("PCI: dwc: Fix MSI msi_msg DMA mapping")
> Reported-by: Isaac J. Manjarres <isaacmanjarres@google.com>
> Signed-off-by: Will McVicker <willmcvicker@google.com>
Acked-by: Jingoo Han <jingoohan1@gmail.com>

> ---
> .../pci/controller/dwc/pcie-designware-host.c | 28 +++++--------------
> drivers/pci/controller/dwc/pcie-designware.h  |  1 -
> 2 files changed, 7 insertions(+), 22 deletions(-)

…..

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32
  2022-08-12  0:03 ` [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32 Will McVicker
  2022-08-12  2:47   ` Han Jingoo
@ 2022-08-12 18:21   ` Rob Herring
  1 sibling, 0 replies; 9+ messages in thread
From: Rob Herring @ 2022-08-12 18:21 UTC (permalink / raw)
  To: Will McVicker
  Cc: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, kernel-team,
	Vidya Sagar, Christoph Hellwig, linux-pci, linux-kernel,
	Isaac J . Manjarres

On Thu, Aug 11, 2022 at 6:03 PM Will McVicker <willmcvicker@google.com> wrote:
>
> Re-work the msi_msg DMA allocation logic to use dmam_alloc_coherent() which
> uses the coherent DMA mask to try to return an allocation within the DMA
> mask limits. With that, we now can drop the msi_page parameter in struct
> dw_pcie_rp. This allows kernel configurations that disable ZONE_DMA32 to
> continue supporting a 32-bit DMA mask. Without this patch, the PCIe host
> device will fail to probe when ZONE_DMA32 is disabled.
>
> Fixes: 35797e672ff0 ("PCI: dwc: Fix MSI msi_msg DMA mapping")
> Reported-by: Isaac J. Manjarres <isaacmanjarres@google.com>
> Signed-off-by: Will McVicker <willmcvicker@google.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 28 +++++--------------
>  drivers/pci/controller/dwc/pcie-designware.h  |  1 -
>  2 files changed, 7 insertions(+), 22 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses
  2022-08-12  0:03 [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses Will McVicker
  2022-08-12  0:03 ` [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32 Will McVicker
  2022-08-12  0:03 ` [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address Will McVicker
@ 2022-08-23 10:01 ` Lorenzo Pieralisi
  2 siblings, 0 replies; 9+ messages in thread
From: Lorenzo Pieralisi @ 2022-08-23 10:01 UTC (permalink / raw)
  To: Will McVicker, Christoph Hellwig, robin.murphy
  Cc: Jingoo Han, Gustavo Pimentel, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, kernel-team,
	Vidya Sagar, linux-pci, linux-kernel

On Fri, Aug 12, 2022 at 12:03:24AM +0000, Will McVicker wrote:
> Hi All,
> 
> I've updated the series to address the review comments. Refer to the v4
> history below for the changes. Please take a look and thanks again for the
> reviews!

This series looks OK to me - it'd be good if Christoph/Robin can have
a look though before merging it.

Lorenzo

> Regards,
> Will
> 
> Will McVicker (2):
>   PCI: dwc: Drop dependency on ZONE_DMA32
>   PCI: dwc: Add support for 64-bit MSI target address
> 
> v4:
>  * Updated commit descriptions.
>  * Renamed msi_64b -> msi_64bit.
>  * Dropped msi_64bit ternary use.
>  * Dropped export of dw_pcie_msi_capabilities.
> 
> v3:
>   * Switched to a managed DMA allocation.
>   * Simplified the DMA allocation cleanup.
>   * Dropped msi_page from struct dw_pcie_rp.
>   * Allocating a u64 instead of a full page.
> 
> v2:
>   * Fixed build error caught by kernel test robot
>   * Fixed error handling reported by Isaac Manjarres
> 
>  .../pci/controller/dwc/pcie-designware-host.c | 42 +++++++++----------
>  drivers/pci/controller/dwc/pcie-designware.c  |  8 ++++
>  drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
>  3 files changed, 28 insertions(+), 24 deletions(-)
> 
> 
> base-commit: 2ae08b36c06ea8df73a79f6b80ff7964e006e9e3
> -- 
> 2.37.1.559.g78731f0fdb-goog
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address
  2022-08-12  0:03 ` [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address Will McVicker
  2022-08-12  2:46   ` Han Jingoo
@ 2022-08-23 11:27   ` Robin Murphy
  2022-08-25 18:39     ` William McVicker
  1 sibling, 1 reply; 9+ messages in thread
From: Robin Murphy @ 2022-08-23 11:27 UTC (permalink / raw)
  To: Will McVicker, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas
  Cc: kernel-team, Vidya Sagar, Christoph Hellwig, linux-pci,
	linux-kernel, kernel test robot

On 2022-08-12 01:03, Will McVicker wrote:
> Since not all devices require a 32-bit MSI address, add support to the
> PCIe host driver to allow setting the DMA mask to 64-bits. This allows
> kernels to disable ZONE_DMA32 and bounce buffering (swiotlb) without
> risking not being able to get a 32-bit address during DMA allocation.
> Basically, in the slim chance that there are no 32-bit allocations
> available, the current PCIe host driver will fail to allocate the
> msi_msg page due to a DMA address overflow (seen in [1]). With this
> patch, the PCIe device can advertise 64-bit support via its MSI
> capabilities to hint to the PCIe host driver to set the DMA mask to
> 64-bits.

It doesn't matter so much what the host's own capabilities are, though, 
what matters is that if you configure the MSI doorbell to decode some 
64-bit address because you can, and later an endpoint shows up whose DMA 
mask doesn't reach that address, you've broken MSIs for that endpoint. 
While the host is probing, it cannot possibly know what future endpoint 
drivers may or may not do.

Now, in the case where no ZONE_DMA{32} is configured, there's a fair 
likelihood that addressing-constrained endpoints are probably going to 
be broken in general, so that should not be fatal for the host. So *if* 
we fail to allocate a 32-bit MSI address, *then* we should fall back to 
a 64-bit one (when the host supports it), but to preserve compatibility 
we still must always attempt the 32-bit allocation first (oh, if there 
isn't a theme emerging here...)

TBH this has come up enough times, and has enough subtle complexity, 
that I think it's high time to factor this all out into a helper between 
the PCI core code and the DMA API for "please give me a bus address 
that's guaranteed not to conflict with any valid DMA address".

Thanks,
Robin.

> [1] https://lore.kernel.org/all/Yo0soniFborDl7+C@google.com/
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Will McVicker <willmcvicker@google.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>   drivers/pci/controller/dwc/pcie-designware-host.c | 14 ++++++++++++--
>   drivers/pci/controller/dwc/pcie-designware.c      |  8 ++++++++
>   drivers/pci/controller/dwc/pcie-designware.h      |  1 +
>   3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 39f3b37d4033..d7831040791b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -330,6 +330,8 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
>   	u64 *msi_vaddr;
>   	int ret;
>   	u32 ctrl, num_ctrls;
> +	bool msi_64bit = false;
> +	u16 msi_capabilities;
>   
>   	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
>   		pp->irq_mask[ctrl] = ~0;
> @@ -367,9 +369,17 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
>   						    dw_chained_msi_isr, pp);
>   	}
>   
> -	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +	msi_capabilities = dw_pcie_msi_capabilities(pci);
> +	if (msi_capabilities & PCI_MSI_FLAGS_ENABLE)
> +		msi_64bit = msi_capabilities & PCI_MSI_FLAGS_64BIT;
> +
> +	dev_dbg(dev, "Setting MSI DMA mask to %s-bit.\n",
> +		msi_64bit ? "64" : "32");
> +	ret = dma_set_mask_and_coherent(dev, msi_64bit ?
> +					DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
>   	if (ret)
> -		dev_warn(dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
> +		dev_warn(dev, "Failed to set DMA mask to %s-bit.\n",
> +			 msi_64bit ? "64" : "32");
>   
>   	msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
>   					GFP_KERNEL);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index c6725c519a47..650a7f22f9d0 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -82,6 +82,14 @@ u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap)
>   }
>   EXPORT_SYMBOL_GPL(dw_pcie_find_capability);
>   
> +u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
> +{
> +	u8 offset;
> +
> +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +	return dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> +}
> +
>   static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
>   					    u8 cap)
>   {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index a871ae7eb59e..45fcdfc8c035 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -332,6 +332,7 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
>   
>   u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
>   u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);
> +u16 dw_pcie_msi_capabilities(struct dw_pcie *pci);
>   
>   int dw_pcie_read(void __iomem *addr, int size, u32 *val);
>   int dw_pcie_write(void __iomem *addr, int size, u32 val);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address
  2022-08-23 11:27   ` Robin Murphy
@ 2022-08-25 18:39     ` William McVicker
  0 siblings, 0 replies; 9+ messages in thread
From: William McVicker @ 2022-08-25 18:39 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, kernel-team,
	Vidya Sagar, Christoph Hellwig, linux-pci, linux-kernel,
	kernel test robot

On 08/23/2022, Robin Murphy wrote:
> On 2022-08-12 01:03, Will McVicker wrote:
> > Since not all devices require a 32-bit MSI address, add support to the
> > PCIe host driver to allow setting the DMA mask to 64-bits. This allows
> > kernels to disable ZONE_DMA32 and bounce buffering (swiotlb) without
> > risking not being able to get a 32-bit address during DMA allocation.
> > Basically, in the slim chance that there are no 32-bit allocations
> > available, the current PCIe host driver will fail to allocate the
> > msi_msg page due to a DMA address overflow (seen in [1]). With this
> > patch, the PCIe device can advertise 64-bit support via its MSI
> > capabilities to hint to the PCIe host driver to set the DMA mask to
> > 64-bits.
> 
> It doesn't matter so much what the host's own capabilities are, though, what
> matters is that if you configure the MSI doorbell to decode some 64-bit
> address because you can, and later an endpoint shows up whose DMA mask
> doesn't reach that address, you've broken MSIs for that endpoint. While the
> host is probing, it cannot possibly know what future endpoint drivers may or
> may not do.
> 
> Now, in the case where no ZONE_DMA{32} is configured, there's a fair
> likelihood that addressing-constrained endpoints are probably going to be
> broken in general, so that should not be fatal for the host. So *if* we fail
> to allocate a 32-bit MSI address, *then* we should fall back to a 64-bit one
> (when the host supports it), but to preserve compatibility we still must
> always attempt the 32-bit allocation first (oh, if there isn't a theme
> emerging here...)
> 
> TBH this has come up enough times, and has enough subtle complexity, that I
> think it's high time to factor this all out into a helper between the PCI
> core code and the DMA API for "please give me a bus address that's
> guaranteed not to conflict with any valid DMA address".
> 
> Thanks,
> Robin.

Hi Robin,

Thanks for the reviews! I'm happy with your suggestion of trying a 32-bit mask
first for the reasons you mentioned. I have tested this out on my Pixel 6 and
db845c and will push the updated patchset as v5.

Regards,
Will

> 
> > [1] https://lore.kernel.org/all/Yo0soniFborDl7+C@google.com/
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >   drivers/pci/controller/dwc/pcie-designware-host.c | 14 ++++++++++++--
> >   drivers/pci/controller/dwc/pcie-designware.c      |  8 ++++++++
> >   drivers/pci/controller/dwc/pcie-designware.h      |  1 +
> >   3 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 39f3b37d4033..d7831040791b 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -330,6 +330,8 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> >   	u64 *msi_vaddr;
> >   	int ret;
> >   	u32 ctrl, num_ctrls;
> > +	bool msi_64bit = false;
> > +	u16 msi_capabilities;
> >   	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
> >   		pp->irq_mask[ctrl] = ~0;
> > @@ -367,9 +369,17 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> >   						    dw_chained_msi_isr, pp);
> >   	}
> > -	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > +	msi_capabilities = dw_pcie_msi_capabilities(pci);
> > +	if (msi_capabilities & PCI_MSI_FLAGS_ENABLE)
> > +		msi_64bit = msi_capabilities & PCI_MSI_FLAGS_64BIT;
> > +
> > +	dev_dbg(dev, "Setting MSI DMA mask to %s-bit.\n",
> > +		msi_64bit ? "64" : "32");
> > +	ret = dma_set_mask_and_coherent(dev, msi_64bit ?
> > +					DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
> >   	if (ret)
> > -		dev_warn(dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
> > +		dev_warn(dev, "Failed to set DMA mask to %s-bit.\n",
> > +			 msi_64bit ? "64" : "32");
> >   	msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
> >   					GFP_KERNEL);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index c6725c519a47..650a7f22f9d0 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -82,6 +82,14 @@ u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap)
> >   }
> >   EXPORT_SYMBOL_GPL(dw_pcie_find_capability);
> > +u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
> > +{
> > +	u8 offset;
> > +
> > +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > +	return dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> > +}
> > +
> >   static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> >   					    u8 cap)
> >   {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index a871ae7eb59e..45fcdfc8c035 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -332,6 +332,7 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
> >   u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
> >   u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);
> > +u16 dw_pcie_msi_capabilities(struct dw_pcie *pci);
> >   int dw_pcie_read(void __iomem *addr, int size, u32 *val);
> >   int dw_pcie_write(void __iomem *addr, int size, u32 val);

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-08-25 18:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-12  0:03 [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses Will McVicker
2022-08-12  0:03 ` [PATCH v4 1/2] PCI: dwc: Drop dependency on ZONE_DMA32 Will McVicker
2022-08-12  2:47   ` Han Jingoo
2022-08-12 18:21   ` Rob Herring
2022-08-12  0:03 ` [PATCH v4 2/2] PCI: dwc: Add support for 64-bit MSI target address Will McVicker
2022-08-12  2:46   ` Han Jingoo
2022-08-23 11:27   ` Robin Murphy
2022-08-25 18:39     ` William McVicker
2022-08-23 10:01 ` [PATCH v4 0/2] PCI: dwc: Add support for 64-bit MSI target addresses Lorenzo Pieralisi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).