PCI: dwc: Add support for 64-bit MSI target addresses
cover PCI: dwc: Add support for 64-bit MSI target addresses
Hi,
I have two patches that address a couple of issues I've run into with the PCIe DWC host driver:
(1) the host driver fails to probe when ZONE_DMA32 is disabled.
(2) the host driver will fail to probe if a 32-bit address is not available even on devices that support 64-bit target addresses.
I have addressed both of these issues with the two patches (details can found in each of the patch commit messages). Please take a look and let me know your thoughts.
With regards to testing, I have verified them on Android with a Pixel 6 device on the 5.19 + pci-v5.20-changes kernel. My testing included running with both ZONE_DMA32 and swiotlb disabled.
Thanks,
Will
Will McVicker (2):
PCI: dwc: drop dependency on ZONE_DMA32
PCI: dwc: add support for 64-bit MSI target address
.../pci/controller/dwc/pcie-designware-host.c | 42 ++++++++++++------- drivers/pci/controller/dwc/pcie-designware.c | 9 ++++ drivers/pci/controller/dwc/pcie-designware.h | 6 +++ 3 files changed, 41 insertions(+), 16 deletions(-)
Signature · 1 line
2.37.1.559.g78731f0fdb-goog
PATCH 1/2 PCI: dwc: drop dependency on ZONE_DMA32
Re-work the msi_msg DMA allocation logic to use dma_alloc_coherent() which uses the coherent DMA mask to try and return an allocation within the DMA mask limits. 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.
--- .../pci/controller/dwc/pcie-designware-host.c | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-)
drivers/pci/controller/dwc/pcie-designware-host.c +14 -14
@@ -272,9 +272,9 @@ static void dw_pcie_free_msi(struct dw_pcie_rp *pp)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); +dma_free_coherent (dev,PAGE_SIZE , pp->msi_page, pp->msi_data); + pp->msi_data =0 ; + pp->msi_page =NULL ; } }
@@ -375,22 +375,22 @@ 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_page =dma_alloc_coherent (dev,PAGE_SIZE , &pp->msi_data, +GFP_KERNEL ); +if (!pp->msi_page ||dma_mapping_error (dev, pp->msi_data)) { +dev_err (dev,"Failed to alloc and map MSI data\n" ); +if (pp->msi_page) { +dma_free_coherent (dev,PAGE_SIZE , pp->msi_page, + pp->msi_data); + pp->msi_page =NULL ; + } pp->msi_data =0 ;dw_pcie_free_msi (pp); - -return ret; +return -ENOMEM ; }return 0 ;
Signature · 1 line
2.37.1.559.g78731f0fdb-goog
Re: [PATCH v1 1/2] PCI: dwc: drop dependency on ZONE_DMA32
drivers/pci/controller/dwc/pcie-designware-host.c static void dw_pcie_free_msi(struct dw_pcie_rp *pp) · line 275
Quoted 7 lines · Will McVicker
Re-work the
msi_msgDMA allocation logic to usedma_alloc_coherent()which uses the coherent DMA mask to try and return an allocation within the DMA mask limits. This allows kernel configurations that disableZONE_DMA32to continue supporting a 32-bit DMA mask. Without this patch, the PCIe host device will fail to probe whenZONE_DMA32is disabled.drivers/pci/controller/dwc/pcie-designware-host.c +1
-0+dma_free_coherent (dev,PAGE_SIZE , pp->msi_page, pp->msi_data);
Isn't msi_page a struct page * which should become a void * now?
Otherwise this looks good and is what the driver should have done from the very beginning.
Re: [PATCH v1 1/2] PCI: dwc: drop dependency on ZONE_DMA32
Quoted 7 lines · Christoph Hellwig
Quoted 5 lines · Will McVicker
Re-work the
msi_msgDMA allocation logic to usedma_alloc_coherent()which uses the coherent DMA mask to try and return an allocation within the DMA mask limits. This allows kernel configurations that disableZONE_DMA32to continue supporting a 32-bit DMA mask. Without this patch, the PCIe host device will fail to probe whenZONE_DMA32is disabled.
Quoted 6 lines
drivers/pci/controller/dwc/pcie-designware-host.c static void dw_pcie_free_msi(struct dw_pcie_rp *pp) · line 275
Quoted 1 line
drivers/pci/controller/dwc/pcie-designware-host.c +1
-0+dma_free_coherent (dev,PAGE_SIZE , pp->msi_page, pp->msi_data);Isn't
msi_pagea struct page * which should become a void * now?Otherwise this looks good and is what the driver should have done from the very beginning.
I've updated this in v3 to use dmam_alloc_coherent() and dropped msi_page since it's not actually used. So this block is dropped now. Thanks for taking a look though!
--Will
PATCH 2/2 PCI: dwc: add support for 64-bit MSI target address
In reply to Will McVickerSince 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 driver can advertise 64-bit support via it's MSI capabilities to hint to the PCIe host driver to set the DMA mask to 64-bits.
[1] https://lore.kernel.org/all/[email protected]/
--- drivers/pci/controller/dwc/pcie-designware-host.c | 14 ++++++++++++-- drivers/pci/controller/dwc/pcie-designware.c | 9 +++++++++ drivers/pci/controller/dwc/pcie-designware.h | 6 ++++++ 3 files changed, 27 insertions(+), 2 deletions(-)
drivers/pci/controller/dwc/pcie-designware-host.c +12 -2
@@ -338,6 +338,8 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)struct platform_device *pdev =to_platform_device (dev);int ret;u32 ctrl, num_ctrls; +bool msi_64b = false; +u16 msi_capabilities;for (ctrl =0 ; ctrl <MAX_MSI_CTRLS ; ctrl++) pp->irq_mask[ctrl] = ~0 ;
@@ -375,9 +377,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_64b = msi_capabilities &PCI_MSI_FLAGS_64BIT ? true : false; + +dev_dbg (dev,"Setting MSI DMA mask to %s-bit.\n" , + msi_64b ?"64" :"32" ); + ret =dma_set_mask_and_coherent (dev, msi_64b ? +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_64b ?"64" :"32" ); pp->msi_page =dma_alloc_coherent (dev,PAGE_SIZE , &pp->msi_data,GFP_KERNEL );
drivers/pci/controller/dwc/pcie-designware.c +9 -0
@@ -82,6 +82,15 @@ 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 ); +} +EXPORT_SYMBOL_GPL (dw_pcie_msi_capabilities); +static u16 dw_pcie_find_next_ext_capability (struct dw_pcie *pci,u16 start,u8 cap) {
drivers/pci/controller/dwc/pcie-designware.h +6 -0
@@ -432,6 +432,7 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp);int dw_pcie_allocate_domains (struct dw_pcie_rp *pp);void __iomem *dw_pcie_own_conf_map_bus (struct pci_bus *bus,unsigned int devfn,int where); +u16 dw_pcie_msi_capabilities (struct dw_pcie *pci);#else static inline irqreturn_t dw_handle_msi_irq (struct dw_pcie_rp *pp) {
@@ -462,6 +463,11 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, {return NULL ; } + +static inline u16 dw_pcie_msi_capabilities (struct dw_pcie *pci) +{ +return 0 ; +}#endif #ifdef CONFIG_PCIE_DW_EP
Signature · 1 line
2.37.1.559.g78731f0fdb-goog
kernel test robot
build botRe: [PATCH v1 2/2] PCI: dwc: add support for 64-bit MSI target address
Thank you for the patch! Yet something to improve:
Hi Will,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on linus/master next-20220810] [cannot apply to v5.19]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Will-McVicker/PCI-dwc-Add-support-for-64-bit-MSI-target-addresses/20220810-020421 base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next config: sparc-buildonly-randconfig-r004-20220810 (https://download.01.org/0day-ci/archive/20220811/[email protected]/config) compiler: sparc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/7a41faa4e02a0a8945f79e7af86d10e371b2fc12
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Will-McVicker/PCI-dwc-Add-support-for-64-bit-MSI-target-addresses/20220810-020421 git checkout 7a41faa4e02a0a8945f79e7af86d10e371b2fc12
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
All errors (new ones prefixed by >>):
>> drivers/pci/controller/dwc/pcie-designware.c:85:5: error: redefinition of 'dw_pcie_msi_capabilities'
85 | u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
| ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/pci/controller/dwc/pcie-designware.c:20:
drivers/pci/controller/dwc/pcie-designware.h:467:19: note: previous definition of 'dw_pcie_msi_capabilities' with type 'u16(struct dw_pcie *)' {aka 'short unsigned int(struct dw_pcie *)'}
467 | static inline u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
| ^~~~~~~~~~~~~~~~~~~~~~~~
vim +/dw_pcie_msi_capabilities +85 drivers/pci/controller/dwc/pcie-designware.c
84
> 85 u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
86 {
87 u8 offset;
88
89 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
90 return dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
91 }
92 EXPORT_SYMBOL_GPL(dw_pcie_msi_capabilities);
93 Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
Re: [PATCH v1 2/2] PCI: dwc: add support for 64-bit MSI target address
In reply to Will McVickerdrivers/pci/controller/dwc/pcie-designware-host.c static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp) · line 341
Quoted 1 line · Will McVicker
drivers/pci/controller/dwc/pcie-designware-host.c +1
-0+bool msi_64b = false;
Spellt out bit here?
drivers/pci/controller/dwc/pcie-designware-host.c static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp) · line 382
Quoted 3 lines
drivers/pci/controller/dwc/pcie-designware-host.c +3
-0+ msi_capabilities =dw_pcie_msi_capabilities (pci); +if (msi_capabilities &PCI_MSI_FLAGS_ENABLE ) + msi_64b = msi_capabilities &PCI_MSI_FLAGS_64BIT ? true : false;
No need for the tenary operator here:
msi_64bit = msi_capabilities & PCI_MSI_FLAGS_64BIT;
Re: [PATCH v1 2/2] PCI: dwc: add support for 64-bit MSI target address
Quoted 5 lines · Christoph Hellwig
drivers/pci/controller/dwc/pcie-designware-host.c static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp) · line 341
Quoted 1 line · Will McVicker
drivers/pci/controller/dwc/pcie-designware-host.c +1
-0+bool msi_64b = false;Spellt out bit here?
Quoted 7 lines
drivers/pci/controller/dwc/pcie-designware-host.c static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp) · line 382
Quoted 3 lines
drivers/pci/controller/dwc/pcie-designware-host.c +3
-0+ msi_capabilities =dw_pcie_msi_capabilities (pci); +if (msi_capabilities &PCI_MSI_FLAGS_ENABLE ) + msi_64b = msi_capabilities &PCI_MSI_FLAGS_64BIT ? true : false;No need for the tenary operator here:
msi_64bit = msi_capabilities & PCI_MSI_FLAGS_64BIT;
Thanks for the suggestions. I'll update in the next patchset.
kernel test robot
build botRe: [PATCH v1 2/2] PCI: dwc: add support for 64-bit MSI target address
In reply to Will McVickerThank you for the patch! Yet something to improve:
Hi Will,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on linus/master next-20220811] [cannot apply to v5.19]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Will-McVicker/PCI-dwc-Add-support-for-64-bit-MSI-target-addresses/20220810-020421 base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next config: powerpc-randconfig-c003-20220810 (https://download.01.org/0day-ci/archive/20220811/[email protected]/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/7a41faa4e02a0a8945f79e7af86d10e371b2fc12
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Will-McVicker/PCI-dwc-Add-support-for-64-bit-MSI-target-addresses/20220810-020421 git checkout 7a41faa4e02a0a8945f79e7af86d10e371b2fc12
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/pci/
If you fix the issue, kindly add following tag where applicable
All errors (new ones prefixed by >>):
>> drivers/pci/controller/dwc/pcie-designware.c:85:5: error: redefinition of 'dw_pcie_msi_capabilities'
u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
^
drivers/pci/controller/dwc/pcie-designware.h:467:19: note: previous definition is here
static inline u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
^1 error generated.
vim +/dw_pcie_msi_capabilities +85 drivers/pci/controller/dwc/pcie-designware.c
84
> 85 u16 dw_pcie_msi_capabilities(struct dw_pcie *pci)
86 {
87 u8 offset;
88
89 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
90 return dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
91 }
92 EXPORT_SYMBOL_GPL(dw_pcie_msi_capabilities);
93 Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp