On Wed, Feb 21, 2024 at 12:18:46PM +0530, Ajay Agarwal wrote: > There can be platforms that do not use/have 32-bit DMA addresses. > The current implementation of 32-bit IOVA allocation can fail for > such platforms, eventually leading to the probe failure. > > Try to allocate a 32-bit msi_data. If this allocation fails, > attempt a 64-bit address allocation. Please note that if the > 64-bit MSI address is allocated, then the EPs supporting 32-bit > MSI address only will not work. > > Signed-off-by: Ajay Agarwal <ajayagarwal@xxxxxxxxxx> > --- > Changelog since v4: > - Remove the 'DW_PCIE_CAP_MSI_DATA_SET' flag > - Refactor the comments and msi_data allocation logic > > Changelog since v3: > - Add a new controller cap flag 'DW_PCIE_CAP_MSI_DATA_SET' > - Refactor the comments and print statements > > Changelog since v2: > - If the vendor driver has setup the msi_data, use the same > > Changelog since v1: > - Use reserved memory, if it exists, to setup the MSI data > - Fallback to 64-bit IOVA allocation if 32-bit allocation fails > > --- > .../pci/controller/dwc/pcie-designware-host.c | 21 ++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index d5fc31f8345f..9c905e5c4904 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -379,15 +379,22 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp) > * memory. > */ > ret = dma_set_coherent_mask(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"); > + if (!ret) > + msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data, > + GFP_KERNEL); msi_vaddr will be left uninitialized if the mask setting fails. Robin had it initialized in v2 discussion: https://lore.kernel.org/linux-pci/7cd42851-37cc-49d6-b9ad-74a256a73904@xxxxxxx/ > > - 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 -ENOMEM; > + dev_warn(dev, "Failed to configure 32-bit MSI address. Devices with only 32-bit MSI support may not work properly\n"); Can we short it up already? I suggested something like "Failed to allocate 32-bit MSI target address" in v2. It means the problem with 32-bit MSIs which can be perceived as the possible reason of the respective peripheral devices not working correctly. > + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(64)); > + if (!ret) Redundant check. It was discussed some time ago. See the comment from Robin: https://lore.kernel.org/linux-pci/335d730d-529e-7ce0-8bac-008a4daa6e3c@xxxxxxx/ -Serge(y) > + msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data, > + GFP_KERNEL); > + > + if (!msi_vaddr) { > + dev_err(dev, "Failed to configure MSI address\n"); > + dw_pcie_free_msi(pp); > + return -ENOMEM; > + } > } > > return 0; > -- > 2.44.0.rc0.258.g7320e95886-goog >
