On Wed, Feb 21, 2024 at 03:52:43PM +0300, Serge Semin wrote: > 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/ > ACK. Will initialize to NULL in the next version. > > > > > - 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. > ACK. Will shorten the message in the next version. > > + 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/ > ACK. Will remove the check in the next version. > -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 > >
