* [PATCH 1/1] EFI iounpam fix for acpi_os_unmap_memory
@ 2006-02-20 23:36 Edgar Hucek
2006-02-21 6:02 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Edgar Hucek @ 2006-02-20 23:36 UTC (permalink / raw)
To: linux-acpi, linux-kernel
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 <hostmaster@ed-soft.at>
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);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] EFI iounpam fix for acpi_os_unmap_memory
2006-02-20 23:36 [PATCH 1/1] EFI iounpam fix for acpi_os_unmap_memory Edgar Hucek
@ 2006-02-21 6:02 ` Andrew Morton
2006-02-21 14:15 ` Andi Kleen
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2006-02-21 6:02 UTC (permalink / raw)
To: Edgar Hucek; +Cc: linux-acpi, linux-kernel, Matt Domsch
Edgar Hucek <hostmaster@ed-soft.at> 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 <hostmaster@ed-soft.at>
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 <hostmaster@ed-soft.at>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
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);
_
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] EFI iounpam fix for acpi_os_unmap_memory
2006-02-21 6:02 ` Andrew Morton
@ 2006-02-21 14:15 ` Andi Kleen
2006-02-21 20:59 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2006-02-21 14:15 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-acpi, linux-kernel, Matt Domsch, hostmaster
Andrew Morton <akpm@osdl.org> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] EFI iounpam fix for acpi_os_unmap_memory
2006-02-21 14:15 ` Andi Kleen
@ 2006-02-21 20:59 ` Andrew Morton
2006-02-21 21:09 ` Andi Kleen
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2006-02-21 20:59 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-acpi, linux-kernel, Matt_Domsch, hostmaster
Andi Kleen <ak@suse.de> wrote:
>
> Andrew Morton <akpm@osdl.org> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] EFI iounpam fix for acpi_os_unmap_memory
2006-02-21 20:59 ` Andrew Morton
@ 2006-02-21 21:09 ` Andi Kleen
2006-02-22 6:50 ` Stelian Pop
0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2006-02-21 21:09 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-acpi, linux-kernel, Matt_Domsch, hostmaster, Bjorn Helgaas
On Tuesday 21 February 2006 21:59, Andrew Morton wrote:
> Andi Kleen <ak@suse.de> wrote:
> >
> > Andrew Morton <akpm@osdl.org> 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 <ak@suse.de>
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;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] EFI iounpam fix for acpi_os_unmap_memory
2006-02-21 21:09 ` Andi Kleen
@ 2006-02-22 6:50 ` Stelian Pop
0 siblings, 0 replies; 6+ messages in thread
From: Stelian Pop @ 2006-02-22 6:50 UTC (permalink / raw)
To: Andi Kleen
Cc: Andrew Morton, linux-acpi, linux-kernel, Matt_Domsch, hostmaster,
Bjorn Helgaas
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 <stelian@popies.net>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-02-22 6:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-20 23:36 [PATCH 1/1] EFI iounpam fix for acpi_os_unmap_memory Edgar Hucek
2006-02-21 6:02 ` Andrew Morton
2006-02-21 14:15 ` Andi Kleen
2006-02-21 20:59 ` Andrew Morton
2006-02-21 21:09 ` Andi Kleen
2006-02-22 6:50 ` Stelian Pop
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox