Ingo Molnar wrote:
* Ingo Molnar <[EMAIL PROTECTED]> wrote:
#define __PAGE_KERNEL_EXEC \
- (_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_GLOBAL)
+ (_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED)
This shouldn't be necessary. The old 64-bit code defined everything
without _PAGE_GLOBAL, but then used a MAKE_GLOBAL() macro to OR it
in later. This seemed a bit roundabout to me, so I just put it in
from the outset.
actually, this is wrong.
a couple of places use __PAGE_* values, which you've now changed to
include the _PAGE_GLOBAL flag.
yep, fixing this resolves the crash.
Bugger. OK.
And I don't see quite how the global flag is causing the BUG bug in
change_page_attr(). The logic is:
if (pgprot_val(prot) != pgprot_val(ref_prot)) {
...
} else {
if (!pte_huge(*kpte)) {
...
} else
BUG();
}
Is _PAGE_GLOBAL causing the first if() to fall through to the second
clause? Because otherwise it shouldn't have any effect on the
pte_huge() test.
But given that ref_prot is set to PAGE_KERNEL or PAGE_KERNEL_EXEC, which
will have _PAGE_GLOBAL in it either way, I don't see where the problem
is coming from.
Gah! This can't be right! I think the original change_page_attr() code
is plain buggy.
The crash call chain is:
[<ffffffff8021db68>] change_page_attr_addr+0x9e/0x119
[<ffffffff8021d44f>] ioremap_change_attr+0x49/0x58
[<ffffffff8021d626>] iounmap+0xbe/0xe0
...
ioremap_change_attr does:
err =
change_page_attr_addr(vaddr,npages,__pgprot(__PAGE_KERNEL|flags));
Now, in the current code (ie, before my patch), __PAGE_KERNEL doesn't
have _PAGE_GLOBAL set, but PAGE_KERNEL does. Therefore,
change_page_attr_addr calls
__change_page_attr(address, pfn, prot, PAGE_KERNEL);
which means:
__change_page_attr(address, pfn, pgprot(__PAGE_KERNEL), PAGE_KERNEL);
(iounmap always passes flags of 0) which just happens to fail the test:
if (pgprot_val(prot) != pgprot_val(ref_prot)) {
because prot doesn't contain _PAGE_GLOBAL and ref_prot does.
In other words, prot and ref_prot can never be equal, so this path is
always taken, and the other branch which tests pte_huge() is never run.
Andi? Jan? Is this code just buggy, or is there something else going
on here?
J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
