Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752733AbbKPVU6 (ORCPT ); Mon, 16 Nov 2015 16:20:58 -0500 Received: from mx2.suse.de ([195.135.220.15]:38133 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751903AbbKPVUx (ORCPT ); Mon, 16 Nov 2015 16:20:53 -0500 Date: Mon, 16 Nov 2015 22:20:42 +0100 From: Borislav Petkov To: Thomas Gleixner Cc: Matt Fleming , Ingo Molnar , "H . Peter Anvin" , Toshi Kani , linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org, Sai Praneeth Prakhya , Dave Hansen Subject: Re: [PATCH v2 1/5] x86/mm/pageattr: Ensure cpa->pfn only contains page frame numbers Message-ID: <20151116212042.GE20435@pd.tnic> References: <1447538451-5793-1-git-send-email-matt@codeblueprint.co.uk> <1447538451-5793-2-git-send-email-matt@codeblueprint.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1925 Lines: 52 On Mon, Nov 16, 2015 at 09:19:01PM +0100, Thomas Gleixner wrote: > On Sat, 14 Nov 2015, Matt Fleming wrote: > > The x86 pageattr code is confused about the data that is stored > > cpa->pfn, sometimes it's treated as a page frame number, sometimes > > it's treated as an unshifted physical address, and in one place it's > > treated as a pte. > > Yuck. This paragraph should read like this instead: "Boris used cpa->pfn as a scratch variable to contain the physical address. He realizes now that he should've added a separate cpa_data.phys_addr then, instead of confusing everybody." > > The result of this is that the mapping functions do not map the > > intended physical address. > > > > This isn't a problem in practice because most of the addresses we're > > mapping in the EFI code paths are already mapped in 'trampoline_pgd' > > and so the pageattr mappings functions don't actually do anything in > > this case. But when we move to using a separate page table for the EFI > > runtime this will be an issue. > > Are you sure that this does not affect existing kernel versions? Shouldn't because with this new patchset we're copying all the PGDs from the kernel page table before doing an EFI call, see efi_sync_low_kernel_mappings() in patch 5. > > while (num_pages-- && start < end) { > > - > > - /* deal with the NX bit */ > > - if (!(pgprot_val(pgprot) & _PAGE_NX)) > > - cpa->pfn &= ~_PAGE_NX; > > That should be a seperate patch because this is just bogus code and > has nothing to do with the pfn confusion. Why bogus? -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/