From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave McCracken Subject: Re: [Linux PATCH] Fix to hugepages to work around new PWT handling Date: Wed, 9 Jun 2010 14:26:03 -0500 Message-ID: <201006091426.03926.dcm@mccr.org> References: <20100609140252.18351.48175.sendpatchset@magnum.int.mccr.org> <201006091335.52854.dcm@mccr.org> <4C0FE359.1000305@goop.org> Mime-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4C0FE359.1000305@goop.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jeremy Fitzhardinge Cc: Xen Developers List List-Id: xen-devel@lists.xenproject.org On Wednesday, June 09, 2010, Jeremy Fitzhardinge wrote: > >>> 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. Um, this is the upper level code. The entire purpose of make_huge_pte is to construct a present huge pte from page and pgprot. The problem is that the original code makes the pte, then sets the present bit via pte_mkhuge(). This means the Xen-specific macro that triggers on present is misled and doesn't do the pfn_to_mfn(). Without this patch hugepages is handing pfns to the hypervisor to map instead of mfns. Dave