On Fri, Dec 22, 2006 at 02:49:53PM +0000, Jan Beulich wrote:
> Patch update, fixing a bug on x86/PAE, and making
> include/xen/swiotlb.h look a lot nicer (but still not really
> nice). My plan is to submit the non-Xen ones to lkml right after New
> Year, unless I hear negative feedback.
> >Patch order is
> >swiotlb-bugs.patch
> >swiotlb-bus.patch
> >swiotlb-cleanup.patch
> >swiotlb-split.patch
> >xen-swiotlb.patch
Comments inline.
swiotlb-bugs.patch:
[snip]
> /*
> @@ -758,8 +739,10 @@ swiotlb_sync_sg(struct device *hwdev, st
>
> for (i = 0; i < nelems; i++, sg++)
> if (sg->dma_address != SG_ENT_PHYS_ADDRESS(sg))
> - sync_single(hwdev, (void *) sg->dma_address,
> + sync_single(hwdev, phys_to_virt(sg->dma_address),
> sg->dma_length, dir, target);
Fix looks correct and bug looks painful. I think you should send this
one to mainline immediately.
swiotlb-bus.patch:
> Convert all phys_to_virt/virt_to_phys uses to
> bus_to_virt/virt_to_bus.
"... because Xen needs it", otherwise someone is bound to ask why, as
all other archs define _bus as _phys.
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
Acked-by: Muli Ben-Yehuda <muli@xxxxxxxxxx>
swiotlb-cleanup:
> This patch
> - adds proper __init decoration to swiotlb's init code (and the code calling
> it, where not already the case)
> - replaces uses of 'unsigned long' with dma_addr_t where appropriate
> - does miscellaneous simplicfication and cleanup
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
Looks good in general, not acking yet because I want to give it a spin
first.
swiotlb-split.patch:
> This patch adds abstraction so that the file can be used by environments other
> than IA64 and EM64T, namely for Xen.
I'm sorry, but this patch is horrible. swiotlb.c is now pretty much
unreadable. I'd be surprised if mainline accepted it - I would
certainly NAK it with my mainline hat on, especially for an unmerged
architecture.
If Xen needs so many "abstractions", I have to ask whether it isn't
better off just using its own swiotlb.c as we are now.
I'll take another look at this later and try to come up with a
different way of merging them that isn't quite this horrible. Maybe
using function pointers for the "low level" operations?
Cheers,
Muli
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home