On 08.04.2013 16:15, Borislav Petkov wrote: > On Mon, Apr 08, 2013 at 03:10:00PM +0200, Stefan Bader wrote: >> * that we limited the number of possible pages already to >> * the number of pages in the large page. >> */ >> if (address == (address & pmask) && cpa->numpages == (psize >> >> PAGE_SHIFT)) { >> /* >> * The address is aligned and the number of pages >> * covers the full page. >> */ >> new_pte = pfn_pte(pte_pfn(old_pte), new_prot); >> ^ >> >> This one. The first patch changed >> >> - new_pte = pfn_pte(pte_pfn(old_pte), canon_pgprot(new_prot)); >> + new_pte = pfn_pte(pte_pfn(old_pte), new_prot); >> >> The fixup patch drops new_prot completely from being initialized and only >> works >> on req_prot. Probably it would be best to also drop the definition of >> new_prot. >> I think it then completely unused. > > Actually, we do need and initialize new_prot at line 495: > > pfn = pte_pfn(old_pte) + ((address & (psize - 1)) >> PAGE_SHIFT); > cpa->pfn = pfn; > > new_prot = static_protections(req_prot, address, pfn); <---
You are right. Seems I missed that and a couble of other places. I can see them now... Hm, Monday morning or just morning issue... So, yes, its new_prot is initialized and is still needed, otherwise the loop over the whole range would be subtly different. Sorry for the noise. -Stefan > > and we need it for the subsequent loop where we go over the 512 PTEs to > decide whether to split or not. > > So it is needed after all, AFAICT. >
signature.asc
Description: OpenPGP digital signature