[PATCH 3/5] swiotlb: swiotlb_bus_to_virt/phys prototype changes
Becky Bruce
platforms it contains a per-device offset that is used to convert
from bus addresses to/from other types of addresses.
Also, make these weak so architectures can override the default
behavior (for example, by adding an offset in the hwdev).
Signed-off-by: Becky Bruce <bec...@kernel.crashing.org>
---
arch/x86/kernel/pci-swiotlb.c | 2 +-
include/linux/swiotlb.h | 4 +++-
lib/swiotlb.c | 31 +++++++++++++++++++------------
3 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index 34f12e9..887388a 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -28,7 +28,7 @@ dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
return paddr;
}
-phys_addr_t swiotlb_bus_to_phys(dma_addr_t baddr)
+phys_addr_t swiotlb_bus_to_phys(struct device *hwdev, dma_addr_t baddr)
{
return baddr;
}
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index ac9ff54..a27bca3 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -29,7 +29,9 @@ extern void *swiotlb_alloc(unsigned order, unsigned long nslabs);
extern dma_addr_t swiotlb_phys_to_bus(struct device *hwdev,
phys_addr_t address);
-extern phys_addr_t swiotlb_bus_to_phys(dma_addr_t address);
+extern phys_addr_t swiotlb_bus_to_phys(struct device *hwdev,
+ dma_addr_t address);
+extern void *swiotlb_bus_to_virt(struct device *hwdev, dma_addr_t address);
extern int swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size);
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 62f5f75..c152f17 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -129,20 +129,20 @@ dma_addr_t __weak swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
return paddr;
}
-phys_addr_t __weak swiotlb_bus_to_phys(dma_addr_t baddr)
+phys_addr_t __weak swiotlb_bus_to_phys(struct device *hwdev, dma_addr_t baddr)
{
return baddr;
}
-static dma_addr_t swiotlb_virt_to_bus(struct device *hwdev,
+dma_addr_t __weak swiotlb_virt_to_bus(struct device *hwdev,
volatile void *address)
{
return swiotlb_phys_to_bus(hwdev, virt_to_phys(address));
}
-static void *swiotlb_bus_to_virt(dma_addr_t address)
+void __weak *swiotlb_bus_to_virt(struct device *hwdev, dma_addr_t address)
{
- return phys_to_virt(swiotlb_bus_to_phys(address));
+ return phys_to_virt(swiotlb_bus_to_phys(hwdev, address));
}
int __weak swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
@@ -687,7 +687,7 @@ void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
size_t size, enum dma_data_direction dir,
struct dma_attrs *attrs)
{
- char *dma_addr = swiotlb_bus_to_virt(dev_addr);
+ char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr);
BUG_ON(dir == DMA_NONE);
if (is_swiotlb_buffer(dma_addr))
@@ -711,7 +711,7 @@ static void
swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
size_t size, int dir, int target)
{
- char *dma_addr = swiotlb_bus_to_virt(dev_addr);
+ char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr);
BUG_ON(dir == DMA_NONE);
if (is_swiotlb_buffer(dma_addr))
@@ -744,7 +744,7 @@ swiotlb_sync_single_range(struct device *hwdev, dma_addr_t dev_addr,
unsigned long offset, size_t size,
int dir, int target)
{
- char *dma_addr = swiotlb_bus_to_virt(dev_addr) + offset;
+ char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr) + offset;
BUG_ON(dir == DMA_NONE);
if (is_swiotlb_buffer(dma_addr))
@@ -847,10 +847,14 @@ swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
for_each_sg(sgl, sg, nelems, i) {
if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
- unmap_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
- sg->dma_length, dir);
+ unmap_single(
+ hwdev,
+ swiotlb_bus_to_virt(hwdev, sg->dma_address),
+ sg->dma_length, dir);
else if (dir == DMA_FROM_DEVICE)
- dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg->dma_length);
+ dma_mark_clean(
+ swiotlb_bus_to_virt(hwdev, sg->dma_address),
+ sg->dma_length);
}
}
EXPORT_SYMBOL(swiotlb_unmap_sg_attrs);
@@ -881,10 +885,13 @@ swiotlb_sync_sg(struct device *hwdev, struct scatterlist *sgl,
for_each_sg(sgl, sg, nelems, i) {
if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
- sync_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
+ sync_single(hwdev,
+ swiotlb_bus_to_virt(hwdev, sg->dma_address),
sg->dma_length, dir, target);
else if (dir == DMA_FROM_DEVICE)
- dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg->dma_length);
+ dma_mark_clean(
+ swiotlb_bus_to_virt(hwdev, sg->dma_address),
+ sg->dma_length);
}
}
--
1.5.6.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Becky Bruce
if a device can dma to an address and need to provide their
own address_needs_mapping..
Signed-off-by: Becky Bruce <bec...@kernel.crashing.org>
---
lib/swiotlb.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index b33180e..f365735 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -145,6 +145,12 @@ void __weak *swiotlb_bus_to_virt(struct device *hwdev, dma_addr_t address)
return phys_to_virt(swiotlb_bus_to_phys(hwdev, address));
}
+int __weak swiotlb_arch_address_needs_mapping(struct device *hwdev,
+ dma_addr_t addr, size_t size)
+{
+ return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
+}
+
int __weak swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
{
return 0;
@@ -309,10 +315,10 @@ cleanup1:
return -ENOMEM;
}
-static int
+static inline int
address_needs_mapping(struct device *hwdev, dma_addr_t addr, size_t size)
{
- return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
+ return swiotlb_arch_address_needs_mapping(hwdev, addr, size);
}
static inline int range_needs_mapping(phys_addr_t paddr, size_t size)
Becky Bruce
trivially change all the comments to reference new names.
Also, there were some comments that should have been
referring to just plain old map_single, not swiotlb_map_single;
fix those as well. Also change a use of the word "pointer", when
what is referred to is actually a dma/physical address.
Signed-off-by: Becky Bruce <bec...@kernel.crashing.org>
---
lib/swiotlb.c | 24 +++++++++++-------------
1 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 32e2bd3..f59cf30 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -60,8 +60,8 @@ enum dma_sync_target {
int swiotlb_force;
/*
- * Used to do a quick range check in swiotlb_unmap_single and
- * swiotlb_sync_single_*, to see if the memory was in fact allocated by this
+ * Used to do a quick range check in unmap_single and
+ * sync_single_*, to see if the memory was in fact allocated by this
* API.
*/
static char *io_tlb_start, *io_tlb_end;
@@ -560,7 +560,6 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
size)) {
/*
* The allocated memory isn't reachable by the device.
- * Fall back on swiotlb_map_single().
*/
free_pages((unsigned long) ret, order);
ret = NULL;
@@ -568,9 +567,8 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
if (!ret) {
/*
* We are either out of memory or the device can't DMA
- * to GFP_DMA memory; fall back on
- * swiotlb_map_single(), which will grab memory from
- * the lowest available address range.
+ * to GFP_DMA memory; fall back on map_single(), which
+ * will grab memory from the lowest available address range.
*/
ret = map_single(hwdev, 0, size, DMA_FROM_DEVICE);
if (!ret)
@@ -634,7 +632,7 @@ swiotlb_full(struct device *dev, size_t size, int dir, int do_panic)
* physical address to use is returned.
*
* Once the device is given the dma address, the device owns this memory until
- * either swiotlb_unmap_single or swiotlb_dma_sync_single is performed.
+ * either swiotlb_unmap_page or swiotlb_dma_sync_single is performed.
*/
dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
unsigned long offset, size_t size,
@@ -648,7 +646,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
BUG_ON(dir == DMA_NONE);
/*
- * If the pointer passed in happens to be in the device's DMA window,
+ * If the address happens to be in the device's DMA window,
* we can safely return the device addr and not worry about bounce
* buffering it.
*/
@@ -679,7 +677,7 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
/*
* Unmap a single streaming mode DMA translation. The dma_addr and size must
- * match what was provided for in a previous swiotlb_map_single call. All
+ * match what was provided for in a previous swiotlb_map_page call. All
* other usages are undefined.
*
* After this call, reads by the cpu to the buffer are guaranteed to see
@@ -703,7 +701,7 @@ EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
* Make physical memory consistent for a single streaming mode DMA translation
* after a transfer.
*
- * If you perform a swiotlb_map_single() but wish to interrogate the buffer
+ * If you perform a swiotlb_map_page() but wish to interrogate the buffer
* using the cpu, yet do not wish to teardown the dma mapping, you must
* call this function before doing so. At the next point you give the dma
* address back to the card, you must first perform a
@@ -777,7 +775,7 @@ EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
/*
* Map a set of buffers described by scatterlist in streaming mode for DMA.
- * This is the scatter-gather version of the above swiotlb_map_single
+ * This is the scatter-gather version of the above swiotlb_map_page
* interface. Here the scatter gather list elements are each tagged with the
* appropriate dma address and length. They are obtained via
* sg_dma_{address,length}(SG).
@@ -788,7 +786,7 @@ EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
* The routine returns the number of addr/length pairs actually
* used, at most nents.
*
- * Device ownership issues as mentioned above for swiotlb_map_single are the
+ * Device ownership issues as mentioned above for swiotlb_map_page are the
* same here.
*/
int
@@ -836,7 +834,7 @@ EXPORT_SYMBOL(swiotlb_map_sg);
/*
* Unmap a set of streaming mode DMA translations. Again, cpu read rules
- * concerning calls here are the same as for swiotlb_unmap_single() above.
+ * concerning calls here are the same as for swiotlb_unmap_page() above.
*/
void
swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
Becky Bruce
with 2 different types. Cast the first arg to size_t, which is the
type of the second, and should be portable across architectures.
Signed-off-by: Becky Bruce <bec...@kernel.crashing.org>
---
lib/swiotlb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index f59cf30..62f5f75 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -341,7 +341,7 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
unsigned long flags;
while (size) {
- sz = min(PAGE_SIZE - offset, size);
+ sz = min((size_t)(PAGE_SIZE - offset), size);
local_irq_save(flags);
buffer = kmap_atomic(pfn_to_page(pfn),
Becky Bruce
be in highmem, which is bad. This wasn't needed, anyway, because
we already have the physical address we need. Get rid of the
now-unused virtual address as well.
Signed-off-by: Becky Bruce <bec...@kernel.crashing.org>
---
lib/swiotlb.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index c152f17..b33180e 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -640,7 +640,6 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
struct dma_attrs *attrs)
{
phys_addr_t phys = page_to_phys(page) + offset;
- void *ptr = page_address(page) + offset;
dma_addr_t dev_addr = swiotlb_phys_to_bus(dev, phys);
void *map;
@@ -651,7 +650,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
* buffering it.
*/
if (!address_needs_mapping(dev, dev_addr, size) &&
- !range_needs_mapping(virt_to_phys(ptr), size))
+ !range_needs_mapping(phys, size))
return dev_addr;
/*
FUJITA Tomonori
Looks fine to me. Though I guess that we could clean up
swiotlb_arch_range_needs_mapping and
swiotlb_arch_address_needs_mapping a bit (at least, the names are
confusing).
FUJITA Tomonori
Becky Bruce <bec...@kernel.crashing.org> wrote:
> Squash a build warning seen on 32-bit powerpc caused by calling min()
> with 2 different types. Cast the first arg to size_t, which is the
> type of the second, and should be portable across architectures.
>
> Signed-off-by: Becky Bruce <bec...@kernel.crashing.org>
> ---
> lib/swiotlb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index f59cf30..62f5f75 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -341,7 +341,7 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
> unsigned long flags;
>
> while (size) {
> - sz = min(PAGE_SIZE - offset, size);
> + sz = min((size_t)(PAGE_SIZE - offset), size);
>
> local_irq_save(flags);
> buffer = kmap_atomic(pfn_to_page(pfn),
?
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index f59cf30..fa62498 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -341,7 +341,7 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
unsigned long flags;
while (size) {
- sz = min(PAGE_SIZE - offset, size);
+ sz = min_t(size_t, PAGE_SIZE - offset, size);
local_irq_save(flags);
buffer = kmap_atomic(pfn_to_page(pfn),
--
FUJITA Tomonori
FUJITA Tomonori
Becky Bruce <bec...@kernel.crashing.org> wrote:
> Make these functions take the hwdev as an argument because on some
> platforms it contains a per-device offset that is used to convert
> from bus addresses to/from other types of addresses.
>
> Also, make these weak so architectures can override the default
> behavior (for example, by adding an offset in the hwdev).
>
> Signed-off-by: Becky Bruce <bec...@kernel.crashing.org>
> ---
> arch/x86/kernel/pci-swiotlb.c | 2 +-
> include/linux/swiotlb.h | 4 +++-
> lib/swiotlb.c | 31 +++++++++++++++++++------------
> 3 files changed, 23 insertions(+), 14 deletions(-)
> @@ -847,10 +847,14 @@ swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
>
> for_each_sg(sgl, sg, nelems, i) {
> if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
> - unmap_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
> - sg->dma_length, dir);
> + unmap_single(
> + hwdev,
> + swiotlb_bus_to_virt(hwdev, sg->dma_address),
> + sg->dma_length, dir);
> else if (dir == DMA_FROM_DEVICE)
> - dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg->dma_length);
> + dma_mark_clean(
> + swiotlb_bus_to_virt(hwdev, sg->dma_address),
> + sg->dma_length);
> }
The coding style looks a bit odd to me. How about something like this?
for_each_sg(sgl, sg, nelems, i) {
void *virt = swiotlb_bus_to_virt(hwdev, sg->dma_address),
if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
unmap_single(hwdev, virt, sg->dma_length, dir);
else if (dir == DMA_FROM_DEVICE)
dma_mark_clean(virt, sg->dma_length);
> }
> EXPORT_SYMBOL(swiotlb_unmap_sg_attrs);
> @@ -881,10 +885,13 @@ swiotlb_sync_sg(struct device *hwdev, struct scatterlist *sgl,
>
> for_each_sg(sgl, sg, nelems, i) {
> if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
> - sync_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
> + sync_single(hwdev,
> + swiotlb_bus_to_virt(hwdev, sg->dma_address),
> sg->dma_length, dir, target);
> else if (dir == DMA_FROM_DEVICE)
> - dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg->dma_length);
> + dma_mark_clean(
> + swiotlb_bus_to_virt(hwdev, sg->dma_address),
> + sg->dma_length);
> }
Ditto.
The rest looks fine to me.
Becky Bruce
OK, we're clearly pointed at different trees here, and it looks like
I'm behind - this patch is based on Ingo's master. Which tree has
this change?
Thanks,
Becky
FUJITA Tomonori
Becky Bruce <bec...@kernel.crashing.org> wrote:
I also use tip/master. I think that we are on the same page. No tree
has this change. I just send it because I'm not sure why you don't use
min_t().
Becky Bruce
On Mar 24, 2009, at 9:58 PM, FUJITA Tomonori wrote:
Heh, I was trying to avoid > 80 character lines there, and it ended up
looking a bit gross. I originally had *exactly* what you suggest in
my tree, but changed it, so I'm more than happy to change this back.
>
>
>
>> }
>> EXPORT_SYMBOL(swiotlb_unmap_sg_attrs);
>> @@ -881,10 +885,13 @@ swiotlb_sync_sg(struct device *hwdev, struct
>> scatterlist *sgl,
>>
>> for_each_sg(sgl, sg, nelems, i) {
>> if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
>> - sync_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
>> + sync_single(hwdev,
>> + swiotlb_bus_to_virt(hwdev, sg->dma_address),
>> sg->dma_length, dir, target);
>> else if (dir == DMA_FROM_DEVICE)
>> - dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg-
>> >dma_length);
>> + dma_mark_clean(
>> + swiotlb_bus_to_virt(hwdev, sg->dma_address),
>> + sg->dma_length);
>> }
>
> Ditto.
>
>
> The rest looks fine to me.
Thanks for the review!
Cheers,
-Becky
Ingo Molnar
yes, min_t() is safer and cleaner than min()+cast.
Ingo
Ingo Molnar
that looks odd too.
> Heh, I was trying to avoid > 80 character lines there, and it
> ended up looking a bit gross. I originally had *exactly* what you
> suggest in my tree, but changed it, so I'm more than happy to
> change this back.
The proper solution is to split out the loop body into a helper
function, __swiotlb_unmap_sg_attrs() or so. Something like:
__swiotlb_unmap_sg_attrs()
{
if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
unmap_single(hwdev,
swiotlb_bus_to_virt(hwdev, sg->dma_address),
sg->dma_length, dir);
return;
}
if (dir != DMA_FROM_DEVICE)
return;
dma_mark_clean(swiotlb_bus_to_virt(hwdev, sg->dma_address),
sg->dma_length);
}
Ingo
Becky Bruce
Ahh, then, that's just an oversight on my part, and I'll fix it. I'll
send out a respun set of patches as soon as I'm out of today's endless
set of meetings.
Thanks,
Becky