When EFI is enabled acpi_os_unmap_memory trys to unmap memory
which was not mapped by acpi_os_map_memory.
This patch for it.
Signed-off-by: Edgar Hucek <[email protected]>
diff -uNr linux-2.6.16-rc4.orig/drivers/acpi/osl.c
linux-2.6.16-rc4/drivers/acpi/osl.c
--- linux-2.6.16-rc4.orig/drivers/acpi/osl.c 2006-02-19
18:48:58.000000000 +0100
+++ linux-2.6.16-rc4/drivers/acpi/osl.c 2006-02-20 15:31:44.000000000 +0100
@@ -208,7 +208,13 @@
void acpi_os_unmap_memory(void __iomem * virt, acpi_size size)
{
- iounmap(virt);
+ if(efi_enabled) {
+ if (!(EFI_MEMORY_WB &
efi_mem_attributes(virt_to_phys(virt)))) {
+ iounmap(virt);
+ }
+ } else {
+ iounmap(virt);
+ }
}
EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
Edgar Hucek <[email protected]> wrote:
>
> When EFI is enabled acpi_os_unmap_memory trys to unmap memory
> which was not mapped by acpi_os_map_memory.
Your email client replaces tabs with spaces and wordwraps things.
The patch could be cleaned up a bit..
Matt, ACPi people: please ack or nack asap.
From: Edgar Hucek <[email protected]>
When EFI is enabled acpi_os_unmap_memory t] rys to unmap memory
which was not mapped by acpi_os_map_memory.
Signed-off-by: Edgar Hucek <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
drivers/acpi/osl.c | 4 ++++
1 files changed, 4 insertions(+)
diff -puN drivers/acpi/osl.c~efi-iounpam-fix-for-acpi_os_unmap_memory drivers/acpi/osl.c
--- devel/drivers/acpi/osl.c~efi-iounpam-fix-for-acpi_os_unmap_memory 2006-02-20 21:55:48.000000000 -0800
+++ devel-akpm/drivers/acpi/osl.c 2006-02-20 21:58:36.000000000 -0800
@@ -208,6 +208,10 @@ EXPORT_SYMBOL_GPL(acpi_os_map_memory);
void acpi_os_unmap_memory(void __iomem * virt, acpi_size size)
{
+ /* Don't unmap memory which was not mapped by acpi_os_map_memory */
+ if (efi_enabled &&
+ (efi_mem_attributes(virt_to_phys(virt)) & EFI_MEMORY_WB))
+ return;
iounmap(virt);
}
EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
_
Andrew Morton <[email protected]> writes:
>
> void acpi_os_unmap_memory(void __iomem * virt, acpi_size size)
> {
> + /* Don't unmap memory which was not mapped by acpi_os_map_memory */
> + if (efi_enabled &&
> + (efi_mem_attributes(virt_to_phys(virt)) & EFI_MEMORY_WB))
> + return;
The patch is wrong because if the address came from ioremap
virt_to_phys doesn't give the real physical address. Also looking
at acpi_os_map_memory it doesn't quite match the logic there.
One working way to check for ioremap memory is
virt >= VMALLOC_START && virt < VMALLOC_END
-Andi
Andi Kleen <[email protected]> wrote:
>
> Andrew Morton <[email protected]> writes:
>
> >
> > void acpi_os_unmap_memory(void __iomem * virt, acpi_size size)
> > {
> > + /* Don't unmap memory which was not mapped by acpi_os_map_memory */
> > + if (efi_enabled &&
> > + (efi_mem_attributes(virt_to_phys(virt)) & EFI_MEMORY_WB))
> > + return;
>
>
> The patch is wrong because if the address came from ioremap
> virt_to_phys doesn't give the real physical address. Also looking
> at acpi_os_map_memory it doesn't quite match the logic there.
>
> One working way to check for ioremap memory is
> virt >= VMALLOC_START && virt < VMALLOC_END
>
OK, thanks. I don't think we actually know who is trying to unmap some
memory which acpi didn't map.
Edgar, can you please describe the bug which you're trying to fix?
On Tuesday 21 February 2006 21:59, Andrew Morton wrote:
> Andi Kleen <[email protected]> wrote:
> >
> > Andrew Morton <[email protected]> writes:
> >
> > >
> > > void acpi_os_unmap_memory(void __iomem * virt, acpi_size size)
> > > {
> > > + /* Don't unmap memory which was not mapped by acpi_os_map_memory */
> > > + if (efi_enabled &&
> > > + (efi_mem_attributes(virt_to_phys(virt)) & EFI_MEMORY_WB))
> > > + return;
> >
> >
> > The patch is wrong because if the address came from ioremap
> > virt_to_phys doesn't give the real physical address. Also looking
> > at acpi_os_map_memory it doesn't quite match the logic there.
> >
> > One working way to check for ioremap memory is
> > virt >= VMALLOC_START && virt < VMALLOC_END
> >
>
> OK, thanks. I don't think we actually know who is trying to unmap some
> memory which acpi didn't map.
>
> Edgar, can you please describe the bug which you're trying to fix?
I think the bug is clear - the logic in acpi_os_unmap_memory needs to match
what acpi_os_map_memory() does for EFI. In particular this means not calling
iounmap.
He probably has a EFI system where this caused troubles.
But think even acpi_os_miap_memory is broken - I doubt the efi_mem_attributes
thing guarantees that the memory is mapped. I guess this was added for
IA64 because ioremap used to return uncached memory there and IA64 doesn't
like this for memory accesses or something like that.
But Bjorn afaik recently fixed this by making ioremap handle it. So the right
fix probably would be to just drop all the efi_enabled gunk in acpi_os_map_memory
and just always use ioremap()
Also removed this ptr > ULONG_MAX check. Obvious it can never trigger.
<untested patch follows>
Signed-off-by: Andi Kleen <[email protected]>
Index: linux/drivers/acpi/osl.c
===================================================================
--- linux.orig/drivers/acpi/osl.c
+++ linux/drivers/acpi/osl.c
@@ -182,23 +182,10 @@ acpi_status
acpi_os_map_memory(acpi_physical_address phys, acpi_size size,
void __iomem ** virt)
{
- if (efi_enabled) {
- if (EFI_MEMORY_WB & efi_mem_attributes(phys)) {
- *virt = (void __iomem *)phys_to_virt(phys);
- } else {
- *virt = ioremap(phys, size);
- }
- } else {
- if (phys > ULONG_MAX) {
- printk(KERN_ERR PREFIX "Cannot map memory that high\n");
- return AE_BAD_PARAMETER;
- }
- /*
- * ioremap checks to ensure this is in reserved space
- */
- *virt = ioremap((unsigned long)phys, size);
- }
-
+ /*
+ * ioremap checks to ensure this is in reserved space
+ */
+ *virt = ioremap((unsigned long)phys, size);
if (!*virt)
return AE_NO_MEMORY;
>
Le mardi 21 f?vrier 2006 ? 22:09 +0100, Andi Kleen a ?crit :
> On Tuesday 21 February 2006 21:59, Andrew Morton wrote:
> > OK, thanks. I don't think we actually know who is trying to unmap some
> > memory which acpi didn't map.
> >
> > Edgar, can you please describe the bug which you're trying to fix?
>
> I think the bug is clear - the logic in acpi_os_unmap_memory needs to match
> what acpi_os_map_memory() does for EFI. In particular this means not calling
> iounmap.
>
> He probably has a EFI system where this caused troubles.
This EFI system is the new Intel Core Duo based Apple iMac (Edgar
described the process of booting Linux on this pretty box at
http://www.mactel-linux.org/)
In particular, the iounmap problem is visible in the logs at
http://www.mactel-linux.org/wiki/Dmesg
Stelian.
--
Stelian Pop <[email protected]>