From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932573AbcBANjC (ORCPT ); Mon, 1 Feb 2016 08:39:02 -0500 Received: from foss.arm.com ([217.140.101.70]:48428 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932183AbcBANi7 (ORCPT ); Mon, 1 Feb 2016 08:38:59 -0500 Subject: Re: [PATCH v2 07/21] arm64: KVM: VHE: Patch out kern_hyp_va To: Christoffer Dall References: <1453737235-16522-1-git-send-email-marc.zyngier@arm.com> <1453737235-16522-8-git-send-email-marc.zyngier@arm.com> <20160201132042.GJ1478@cbox> Cc: Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu From: Marc Zyngier Organization: ARM Ltd Message-ID: <56AF5FF1.4010701@arm.com> Date: Mon, 1 Feb 2016 13:38:57 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 MIME-Version: 1.0 In-Reply-To: <20160201132042.GJ1478@cbox> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/02/16 13:20, Christoffer Dall wrote: > On Mon, Jan 25, 2016 at 03:53:41PM +0000, Marc Zyngier wrote: >> The kern_hyp_va macro is pretty meaninless with VHE, as there is >> only one mapping - the kernel one. >> >> In order to keep the code readable and efficient, use runtime >> patching to replace the 'and' instruction used to compute the VA >> with a 'nop'. >> >> Signed-off-by: Marc Zyngier >> --- >> arch/arm64/include/asm/kvm_mmu.h | 11 ++++++++++- >> arch/arm64/kvm/hyp/hyp.h | 25 ++++++++++++++++++++++--- >> 2 files changed, 32 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h >> index d3e6d7b..62f0d14 100644 >> --- a/arch/arm64/include/asm/kvm_mmu.h >> +++ b/arch/arm64/include/asm/kvm_mmu.h >> @@ -23,13 +23,16 @@ >> #include >> >> /* >> - * As we only have the TTBR0_EL2 register, we cannot express >> + * As ARMv8.0 only has the TTBR0_EL2 register, we cannot express >> * "negative" addresses. This makes it impossible to directly share >> * mappings with the kernel. >> * >> * Instead, give the HYP mode its own VA region at a fixed offset from >> * the kernel by just masking the top bits (which are all ones for a >> * kernel address). >> + * >> + * ARMv8.1 (using VHE) does have a TTBR1_EL2, and doesn't use these >> + * macros (the entire kernel runs at EL2). >> */ >> #define HYP_PAGE_OFFSET_SHIFT VA_BITS >> #define HYP_PAGE_OFFSET_MASK ((UL(1) << HYP_PAGE_OFFSET_SHIFT) - 1) >> @@ -56,6 +59,8 @@ >> >> #ifdef __ASSEMBLY__ >> >> +#include >> +#include >> #include >> >> .macro setup_vtcr tmp1, tmp2 >> @@ -84,7 +89,11 @@ >> * reg: VA to be converted. >> */ >> .macro kern_hyp_va reg >> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN >> and \reg, \reg, #HYP_PAGE_OFFSET_MASK >> +alternative_else >> + nop >> +alternative_endif >> .endm >> >> #else >> diff --git a/arch/arm64/kvm/hyp/hyp.h b/arch/arm64/kvm/hyp/hyp.h >> index fb27517..fc502f3 100644 >> --- a/arch/arm64/kvm/hyp/hyp.h >> +++ b/arch/arm64/kvm/hyp/hyp.h >> @@ -25,9 +25,28 @@ >> >> #define __hyp_text __section(.hyp.text) notrace >> >> -#define kern_hyp_va(v) (typeof(v))((unsigned long)(v) & HYP_PAGE_OFFSET_MASK) >> -#define hyp_kern_va(v) (typeof(v))((unsigned long)(v) - HYP_PAGE_OFFSET \ >> - + PAGE_OFFSET) >> +static inline unsigned long __kern_hyp_va(unsigned long v) >> +{ >> + asm volatile(ALTERNATIVE("and %0, %0, %1", >> + "nop", >> + ARM64_HAS_VIRT_HOST_EXTN) >> + : "+r" (v) : "i" (HYP_PAGE_OFFSET_MASK)); >> + return v; >> +} >> + >> +#define kern_hyp_va(v) (typeof(v))(__kern_hyp_va((unsigned long)(v))) >> + >> +static inline unsigned long __hyp_kern_va(unsigned long v) >> +{ >> + u64 offset = PAGE_OFFSET - HYP_PAGE_OFFSET; >> + asm volatile(ALTERNATIVE("add %0, %0, %1", >> + "nop", >> + ARM64_HAS_VIRT_HOST_EXTN) >> + : "+r" (v) : "r" (offset)); >> + return v; >> +} >> + >> +#define hyp_kern_va(v) (typeof(v))(__hyp_kern_va((unsigned long)(v))) > > why do we need this casting of values instead of just defining these > inlines and calling them directly with proper typing? We commonly pass both pointers and unsigned long to this helper. Do you really want a separate helper for each type instead of one that does it all? Thanks, M. -- Jazz is not dead. It just smells funny...