Re: [PATCH] swiotlb updates for Xen dom0

From: FUJITA Tomonori
Date: Thu Apr 02 2009 - 02:31:02 EST


On Tue, 31 Mar 2009 15:52:06 -0700
Jeremy Fitzhardinge <jeremy@xxxxxxxx> wrote:

> This series adds Xen support to x86 swiotlb. This is mostly a matter of
> adding some Xen code into the existing hooks in pci-swiotlb_64.c:
> - swiotlb_alloc_boot
> - swiotlb_arch_range_needs_mapping
> - swiotlb_phys_to_bus/bus_to_phys
>
> These hooks are conditional on xen_pv_domain(), which compiles to a
> constant 0 when CONFIG_XEN is not enabled (and is a simple variable
> read when it is).
>
> All the Xen-specific code is in xen-iommu.c.
>
> This series is just the patches which touch lib/swiotlb.c or
> pci-swiotlb_64.c. You can see them with more context in:
> git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git for-linus/xen/dom0/swiotlb
>
> Thanks,
> J
>
> arch/x86/kernel/pci-swiotlb_64.c | 31 ++++++++++++++++-
> arch/x86/xen/Kconfig | 1
> drivers/pci/xen-iommu.c | 68 ++++++++++++++++++++++++++++++++++++---
> include/xen/swiotlb.h | 38 +++++++++++++++++++++
> lib/swiotlb.c | 3 +
> 5 files changed, 133 insertions(+), 8 deletions(-)

This patchset looks ugly.

You add 'if we are Xen, we do A. We do B if not' code into 6 functions
though pic-swiotlb_64.c has only 8 functions. In addition, 5 of 8
functions were added for Xen.

Why can't you have something like arch/x86/xen/pci-swiotlb.c, which
works as arch/x86/kernel/pci-swiotlb_64.c? That should be much
cleaner.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/