Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752153AbcDUIla (ORCPT ); Thu, 21 Apr 2016 04:41:30 -0400 Received: from mail-wm0-f44.google.com ([74.125.82.44]:35421 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751604AbcDUIl2 (ORCPT ); Thu, 21 Apr 2016 04:41:28 -0400 Subject: Re: [PATCH v7 09/10] iommu/dma-reserved-iommu: iommu_msi_mapping_translate_msg To: Robin Murphy , eric.auger@st.com, alex.williamson@redhat.com, will.deacon@arm.com, joro@8bytes.org, tglx@linutronix.de, jason@lakedaemon.net, marc.zyngier@arm.com, christoffer.dall@linaro.org, linux-arm-kernel@lists.infradead.org References: <1461084994-2355-1-git-send-email-eric.auger@linaro.org> <1461084994-2355-10-git-send-email-eric.auger@linaro.org> <5717BC43.5070005@arm.com> Cc: patches@linaro.org, linux-kernel@vger.kernel.org, Bharat.Bhushan@freescale.com, pranav.sawargaonkar@gmail.com, p.fedin@samsung.com, iommu@lists.linux-foundation.org, Jean-Philippe.Brucker@arm.com, julien.grall@arm.com From: Eric Auger Message-ID: <571891EA.9030002@linaro.org> Date: Thu, 21 Apr 2016 10:40:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <5717BC43.5070005@arm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5873 Lines: 182 On 04/20/2016 07:28 PM, Robin Murphy wrote: > On 19/04/16 17:56, Eric Auger wrote: >> Introduce iommu_msi_mapping_translate_msg whose role consists in >> detecting whether the device's MSIs must to be mapped into an IOMMU. >> It case it must, the function overrides the MSI msg originally composed >> and replaces the doorbell's PA by a pre-allocated and pre-mapped reserved >> IOVA. In case the corresponding PA region is not found, the function >> returns an error. >> >> This function is likely to be called in some code that cannot sleep. This >> is the reason why the allocation/mapping is done separately. >> >> Signed-off-by: Eric Auger >> >> --- >> >> v7: creation >> --- >> drivers/iommu/dma-reserved-iommu.c | 69 >> ++++++++++++++++++++++++++++++++++++++ >> include/linux/dma-reserved-iommu.h | 27 +++++++++++++++ >> 2 files changed, 96 insertions(+) >> >> diff --git a/drivers/iommu/dma-reserved-iommu.c >> b/drivers/iommu/dma-reserved-iommu.c >> index 907a17f..603ee45 100644 >> --- a/drivers/iommu/dma-reserved-iommu.c >> +++ b/drivers/iommu/dma-reserved-iommu.c >> @@ -18,6 +18,14 @@ >> #include >> #include >> #include >> +#include >> + >> +#ifdef CONFIG_PHYS_ADDR_T_64BIT >> +#define msg_to_phys_addr(msg) \ >> + (((phys_addr_t)((msg)->address_hi) << 32) | (msg)->address_lo) >> +#else >> +#define msg_to_phys_addr(msg) ((msg)->address_lo) >> +#endif >> >> struct reserved_iova_domain { >> struct iova_domain *iovad; >> @@ -351,3 +359,64 @@ struct iommu_domain >> *iommu_msi_mapping_desc_to_domain(struct msi_desc *desc) >> return d; >> } >> EXPORT_SYMBOL_GPL(iommu_msi_mapping_desc_to_domain); >> + >> +static dma_addr_t iommu_find_reserved_iova(struct iommu_domain *domain, >> + phys_addr_t addr, size_t size) >> +{ >> + unsigned long base_pfn, end_pfn, nb_iommu_pages, order, flags; >> + size_t iommu_page_size, binding_size; >> + struct iommu_reserved_binding *b; >> + phys_addr_t aligned_base, offset; >> + dma_addr_t iova = DMA_ERROR_CODE; >> + struct iova_domain *iovad; >> + >> + spin_lock_irqsave(&domain->reserved_lock, flags); >> + >> + iovad = (struct iova_domain *)domain->reserved_iova_cookie; >> + >> + if (!iovad) >> + goto unlock; >> + >> + order = iova_shift(iovad); >> + base_pfn = addr >> order; >> + end_pfn = (addr + size - 1) >> order; >> + aligned_base = base_pfn << order; >> + offset = addr - aligned_base; >> + nb_iommu_pages = end_pfn - base_pfn + 1; >> + iommu_page_size = 1 << order; >> + binding_size = nb_iommu_pages * iommu_page_size; > > This all looks rather familiar... > >> + b = find_reserved_binding(domain, aligned_base, binding_size); > > ...which implies that at least some of it should be factored into that guy. ok. Besides, with your compact rewriting proposal, maybe it is not worth anymore ;-) Eric > >> + if (b && (b->addr <= aligned_base) && >> + (aligned_base t + binding_size <= b->addr + b->size)) >> + iova = b->iova + offset + aligned_base - b->addr; >> +unlock: >> + spin_unlock_irqrestore(&domain->reserved_lock, flags); >> + return iova; >> +} >> + >> +int iommu_msi_mapping_translate_msg(struct irq_data *data, struct >> msi_msg *msg) >> +{ >> + struct iommu_domain *d; >> + struct msi_desc *desc; >> + dma_addr_t iova; >> + >> + desc = irq_data_get_msi_desc(data); >> + if (!desc) >> + return -EINVAL; >> + >> + d = iommu_msi_mapping_desc_to_domain(desc); >> + if (!d) >> + return 0; >> + >> + iova = iommu_find_reserved_iova(d, msg_to_phys_addr(msg), >> + sizeof(phys_addr_t)); >> + >> + if (iova == DMA_ERROR_CODE) >> + return -EINVAL; >> + >> + msg->address_lo = lower_32_bits(iova); >> + msg->address_hi = upper_32_bits(iova); >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(iommu_msi_mapping_translate_msg); >> diff --git a/include/linux/dma-reserved-iommu.h >> b/include/linux/dma-reserved-iommu.h >> index 8373929..04e1912f 100644 >> --- a/include/linux/dma-reserved-iommu.h >> +++ b/include/linux/dma-reserved-iommu.h >> @@ -20,6 +20,8 @@ >> >> struct iommu_domain; >> struct msi_desc; >> +struct irq_data; >> +struct msi_msg; >> >> #ifdef CONFIG_IOMMU_DMA_RESERVED >> >> @@ -82,6 +84,25 @@ void iommu_put_reserved_iova(struct iommu_domain >> *domain, phys_addr_t addr); >> */ >> struct iommu_domain *iommu_msi_mapping_desc_to_domain(struct >> msi_desc *desc); >> >> +/** >> + * iommu_msi_mapping_translate_msg: in case the MSI transaction is >> translated >> + * by an IOMMU, the msg address must be an IOVA instead of a physical >> address. >> + * This function overwrites the original MSI message containing the >> doorbell >> + * physical address, result of the primary composition, with the >> doorbell IOVA. >> + * >> + * The doorbell physical address must be bound previously to an IOVA >> using >> + * iommu_get_reserved_iova >> + * >> + * @data: irq data handle >> + * @msg: original msi message containing the PA to be overwritten with >> + * the IOVA >> + * >> + * return 0 if the MSI does not need to be mapped or when the PA/IOVA >> + * were successfully swapped; return -EINVAL if the addresses need >> + * to be swapped but not IOMMU binding is found >> + */ >> +int iommu_msi_mapping_translate_msg(struct irq_data *data, struct >> msi_msg *msg); >> + >> #else >> >> static inline int >> @@ -111,5 +132,11 @@ iommu_msi_mapping_desc_to_domain(struct msi_desc >> *desc) >> return NULL; >> } >> >> +static inline int iommu_msi_mapping_translate_msg(struct irq_data *data, >> + struct msi_msg *msg) >> +{ >> + return 0; >> +} >> + >> #endif /* CONFIG_IOMMU_DMA_RESERVED */ >> #endif /* __DMA_RESERVED_IOMMU_H */ >> >