Re: [Linux PATCH] Fix to hugepages to work around new PWT handling

[Jeremy Fitzhardinge <jeremy@goop.org>]

On 06/09/2010 11:35 AM, Dave McCracken wrote:
> On Wednesday, June 09, 2010, Jeremy Fitzhardinge wrote:
>   
>>> Recent changes to Linux include code to set new flags in the pte,
>>> including _PAGE_PAT and _PAGE_PWT.  That change conflicts with hugepage
>>> using the pte macros to set up its pmd entries.  This patch resolves
>>> that problem.
>>>
>>>   
>>>       
>> Could you explain this a bit more clearly?  Why is using __pmd not
>> working in this case?  Is it because the kernel is now setting PAT and
>> PWT on huge pages?  But PAT isn't even the same flag for huge pages...
>>     
> For some reason the latest xen_make_pte() and xen_pte_val() are attempting to 
> do some magic with PAT, PCD, and PWT.  The previous version of the code in 
> set_huge_pte_at() did "__pmd(pte_val(pte))", and the end result was that PSE 
> got turned off and PWT was turned on.  Doing the native versions of those calls 
> avoids this issue, since all the code is really trying to do here is a 
> typecast.
>   

Yes, the pte code is converting the Linux PAT encodings to the Xen ones,
so that the resulting mappings have the right page properties.

Does the Linux hugepage code set PAT_LARGE on huge ptes, and does Xen
support huge mappings with PAT properties?  If so, you'll probably need
to do the same thing.

> I realize the more complete fix probably involves something like converting 
> hugepages to use pmd throughout instead of pte, but that's a much bigger 
> change and this solves the immediate problem.
>   

I think that's actually pretty straightforward.  It shouldn't be using
the plain pte/pmd macros anyway, but the huge_ counterparts.  huge_pte
should map to pmd.

But this portion of the patch looks OK for now.

>>> An additional fix here is to make sure the _PAGE_PRESENT bit is set
>>> before hugepages does a mk_pte(), since Xen depends on that bit to
>>> trigger the pfn->mfn translation.
>>>
>>>   
>>>       
>> Why is the kernel creating a non-present mapping?  If it isn't present,
>> why does it matter whether we do the pfn->mfn conversion?
>>     
> The hugepage function make_huge_pte() called mk_pte() to turn a page and a 
> pgprot into a pte before it set PRESENT.  The PRESENT flag was set after the 
> pte was made.  This meant that the Xen version of the macro did not see 
> PRESENT so did not do the pfn_to_mfn().  My patch sets PRESENT first so the 
> right thing will happen.
>   

But in general kernel code shouldn't be just nakedly setting present on
the pte without also remaking the whole thing.  That doesn't happen with
normal ptes, and it probably shouldn't happen with huge ptes.  Forcing
present on a pte at this level seems very bogus.   Why not change the
upper code to set present if that's want it wants?

I'll skip this chunk for now.

    J