Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Mon, 12 Oct 2020 08:25:26 +0100
From:   Marc Zyngier <maz@kernel.org>
To:     l00484210 <limingwang@huawei.com>
Cc:     catalin.marinas@arm.com, will@kernel.org, broonie@kernel.org,
        suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, fanhenglong@huawei.com,
        wanghaibin.wang@huawei.com, tangnianyao@huawei.com,
        jiangyifei@huawei.com, dengkai1@huawei.com,
        zhang.zhanghailiang@huawei.com, victor.zhangxiaofeng@huawei.com
Subject: Re: [PATCH] arm64: KVM: marking pages as XN in Stage-2 does not care
 about CTR_EL0.DIC
In-Reply-To: <20201012010852.15932-1-limingwang@huawei.com>
References: <20201012010852.15932-1-limingwang@huawei.com>
User-Agent: Roundcube Webmail/1.4.9
Message-ID: <47f80f46b9bac66846871b2db32a3f92@kernel.org>
Precedence: bulk

Hi Li,

On 2020-10-12 02:08, l00484210 wrote:
> From: MingWang Li <limingwang@huawei.com>
> 
> When testing the ARMv8.2-TTS2UXN feature, setting bits of XN is 
> unavailable.
> Because the control bit CTR_EL0.DIC is set by default on system.
> 
> But when CTR_EL0.DIC is set, software does not need to flush icache 
> actively,
> instead of clearing XN bits.The patch, the commit id of which
> is 6ae4b6e0578886eb36cedbf99f04031d93f9e315, has implemented the 
> function
> of CTR_EL0.DIC.
> 
> Signed-off-by: MingWang Li <limingwang@huawei.com>
> Signed-off-by: Henglong Fan <fanhenglong@huawei.com>
> ---
>  arch/arm64/include/asm/pgtable-prot.h | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable-prot.h
> b/arch/arm64/include/asm/pgtable-prot.h
> index 4d867c6446c4..5feb94882bf7 100644
> --- a/arch/arm64/include/asm/pgtable-prot.h
> +++ b/arch/arm64/include/asm/pgtable-prot.h
> @@ -79,17 +79,7 @@ extern bool arm64_use_ng_mappings;
>  		__val;							\
>  	 })
> 
> -#define PAGE_S2_XN							\
> -	({								\
> -		u64 __val;						\
> -		if (cpus_have_const_cap(ARM64_HAS_CACHE_DIC))		\
> -			__val = 0;					\
> -		else							\
> -			__val = PTE_S2_XN;				\
> -		__val;							\
> -	})
> -
> -#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) |
> PTE_S2_RDONLY | PAGE_S2_XN)
> +#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) |
> PTE_S2_RDONLY | PTE_S2_XN)
>  #define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT |
> PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
> 
>  #define PAGE_NONE		__pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) |
> PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN)

I don't understand what you are trying to achieve here.

This whole point of not setting XN in the page tables when DIC is 
present
is to avoid a pointless permission fault at run time. At you noticed
above, no icache invalidation is necessary. So why would you ever want
to take a fault on exec the first place?

         M.
-- 
Jazz is not dead. It just smells funny...