Messages in this thread | | | From | "Sricharan" <> | Subject | RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED | Date | Mon, 2 Jan 2017 18:52:45 +0530 |
| |
Hi Robin,
>> >> <snip..> >> >>>>>>> return prot | IOMMU_READ | IOMMU_WRITE; >>>>>> >>>>>> ...and applying against -next now also needs this hunk: >>>>>> >>>>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device >>>>>> *dev, phys_addr_t phys, >>>>>> size_t size, enum dma_data_direction dir, unsigned long attrs) >>>>>> { >>>>>> return __iommu_dma_map(dev, phys, size, >>>>>> - dma_direction_to_prot(dir, false) | IOMMU_MMIO); >>>>>> + dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO); >>>>>> } >>>>>> >>>>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle, >>>>>> >>>>>> With those two issues fixed up, I've given the series (applied to >>>>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out. >>>>>> >>>>> >>>>> oops, sorry that i missed this in rebase. I can repost now with this fixed, >>>>> 'checks out' you mean something is not working correct ? >>>> >>>> No, I mean it _is_ still correct - I guess that's more of an idiom than >>>> I thought :) >>>> >>> >>> ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now. >> >> Just while checking that i have not missed anything else, realized that the >> dma-mapping apis in arm as to be modified to pass the PRIVILIGED attributes >> as well. While my testing path was using the iommu_map directly i was not >> seeing this, but then i did a patch like below. I will just figure out another >> other codebase where the master uses the dma apis, test and add it in the >> V8 that i would send. > >True, adding support to 32-bit as well can't hurt, and I guess it's >equally relevant to QC's GPU use-case. I haven't considered it myself >because AArch32 is immune to the specific PL330 problem which caught me >out - that subtle corner of VMSAv8 is unique to AArch64. > >> From: Sricharan R <sricharan@codeaurora.org> >> Date: Tue, 13 Dec 2016 23:25:01 +0530 >> Subject: [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED >> >> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that >> are only accessible to privileged DMA engines. Implementing it in dma-mapping >> for it to get used from the dma mappings apis. >> >> Signed-off-by: Sricharan R <sricharan@codeaurora.org> >> --- >> arch/arm/mm/dma-mapping.c | 24 +++++++++++++++--------- >> 1 file changed, 15 insertions(+), 9 deletions(-) >> >> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c >> index ab77100..e0d9923 100644 >> --- a/arch/arm/mm/dma-mapping.c >> +++ b/arch/arm/mm/dma-mapping.c >> @@ -1394,7 +1394,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages, >> * Create a mapping in device IO address space for specified pages >> */ >> static dma_addr_t >> -__iommu_create_mapping(struct device *dev, struct page **pages, size_t size) >> +__iommu_create_mapping(struct device *dev, struct page **pages, size_t size, >> + unsigned long attrs) >> { >> struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev); >> unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT; >> @@ -1419,7 +1420,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages, >> >> len = (j - i) << PAGE_SHIFT; >> ret = iommu_map(mapping->domain, iova, phys, len, >> - IOMMU_READ|IOMMU_WRITE); >> + __dma_info_to_prot(DMA_BIRECTIONAL, attrs)); >> if (ret < 0) >> goto fail; >> iova += len; >> @@ -1476,7 +1477,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs) >> } >> >> static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp, >> - dma_addr_t *handle, int coherent_flag) >> + dma_addr_t *handle, int coherent_flag, >> + unsigned long attrs) >> { >> struct page *page; >> void *addr; >> @@ -1488,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp, >> if (!addr) >> return NULL; >> >> - *handle = __iommu_create_mapping(dev, &page, size); >> + *handle = __iommu_create_mapping(dev, &page, size, attrs); >> if (*handle == DMA_ERROR_CODE) >> goto err_mapping; >> >> @@ -1522,7 +1524,8 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size, >> >> if (coherent_flag == COHERENT || !gfpflags_allow_blocking(gfp)) >> return __iommu_alloc_simple(dev, size, gfp, handle, >> - coherent_flag); >> + coherent_flag, >> + attrs); > >Super-nit: unnecessary line break. > >> >> /* >> * Following is a work-around (a.k.a. hack) to prevent pages >> @@ -1672,10 +1675,13 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt, >> GFP_KERNEL); >> } >> >> -static int __dma_direction_to_prot(enum dma_data_direction dir) >> +static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs) >> { >> int prot; >> >> + if (attrs & DMA_ATTR_PRIVILEGED) >> + prot |= IOMMU_PRIV; >> + >> switch (dir) { >> case DMA_BIDIRECTIONAL: >> prot = IOMMU_READ | IOMMU_WRITE; >> @@ -1722,7 +1728,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg, >> if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0) >> __dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir); >> >> - prot = __dma_direction_to_prot(dir); >> + prot = __dma_info_to_prot(dir, attrs); >> >> ret = iommu_map(mapping->domain, iova, phys, len, prot); >> if (ret < 0) >> @@ -1930,7 +1936,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p >> if (dma_addr == DMA_ERROR_CODE) >> return dma_addr; >> >> - prot = __dma_direction_to_prot(dir); >> + prot = __dma_info_to_prot(dir, attrs); >> >> ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot); >> if (ret < 0) >> @@ -2036,7 +2042,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev, >> if (dma_addr == DMA_ERROR_CODE) >> return dma_addr; >> >> - prot = __dma_direction_to_prot(dir) | IOMMU_MMIO; >> + prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO; >> >> ret = iommu_map(mapping->domain, dma_addr, addr, len, prot); >> if (ret < 0) >> > >Looks reasonable to me. Assuming it survives testing: > >Acked-by: Robin Murphy <robin.murphy@arm.com>
Posted V8 [1]. I changed a few more things after the testing, though functionally the same. So have not taken your ack and would be nice to have it once again
[1] https://lkml.org/lkml/2017/1/2/224
Regards, Sricharan
| |