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

Jeremy Fitzhardinge
2008-01-09T01:59:28Z

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