|
xen-devel
[Xen-devel] Re: [PATCH 00 of 14] swiotlb/x86: lay groundwork for xen dom
FUJITA Tomonori wrote:
On Wed, 17 Dec 2008 08:31:43 -0800
Jeremy Fitzhardinge <jeremy@xxxxxxxx> wrote:
I think that the whole patchset is against the swiotlb design. swiotlb
is designed to be used as a library. Each architecture implements the
own swiotlb by using swiotlb library
(e.g. arch/x86/kernel/pci-swiotlb_64.c).
The whole patchset? The bulk of the changes to lib/swiotlb.c are
relatively minor to remove the unwarranted assumptions it is making in
the face of a new user. They will have no effect on other existing
users, including non-Xen x86 builds.
If you have specific objections we can discuss those, but I don't think
there's anything fundamentally wrong with making lib/swiotlb.c a bit
more generically useful.
Sorry, but the highmem support is not generically useful.
That's a circular argument. lib/swiotlb currently used by 1 1/2 of the
23 architectures, neither of which happens to use highmem. If you
consider swiotlb to be a general purpose mechanism, then presumably the
other 21 1/2 architectures are at least potential users (and 6 1/2 of
those have highmem configurations). If you base your judgement of
what's a "generically useful" change based on what the current users
need, then you'll naturally exclude the requirements of all the other
(potential) users.
And the matter arises now because we're trying to unify the use of
swiotlb in x86, bringing the number of users up to 2.
I'm especially against the highmem support. As you said, the rest
looks fine but if you go with pci-swiotlb_32.c, I think that you don't
need the most of them.
I really don't want to have to duplicate a lot of code just to
incorporate a few small changes. In fact the original Xen patch set
included its own swiotlb implementation, and that was rejected on the
grounds that we should use the common swiotlb.c.
This patch set abstracts out a few more architecture-specific details,
to allow the common swiotlb logic to be in one place. By not putting
highmem support into lib/swiotlb.c, you're effectively saying that we
should have separate lib/swiotlb-nohighmem.c and lib/swiotlb-highmem.c
files, with almost identical contents aside from the few places where we
need to use page+offset rather than a kernel vaddr.
Yes, highmem is ugly, and there'll be cheering in the streets when we
can finally kill the last highmem user. But for now we need to deal
with it, and unfortunately swiotlb is now one of the places that it
affects. Its true that adding highmem support to swiotlb.c increases
the maintenance burden to some extent. But I can't see an alternative
that doesn't increase the burden a lot more.
We can look at abstracting things a bit more so that non-highmem
architectures can keep using simple addresses rather than page+offset,
but that highmem code has got to be *somewhere*.
> have zero effect on the code generated for IA64 or x86-64. This is not
> really a Xen-specific change, but a result of adding swiotlb support for
> i386. Other architectures which support a notion of highmem would also
> need this code if they wanted to use swiotlb.
Can you be more specific? What architecture is plan to use highmem
support in swiotlb?
Well, concretely, x86-32. I don't know about the others, but I don't
see why there's an inherent reason that they won't have a problem that
swiotlb solves.
J
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
Home