This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [PATCH 6 of 7] x86: use early_ioremap in __acpi_map_tabl

To: Ingo Molnar <mingo@xxxxxxx>
Subject: [Xen-devel] Re: [PATCH 6 of 7] x86: use early_ioremap in __acpi_map_table
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Mon, 08 Sep 2008 09:29:12 -0700
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Andi Kleen <andi@xxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, "H. Peter Anvin" <hpa@xxxxxxxxx>
Delivery-date: Mon, 08 Sep 2008 09:29:36 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20080908142619.GA10580@xxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1220826072@localhost> <944fe7ea3da7707eb90f.1220826078@localhost> <20080907234418.GB26079@xxxxxxxxxxxxxxxxxx> <48C46BCB.2060209@xxxxxxxx> <20080908142619.GA10580@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird (X11/20080723)
Ingo Molnar wrote:
> uhm, there's a nasty trap in that route: it can potentially cause a lot 
> of breakage.
> It's not robust to assume that the ACPI code is sane wrt. 
> mapping/unmapping, because it currently simply doesnt rely on robust 
> unmapping (in the linear range).

You mean there's code which just assumes that it can keep using a
linear-mapped acpi even after __acpi_map_table() should have implicitly
unmapped it?
> I tried it in the past and i found tons of crappy ACPI code all around 
> that just never unmapped tables. Leaking ACPI maps are hard to find as 
> well, and it can occur anytime during bootup.

__acpi_map_table() is called by acpi_map_table(), which does have a
acpi_unmap_table() counterpart.  But it only calls iounmap() once we're
past the stage of calling early_*().  I could easily make it call
__acpi_unmap_table()->early_iounmap().  But if the concern is that the
early boot callers of acpi_map_table() "know" that they never need to
unmap, then yes, I see the problem.

> As a general principle it might be worth fixing those places, and we've 
> hardened up the early-ioremap code for leaks during the PAT rewrite, 
> still please realize that it can become non-trivial and it might cause a 
> lot of unhappy users.
> So i'd suggest a different, more carful approach: keep the new code you 
> wrote, but print a WARN()ing if prev_map is not unmapped yet when the 
> next mapping is acquired. That way the ACPI code can be fixed gradually 
> and without breaking existing functionality.


> There's another complication: ACPI might rely on multiple mappings being 
> present at once, so unmapping the previous one might not be safe. But it 
> _should_ be fine most of the time as __acpi_map_table() is only used 
> inearly init code - and we fixed most of these things in the PAT 
> patchset in any case.

And the current behaviour of __acpi_map_table() is to remove the
previous mapping (implicitly, by overwriting the same fixmap slots), so
its only an issue if the callers assume they can keep using
linear-mapped acpi tables after a subsequent call to __acpi_map_table().


Xen-devel mailing list