From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4D59FC43461 for ; Mon, 7 Sep 2020 11:40:18 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A41EB215A4 for ; Mon, 7 Sep 2020 11:40:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="HXtOdkVx"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="SRavKfhW" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A41EB215A4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Subject:To:From: Message-ID:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=lYl28rbV/60A4cktM8ZWBD6ZfdBnO8IMUGj4j4sGWLQ=; b=HXtOdkVxtXPykWkYMhctWHWV/ 4NuZvQjTtpR0svXZrabtDIOnl6eD27NJZqCK7b6B/w/nQB9FFdlL3rT/JVizgtBC7wQcSUGKhn37l VXP9wkwBRdNDHkZaHxB1RF6y5AfhsOFRIZ8A00AjnShYG9Rp55rToxI2uU49WY6HYPqRwjwbcjLaJ p3Kzt8SVm62smfpidPStPP1ZZpoaxe2ZWkCrADGfar7rmqzCgiJ9de6LT6oA3vBSaYlMtHXHmeDhF CjWuSZb2LgDGA+gUU4cRPZbhJLW0q/q4shS5xhKuKGh3wzcUNL9d+f0MsnpMJ/sLmuJE1JMdVCMHf jm+RqI4cQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFFU7-0001CV-2v; Mon, 07 Sep 2020 11:38:55 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFFU3-0001Bw-HF for linux-arm-kernel@lists.infradead.org; Mon, 07 Sep 2020 11:38:52 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 90750215A4; Mon, 7 Sep 2020 11:38:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599478730; bh=ZfnrZ4pbEQ/ixnxUU5AKEQ+NbLFvbri8Xgqh7CSvQ0Y=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=SRavKfhWIw+9o/GdPs0JxnkQIEE35czz3hkDl6FoUzDxgql/leWpYd2sFcxh28QbL PgOA+xdKcOUj5VenzMR8upOXTWXcu27J1g3+gHugZ6UEH7Ol8yXcyom1EJ72I7GhIJ 7HCI8xn8nCMOS4PclWERcDESEIyW98UbPipj+240= Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kFFU0-009kdw-GA; Mon, 07 Sep 2020 12:38:48 +0100 Date: Mon, 07 Sep 2020 12:38:46 +0100 Message-ID: <87y2lllsu1.wl-maz@kernel.org> From: Marc Zyngier To: Andrew Scull Subject: Re: [PATCH v3 06/18] KVM: arm64: nVHE: Use separate vector for the host In-Reply-To: <20200903135307.251331-7-ascull@google.com> References: <20200903135307.251331-1-ascull@google.com> <20200903135307.251331-7-ascull@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/26.3 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: ascull@google.com, kvmarm@lists.cs.columbia.edu, james.morse@arm.com, suzuki.poulose@arm.com, julien.thierry.kdev@gmail.com, will@kernel.org, catalin.marinas@arm.com, kernel-team@android.com, sudeep.holla@arm.com, linux-arm-kernel@lists.infradead.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200907_073851_711643_E2A60AF8 X-CRM114-Status: GOOD ( 42.45 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel-team@android.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, james.morse@arm.com, linux-arm-kernel@lists.infradead.org, Sudeep Holla , will@kernel.org, kvmarm@lists.cs.columbia.edu, julien.thierry.kdev@gmail.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Andrew, On Thu, 03 Sep 2020 14:52:55 +0100, Andrew Scull wrote: > > The host is treated differently from the guests when an exception is > taken so introduce a separate vector that is specialized for the host. > This also allows the nVHE specific code to move out of hyp-entry.S and > into nvhe/host.S. > > The host is only expected to make HVC calls and anything else is > considered invalid and results in a panic. > > Hyp initialization is now passed the vector that is used for the host > and it is swapped for the guest vector during the context switch. > > Signed-off-by: Andrew Scull > --- > arch/arm64/include/asm/kvm_asm.h | 2 + > arch/arm64/kernel/image-vars.h | 1 + > arch/arm64/kvm/arm.c | 11 +++- > arch/arm64/kvm/hyp/hyp-entry.S | 66 -------------------- > arch/arm64/kvm/hyp/nvhe/Makefile | 2 +- > arch/arm64/kvm/hyp/nvhe/host.S | 104 +++++++++++++++++++++++++++++++ > arch/arm64/kvm/hyp/nvhe/switch.c | 3 + > 7 files changed, 121 insertions(+), 68 deletions(-) > create mode 100644 arch/arm64/kvm/hyp/nvhe/host.S > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index 6f9c4162a764..34ec1b558219 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -98,10 +98,12 @@ struct kvm_vcpu; > struct kvm_s2_mmu; > > DECLARE_KVM_NVHE_SYM(__kvm_hyp_init); > +DECLARE_KVM_NVHE_SYM(__kvm_hyp_host_vector); > DECLARE_KVM_HYP_SYM(__kvm_hyp_vector); > > #ifndef __KVM_NVHE_HYPERVISOR__ > #define __kvm_hyp_init CHOOSE_NVHE_SYM(__kvm_hyp_init) > +#define __kvm_hyp_host_vector CHOOSE_NVHE_SYM(__kvm_hyp_host_vector) > #define __kvm_hyp_vector CHOOSE_HYP_SYM(__kvm_hyp_vector) > #endif > > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h > index 8982b68289b7..54bb0eb34b0f 100644 > --- a/arch/arm64/kernel/image-vars.h > +++ b/arch/arm64/kernel/image-vars.h > @@ -71,6 +71,7 @@ KVM_NVHE_ALIAS(kvm_update_va_mask); > /* Global kernel state accessed by nVHE hyp code. */ > KVM_NVHE_ALIAS(arm64_ssbd_callback_required); > KVM_NVHE_ALIAS(kvm_host_data); > +KVM_NVHE_ALIAS(kvm_hyp_vector); > KVM_NVHE_ALIAS(kvm_vgic_global_state); > > /* Kernel constant needed to compute idmap addresses. */ > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 77fc856ea513..b6442c6be5ad 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1277,7 +1277,7 @@ static void cpu_init_hyp_mode(void) > > pgd_ptr = kvm_mmu_get_httbr(); > hyp_stack_ptr = __this_cpu_read(kvm_arm_hyp_stack_page) + PAGE_SIZE; > - vector_ptr = __this_cpu_read(kvm_hyp_vector); > + vector_ptr = (unsigned long)kern_hyp_va(kvm_ksym_ref(__kvm_hyp_host_vector)); > > /* > * Call initialization code, and switch to the full blown HYP code. > @@ -1542,6 +1542,7 @@ static int init_hyp_mode(void) > > for_each_possible_cpu(cpu) { > struct kvm_host_data *cpu_data; > + unsigned long *vector; > > cpu_data = per_cpu_ptr(&kvm_host_data, cpu); > err = create_hyp_mappings(cpu_data, cpu_data + 1, PAGE_HYP); > @@ -1550,6 +1551,14 @@ static int init_hyp_mode(void) > kvm_err("Cannot map host CPU state: %d\n", err); > goto out_err; > } > + > + vector = per_cpu_ptr(&kvm_hyp_vector, cpu); > + err = create_hyp_mappings(vector, vector + 1, PAGE_HYP); > + > + if (err) { > + kvm_err("Cannot map hyp guest vector address\n"); > + goto out_err; > + } > } > > err = hyp_map_aux_data(); > diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S > index 9cb3fbca5d79..f92489250dfc 100644 > --- a/arch/arm64/kvm/hyp/hyp-entry.S > +++ b/arch/arm64/kvm/hyp/hyp-entry.S > @@ -12,7 +12,6 @@ > #include > #include > #include > -#include > #include > > .macro save_caller_saved_regs_vect > @@ -41,20 +40,6 @@ > > .text > > -.macro do_el2_call > - /* > - * Shuffle the parameters before calling the function > - * pointed to in x0. Assumes parameters in x[1,2,3]. > - */ > - str lr, [sp, #-16]! > - mov lr, x0 > - mov x0, x1 > - mov x1, x2 > - mov x2, x3 > - blr lr > - ldr lr, [sp], #16 > -.endm > - > el1_sync: // Guest trapped into EL2 > > mrs x0, esr_el2 > @@ -63,44 +48,6 @@ el1_sync: // Guest trapped into EL2 > ccmp x0, #ESR_ELx_EC_HVC32, #4, ne > b.ne el1_trap > > -#ifdef __KVM_NVHE_HYPERVISOR__ > - mrs x1, vttbr_el2 // If vttbr is valid, the guest > - cbnz x1, el1_hvc_guest // called HVC > - > - /* Here, we're pretty sure the host called HVC. */ > - ldp x0, x1, [sp], #16 > - > - /* Check for a stub HVC call */ > - cmp x0, #HVC_STUB_HCALL_NR > - b.hs 1f > - > - /* > - * Compute the idmap address of __kvm_handle_stub_hvc and > - * jump there. Since we use kimage_voffset, do not use the > - * HYP VA for __kvm_handle_stub_hvc, but the kernel VA instead > - * (by loading it from the constant pool). > - * > - * Preserve x0-x4, which may contain stub parameters. > - */ > - ldr x5, =__kvm_handle_stub_hvc > - ldr_l x6, kimage_voffset > - > - /* x5 = __pa(x5) */ > - sub x5, x5, x6 > - br x5 > - > -1: > - /* > - * Perform the EL2 call > - */ > - kern_hyp_va x0 > - do_el2_call > - > - eret > - sb > -#endif /* __KVM_NVHE_HYPERVISOR__ */ > - > -el1_hvc_guest: > /* > * Fastest possible path for ARM_SMCCC_ARCH_WORKAROUND_1. > * The workaround has already been applied on the host, > @@ -198,18 +145,6 @@ el2_error: > eret > sb > > -#ifdef __KVM_NVHE_HYPERVISOR__ > -SYM_FUNC_START(__hyp_do_panic) > - mov lr, #(PSR_F_BIT | PSR_I_BIT | PSR_A_BIT | PSR_D_BIT |\ > - PSR_MODE_EL1h) > - msr spsr_el2, lr > - ldr lr, =panic > - msr elr_el2, lr > - eret > - sb > -SYM_FUNC_END(__hyp_do_panic) > -#endif > - > .macro invalid_vector label, target = hyp_panic > .align 2 > SYM_CODE_START(\label) > @@ -222,7 +157,6 @@ SYM_CODE_END(\label) > invalid_vector el2t_irq_invalid > invalid_vector el2t_fiq_invalid > invalid_vector el2t_error_invalid > - invalid_vector el2h_sync_invalid > invalid_vector el2h_irq_invalid > invalid_vector el2h_fiq_invalid > invalid_vector el1_fiq_invalid > diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile > index aef76487edc2..ddf98eb07b9d 100644 > --- a/arch/arm64/kvm/hyp/nvhe/Makefile > +++ b/arch/arm64/kvm/hyp/nvhe/Makefile > @@ -6,7 +6,7 @@ > asflags-y := -D__KVM_NVHE_HYPERVISOR__ > ccflags-y := -D__KVM_NVHE_HYPERVISOR__ > > -obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o > +obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o > obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \ > ../fpsimd.o ../hyp-entry.o > > diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S > new file mode 100644 > index 000000000000..9c96b9a3b71d > --- /dev/null > +++ b/arch/arm64/kvm/hyp/nvhe/host.S > @@ -0,0 +1,104 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (C) 2020 - Google Inc > + * Author: Andrew Scull > + */ > + > +#include > + > +#include > +#include > +#include > + > + .text > + > +SYM_FUNC_START(__hyp_do_panic) > + mov lr, #(PSR_F_BIT | PSR_I_BIT | PSR_A_BIT | PSR_D_BIT |\ > + PSR_MODE_EL1h) > + msr spsr_el2, lr > + ldr lr, =panic > + msr elr_el2, lr > + eret > + sb > +SYM_FUNC_END(__hyp_do_panic) > + > +.macro valid_host_el1_sync_vect Do we actually need the "valid_" prefix? The invalid stuff is prefixed as invalid already. Something like el1_sync_host would be good enough IMHO. > + .align 7 > + esb > + stp x0, x1, [sp, #-16]! > + > + mrs x0, esr_el2 > + lsr x0, x0, #ESR_ELx_EC_SHIFT > + cmp x0, #ESR_ELx_EC_HVC64 > + b.ne hyp_panic > + > + ldp x0, x1, [sp], #16 You probably want to restore x0/x1 before branching to hyp_panic. At least your stack pointer will be correct, and x0/x1 may contain interesting stuff (not that it matters much at the moment, but I'm hopeful that at some point we will have a better panic handling). > + > + /* Check for a stub HVC call */ > + cmp x0, #HVC_STUB_HCALL_NR > + b.hs 1f > + > + /* > + * Compute the idmap address of __kvm_handle_stub_hvc and > + * jump there. Since we use kimage_voffset, do not use the > + * HYP VA for __kvm_handle_stub_hvc, but the kernel VA instead > + * (by loading it from the constant pool). > + * > + * Preserve x0-x4, which may contain stub parameters. > + */ > + ldr x5, =__kvm_handle_stub_hvc > + ldr_l x6, kimage_voffset > + > + /* x5 = __pa(x5) */ > + sub x5, x5, x6 > + br x5 > + > +1: > + /* > + * Shuffle the parameters before calling the function > + * pointed to in x0. Assumes parameters in x[1,2,3]. > + */ > + kern_hyp_va x0 > + str lr, [sp, #-16]! > + mov lr, x0 > + mov x0, x1 > + mov x1, x2 > + mov x2, x3 > + blr lr > + ldr lr, [sp], #16 > + > + eret > + sb Please add some checks to ensure that this macro never grows past 128 bytes, which is all you can fit in a single vector entry. Something like .org valid_host_el1_sync_vect + 0x80 should do the trick (the assembler will shout at you if you move the pointer backward in the case the macro becomes too long). > +.endm > + > +.macro invalid_host_vect > + .align 7 > + b hyp_panic > +.endm > + > +/* > + * CONFIG_KVM_INDIRECT_VECTORS is not applied to the host vector because the nit: s/vector/vectors/ > + * host already knows the address of hyp by virtue of loading it there. I find this comment a bit confusing (and I'm easily confused). How about: "... because the host knows about the EL2 vectors already, and there is no point in hiding them"? > + */ > + .align 11 > +SYM_CODE_START(__kvm_hyp_host_vector) > + invalid_host_vect // Synchronous EL2t > + invalid_host_vect // IRQ EL2t > + invalid_host_vect // FIQ EL2t > + invalid_host_vect // Error EL2t > + > + invalid_host_vect // Synchronous EL2h > + invalid_host_vect // IRQ EL2h > + invalid_host_vect // FIQ EL2h > + invalid_host_vect // Error EL2h > + > + valid_host_el1_sync_vect // Synchronous 64-bit EL1 > + invalid_host_vect // IRQ 64-bit EL1 > + invalid_host_vect // FIQ 64-bit EL1 > + invalid_host_vect // Error 64-bit EL1 > + > + invalid_host_vect // Synchronous 32-bit EL1 > + invalid_host_vect // IRQ 32-bit EL1 > + invalid_host_vect // FIQ 32-bit EL1 > + invalid_host_vect // Error 32-bit EL1 > +SYM_CODE_END(__kvm_hyp_host_vector) > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c > index 1e8a31b7c94c..1ab773bb60ca 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -42,6 +42,7 @@ static void __activate_traps(struct kvm_vcpu *vcpu) > } > > write_sysreg(val, cptr_el2); > + write_sysreg(__hyp_this_cpu_read(kvm_hyp_vector), vbar_el2); There is still the pending question of whether this requires extra synchronisation, but at least this becomes a problem common to both VHE and nVHE. > > if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) { > struct kvm_cpu_context *ctxt = &vcpu->arch.ctxt; > @@ -60,6 +61,7 @@ static void __activate_traps(struct kvm_vcpu *vcpu) > > static void __deactivate_traps(struct kvm_vcpu *vcpu) > { > + extern char __kvm_hyp_host_vector[]; > u64 mdcr_el2; > > ___deactivate_traps(vcpu); > @@ -91,6 +93,7 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu) > write_sysreg(mdcr_el2, mdcr_el2); > write_sysreg(HCR_HOST_NVHE_FLAGS, hcr_el2); > write_sysreg(CPTR_EL2_DEFAULT, cptr_el2); > + write_sysreg(__kvm_hyp_host_vector, vbar_el2); > } > > static void __load_host_stage2(void) > -- > 2.28.0.402.g5ffc5be6b7-goog > > Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel