On Tue, 23 Aug 2016, Robin Murphy wrote: > + cookie = domain->iova_cookie; > + iovad = &cookie->iovad; > + > + spin_lock(&cookie->msi_lock); > + list_for_each_entry(msi_page, &cookie->msi_page_list, list) > + if (msi_page->phys_hi == msg->address_hi && > + msi_page->phys_lo - msg->address_lo < iovad->granule) > + goto unlock; > + > + ret = __iommu_dma_map_msi_page(dev, msg, domain, &msi_page); > +unlock: > + spin_unlock(&cookie->msi_lock); > + > + if (!ret) { > + msg->address_hi = upper_32_bits(msi_page->iova); > + msg->address_lo &= iova_mask(iovad); > + msg->address_lo += lower_32_bits(msi_page->iova); > + } else { > + /* > + * We're called from a void callback, so the best we can do is > + * 'fail' by filling the message with obviously bogus values. > + * Since we got this far due to an IOMMU being present, it's > + * not like the existing address would have worked anyway... > + */ > + msg->address_hi = ~0U; > + msg->address_lo = ~0U; > + msg->data = ~0U; > + } The above is really horrible to parse. I had to read it five times to understand the logic. static struct iommu_dma_msi_page * find_or_map_msi_page(struct iommu_dma_cookie *cookie, struct msi_msg *msg) { struct iova_domain *iovad = &cookie->iovad; struct iommu_dma_msi_page *page; list_for_each_entry(*page, &cookie->msi_page_list, list) { if (page->phys_hi == msg->address_hi && page->phys_lo - msg->address_lo < iovad->granule) return page; } /* * FIXME: __iommu_dma_map_msi_page() should return a page or NULL. * The integer return value is pretty pointless. If seperate error * codes are required that's what ERR_PTR() is for .... */ ret = __iommu_dma_map_msi_page(dev, msg, domain, &page); return ret ? ERR_PTR(ret) : page; } So now the code in iommu_dma_map_msi_msg() becomes: spin_lock(&cookie->msi_lock); msi_page = find_or_map_msi_page(cookie, msg); spin_unlock(&cookie->msi_lock); if (!IS_ERR_OR_NULL(msi_page)) { msg->address_hi = upper_32_bits(msi_page->iova); msg->address_lo &= iova_mask(iovad); msg->address_lo += lower_32_bits(msi_page->iova); } else { /* * We're called from a void callback, so the best we can do is * 'fail' by filling the message with obviously bogus values. * Since we got this far due to an IOMMU being present, it's * not like the existing address would have worked anyway... */ msg->address_hi = ~0U; msg->address_lo = ~0U; msg->data = ~0U; } Hmm? Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html