Re: [PATCH 00 of 10] x86: unify asm/pgtable.h

From: Jeremy Fitzhardinge
Date: Tue Jan 08 2008 - 20:59:28 EST


Andi Kleen wrote:
Or to put it another way, what's the underlying rationale for making __PAGE_KERNEL_* not include the GLOBAL flag, but including it in the pgprot versions? It means that code like this in ioremap_64.c:

There is none, but that is not what change_page_attr() cares about.
It just cares that you pass in the correct bits and you don't currently.

I think you're missing the forrest because of all the trees currently.

Yeah, that may be true, but this particular tree is weird, and I'm trying to understand what's going on here. Specifically, 64-bit ioremap()s *don't* set _PAGE_GLOBAL, which appears to be an accident resulting from the strange definitions of __PAGE_KERNEL_* vs PAGE_KERNEL_*.

For example, ioremap_64.c:__ioremap() creates a vma for the io mapping, and explicitly sets _PAGE_GLOBAL in the vma's version of pgprot - but then it calls ioremap_page_range() to actually create the mapping, which ends up making a non-global mapping, because its rolling its own version of PAGE_KERNEL by using pgprot(__PAGE_KERNEL) - which is not the actual definition of PAGE_KERNEL.

I think there's a bug around here, but I think its currently being hidden by accident. I think my changes are correct, and they're exposing some other bug. But I don't really understand how all this stuff is supposed to fit together, so I'm looking for an explanation of what's supposed to be happening - and ideally - why the current code isn't actually buggy.

For example: is ioremap_change_attr() actually *deliberately* creating non-global mappings? Or is it an accident? And if it really intends to create non-global mappings, why? And why is it buggy for it to create global mappings?

J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/