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: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Subject: [Xen-devel] Re: [PATCH 6 of 7] x86: use early_ioremap in __acpi_map_table
From: Ingo Molnar <mingo@xxxxxxx>
Date: Mon, 8 Sep 2008 16:26:19 +0200
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Andi Kleen <andi@xxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, "H. Peter Anvin" <hpa@xxxxxxxxx>
Delivery-date: Mon, 08 Sep 2008 07:26:55 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <48C46BCB.2060209@xxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
* Jeremy Fitzhardinge <jeremy@xxxxxxxx> wrote:

> >> However, unlike early_ioremap(), __acpi_map_table() just maintains 
> >> a single mapping which gets replaced each call, and has no 
> >> corresponding unmap function.  Implement this by just removing the 
> >> previous mapping each time its called.  Unfortunately, this will 
> >> leave a stray mapping at the end.
> >
> > It would be better to just fix the ACPI code to unmap.
> I was concerned that would cause lots of cross-arch churn, but of 
> course the only other relevant architecture is ia64.  I'll prep a 
> followup patch.

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).

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.

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.


Xen-devel mailing list