From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752141AbdLLKiY (ORCPT ); Tue, 12 Dec 2017 05:38:24 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:39405 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751499AbdLLKiX (ORCPT ); Tue, 12 Dec 2017 05:38:23 -0500 X-Google-Smtp-Source: ACJfBouPvQMnOQ01b2TFQlxzAsh3taquDDI6CSDylivx+ITs5xw5mIBntGUXbMI/OO8AQG7nTyzpvA== Date: Tue, 12 Dec 2017 11:38:19 +0100 From: Ingo Molnar To: Jan Beulich Cc: Boris Ostrovsky , Juergen Gross , mingo@elte.hu, tglx@linutronix.de, xen-devel , linux-kernel@vger.kernel.org, hpa@zytor.com Subject: Re: [PATCH 2/2] x86-64/Xen: eliminate W+X mappings Message-ID: <20171212103819.an2xxafjv3cdkuy7@gmail.com> References: <5A2FBC570200007800196B3E@prv-mh.provo.novell.com> <5A2FBE540200007800196B52@prv-mh.provo.novell.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5A2FBE540200007800196B52@prv-mh.provo.novell.com> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Jan Beulich wrote: > A few thousand such pages are usually left around due to the re-use of > L1 tables having been provided by the hypervisor (Dom0) or tool stack > (DomU). Set NX in the direct map variant, which needs to be done in L2 > due to the dual use of the re-used L1s. > > For x86_configure_nx() to actually do what it is supposed to do, call > get_cpu_cap() first. This was broken by commit 4763ed4d45 ("x86, mm: > Clean up and simplify NX enablement") when switching away from the > direct EFER read. > > Signed-off-by: Jan Beulich > --- > While I certainly dislike the added header inclusion to obtain the > prototype for get_cpu_cap(), I couldn't find a better alternative. I'm > open to suggestions. > --- > arch/x86/xen/enlighten_pv.c | 3 +++ > arch/x86/xen/mmu_pv.c | 10 ++++++++++ > 2 files changed, 13 insertions(+) > > --- 4.15-rc3/arch/x86/xen/enlighten_pv.c > +++ 4.15-rc3-x86_64-Xen-avoid-W+X/arch/x86/xen/enlighten_pv.c > @@ -88,6 +88,8 @@ > #include "multicalls.h" > #include "pmu.h" > > +#include "../kernel/cpu/cpu.h" /* get_cpu_cap() */ > + > void *xen_initial_gdt; > > static int xen_cpu_up_prepare_pv(unsigned int cpu); > @@ -1258,6 +1260,7 @@ asmlinkage __visible void __init xen_sta > __userpte_alloc_gfp &= ~__GFP_HIGHMEM; > > /* Work out if we support NX */ > + get_cpu_cap(&boot_cpu_data); > x86_configure_nx(); > > /* Get mfn list */ > --- 4.15-rc3/arch/x86/xen/mmu_pv.c > +++ 4.15-rc3-x86_64-Xen-avoid-W+X/arch/x86/xen/mmu_pv.c > @@ -1902,6 +1902,16 @@ void __init xen_setup_kernel_pagetable(p > /* Graft it onto L4[511][510] */ > copy_page(level2_kernel_pgt, l2); > > + /* Zap execute permission from the ident map. Due to the sharing of > + * L1 entries we need to do this in the L2. */ please use the customary (multi-line) comment style: /* * Comment ..... * ...... goes here. */ specified in Documentation/CodingStyle. > + if (__supported_pte_mask & _PAGE_NX) > + for (i = 0; i < PTRS_PER_PMD; ++i) { > + if (pmd_none(level2_ident_pgt[i])) > + continue; > + level2_ident_pgt[i] = > + pmd_set_flags(level2_ident_pgt[i], _PAGE_NX); So the line break here is quite distracting, especially considering how similar it is to the alignment of the 'continue' statement. I.e. visually it looks like control flow alignment. Would be much better to just leave it a single page and ignore checkpatch here. Thanks, Ingo