On Mon, Oct 24, 2016 at 11:09 AM, Konrad Rzeszutek Wilk
wrote:
> On Mon, Oct 24, 2016 at 08:04:37AM -0400, Alexander Duyck wrote:
>> As a first step to making DMA_ATTR_SKIP_CPU_SYNC apply to architectures
>> beyond just ARM I need to make it so that the swiotlb will respect the
>> flag. In order to do that I also need to update the swiotlb-xen since it
>> heavily makes use of the functionality.
>>
>> Cc: Konrad Rzeszutek Wilk
>> Signed-off-by: Alexander Duyck
>> ---
>> drivers/xen/swiotlb-xen.c | 40 ++
>> include/linux/swiotlb.h |6 --
>> lib/swiotlb.c | 48
>> +++--
>> 3 files changed, 56 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
>> index 87e6035..cf047d8 100644
>> --- a/drivers/xen/swiotlb-xen.c
>> +++ b/drivers/xen/swiotlb-xen.c
>> @@ -405,7 +405,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev,
>> struct page *page,
>>*/
>> trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
>>
>> - map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir);
>> + map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir,
>> + attrs);
>> if (map == SWIOTLB_MAP_ERROR)
>> return DMA_ERROR_CODE;
>>
>> @@ -416,11 +417,13 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev,
>> struct page *page,
>> /*
>>* Ensure that the address returned is DMA'ble
>>*/
>> - if (!dma_capable(dev, dev_addr, size)) {
>> - swiotlb_tbl_unmap_single(dev, map, size, dir);
>> - dev_addr = 0;
>> - }
>> - return dev_addr;
>> + if (dma_capable(dev, dev_addr, size))
>> + return dev_addr;
>> +
>> + swiotlb_tbl_unmap_single(dev, map, size, dir,
>> + attrs | DMA_ATTR_SKIP_CPU_SYNC);
>> +
>> + return DMA_ERROR_CODE;
>
> Why? This change (re-ordering the code - and returning DMA_ERROR_CODE instead
> of 0) does not have anything to do with the title.
>
> If you really feel strongly about it - then please send it as a seperate
> patch.
Okay I can do that. This was mostly just to clean up the formatting
because I was over 80 characters when I added the attribute. Changing
the return value to DMA_ERROR_CODE from 0 was based on the fact that
earlier in the function that is the value you return if there is a
mapping error.
>> }
>> EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
>>
>> @@ -444,7 +447,7 @@ static void xen_unmap_single(struct device *hwdev,
>> dma_addr_t dev_addr,
>>
>> /* NOTE: We use dev_addr here, not paddr! */
>> if (is_xen_swiotlb_buffer(dev_addr)) {
>> - swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
>> + swiotlb_tbl_unmap_single(hwdev, paddr, size, dir, attrs);
>> return;
>> }
>>
>> @@ -557,16 +560,9 @@ void xen_swiotlb_unmap_page(struct device *hwdev,
>> dma_addr_t dev_addr,
>>
>> start_dma_addr,
>>sg_phys(sg),
>>sg->length,
>> - dir);
>> - if (map == SWIOTLB_MAP_ERROR) {
>> - dev_warn(hwdev, "swiotlb buffer is full\n");
>> - /* Don't panic here, we expect map_sg users
>> -to do proper error handling. */
>> - xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
>> -attrs);
>> - sg_dma_len(sgl) = 0;
>> - return 0;
>> - }
>> + dir, attrs);
>> + if (map == SWIOTLB_MAP_ERROR)
>> + goto map_error;
>> xen_dma_map_page(hwdev, pfn_to_page(map >> PAGE_SHIFT),
>> dev_addr,
>> map & ~PAGE_MASK,
>> @@ -589,6 +585,16 @@ void xen_swiotlb_unmap_page(struct device *hwdev,
>> dma_addr_t dev_addr,
>> sg_dma_len(sg) = sg->length;
>> }
>> return nelems;
>> +map_error:
>> + dev_warn(hwdev, "swiotlb buffer is full\n");
>> + /*
>> + * Don't panic here, we expect map_sg users
>> + * to do proper error handling.
>> + */
>> + xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
>> +attrs | DMA_ATTR_SKIP_CPU_SYNC);
>> + sg_dma_len(sgl) = 0;
>> + return 0;
>> }
>
> This too. Why can't that be part of the existing code that was there?
Once again it was a formatting thing. I was indented too far and
adding the attr
