Received: by 2002:a25:5b86:0:0:0:0:0 with SMTP id p128csp772372ybb; Thu, 28 Mar 2019 11:52:59 -0700 (PDT) X-Google-Smtp-Source: APXvYqxX9FieGNPUUy/21Z3MZrkcHmmJIfLABdH3gU8xBUvrW8TR9jxaTWaUZy/kUJp7HIFIBGdU X-Received: by 2002:a63:5858:: with SMTP id i24mr41137016pgm.222.1553799178937; Thu, 28 Mar 2019 11:52:58 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1553799178; cv=none; d=google.com; s=arc-20160816; b=e1wMwSo9vOr4QiFiaim85KY79cvh1X3h/cQuTKVivTtWYeeV7SNjsiOpLM1+fP2OAz cFo+LrUuypUsrOMtGYjdIHIDIh0KF1DtB7jWCOVJs3hSuIND23D0Eg/ChAqvH6uzWVEg XBZOsFTLIbq2J0ObCIm+GqD9nEhNrg8JwGb70Mmr+XSJ81cVunRVAIjuYrmeZx2V2yP/ 5tchfAoV71cnZ72y2REM9cjjd/lLIotMzXq/olFpRSmD9flQXQyyu3GIiK4y7pIFVQTo COjHN+wi3Eevzyh3CuMT7pDVB/mcxpM+mu1e22I4O94xtxpOqhFBLROU/Dk7tR7uJary R5zQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=L9RLxneg7W+DyF5wSPU5fYQni4xtF33By/yQPHI9Sgk=; b=cEqQMfosbXuXW8KMXEybolC5NE0tOFWu3MhMC3m/FOaKrDwu++IzOb90SbySVlas3/ HX1rD/LBCSE4WdEqNqg8q0HnADi2JtR9KOYs1gM/kXfiHVXTqQ6HXYVdK7eVeomG7/Lb KaY7/7OlzjxgcIyf0m5rSEHb8SRCP1+u73uRGfnfcwLw1mM+vM4hmRl3/Xe2rXQf1NVM Fc5sfNFyjWEVuu1J4y40HozzWWUql0C3vty+jE0+j1Y4C2F+Ou7X7zPD55QXb8tLxLR0 Tgj/yX7pU1r6uOXTv0X8XnppKEM5/vF5crAh26pj/QCesxAWcrbnCNoAZ0K3AvOtyRX3 YSFQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 65si23085310plf.288.2019.03.28.11.52.43; Thu, 28 Mar 2019 11:52:58 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726279AbfC1Sva (ORCPT + 99 others); Thu, 28 Mar 2019 14:51:30 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:50052 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726047AbfC1Sva (ORCPT ); Thu, 28 Mar 2019 14:51:30 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4973415AB; Thu, 28 Mar 2019 11:51:29 -0700 (PDT) Received: from [10.1.196.105] (eglon.cambridge.arm.com [10.1.196.105]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 052393F614; Thu, 28 Mar 2019 11:51:26 -0700 (PDT) Subject: Re: [PATCH v7 7/10] KVM: arm/arm64: context-switch ptrauth registers To: Amit Daniel Kachhap , linux-arm-kernel@lists.infradead.org Cc: Christoffer Dall , Marc Zyngier , Catalin Marinas , Will Deacon , Andrew Jones , Dave Martin , Ramana Radhakrishnan , kvmarm@lists.cs.columbia.edu, Kristina Martsenko , linux-kernel@vger.kernel.org, Mark Rutland , Julien Thierry References: <1552984243-7689-1-git-send-email-amit.kachhap@arm.com> <1552984243-7689-8-git-send-email-amit.kachhap@arm.com> From: James Morse Message-ID: <9d929db6-2a24-b356-6c76-8f58341db4bc@arm.com> Date: Thu, 28 Mar 2019 18:51:25 +0000 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <1552984243-7689-8-git-send-email-amit.kachhap@arm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Amit, On 19/03/2019 08:30, Amit Daniel Kachhap wrote: > From: Mark Rutland > > When pointer authentication is supported, a guest may wish to use it. > This patch adds the necessary KVM infrastructure for this to work, with > a semi-lazy context switch of the pointer auth state. > > Pointer authentication feature is only enabled when VHE is built > in the kernel and present in the CPU implementation so only VHE code > paths are modified. > > When we schedule a vcpu, we disable guest usage of pointer > authentication instructions and accesses to the keys. While these are > disabled, we avoid context-switching the keys. When we trap the guest > trying to use pointer authentication functionality, we change to eagerly > context-switching the keys, and enable the feature. The next time the > vcpu is scheduled out/in, we start again. However the host key save is > optimized and implemented inside ptrauth instruction/register access > trap. > > Pointer authentication consists of address authentication and generic > authentication, and CPUs in a system might have varied support for > either. Where support for either feature is not uniform, it is hidden > from guests via ID register emulation, as a result of the cpufeature > framework in the host. > > Unfortunately, address authentication and generic authentication cannot > be trapped separately, as the architecture provides a single EL2 trap > covering both. If we wish to expose one without the other, we cannot > prevent a (badly-written) guest from intermittently using a feature > which is not uniformly supported (when scheduled on a physical CPU which > supports the relevant feature). Hence, this patch expects both type of > authentication to be present in a cpu. > > This switch of key is done from guest enter/exit assembly as preperation > for the upcoming in-kernel pointer authentication support. Hence, these > key switching routines are not implemented in C code as they may cause > pointer authentication key signing error in some situations. > diff --git a/arch/arm64/include/asm/kvm_ptrauth_asm.h b/arch/arm64/include/asm/kvm_ptrauth_asm.h > new file mode 100644 > index 0000000..97bb040 > --- /dev/null > +++ b/arch/arm64/include/asm/kvm_ptrauth_asm.h > +.macro ptrauth_save_state base, reg1, reg2 > + mrs_s \reg1, SYS_APIAKEYLO_EL1 > + mrs_s \reg2, SYS_APIAKEYHI_EL1 > + stp \reg1, \reg2, [\base, #PTRAUTH_REG_OFFSET(CPU_APIAKEYLO_EL1)] A nice-to-have: Because you only use the 'LO' asm-offset definitions here, we have to just assume that the 'HI' ones are adjacent. Is it possible to add a BUILD_BUG() somewhere (probably in asm-offsets.c) to check this? As its just an enum we'd expect the order to be arbitrary, and asm-offsets to take care of any re-ordering... (struct randomisation may one day come to enums!) > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index e2f0268..9f591ad 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -544,3 +544,17 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > > return ret; > } > + > +/** > + * kvm_arm_vcpu_ptrauth_setup_lazy - setup lazy ptrauth for vcpu schedule > + * > + * @vcpu: The VCPU pointer > + * > + * This function may be used to disable ptrauth and use it in a lazy context > + * via traps. > + */ > +void kvm_arm_vcpu_ptrauth_setup_lazy(struct kvm_vcpu *vcpu) > +{ > + if (vcpu_has_ptrauth(vcpu)) > + kvm_arm_vcpu_ptrauth_disable(vcpu); > +} Can you check my reasoning here, to make sure I've understood this properly?!: This clears the API/APK bits to enable the traps, if the system supports ptrauth, and if Qemu requested it for this vcpu. If any of those things aren't true, the guest gets HCR_GUEST_FLAGS, which also has the API/APK bits clear... What this is doing is clearing the API/APK bits that may have been left set by a previous run of a ptrauth-enabled vcpu. > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c > index f16a5f8..00f0639 100644 > --- a/arch/arm64/kvm/reset.c > +++ b/arch/arm64/kvm/reset.c > @@ -128,6 +128,13 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu) > if (loaded) > kvm_arch_vcpu_put(vcpu); > > + if (test_bit(KVM_ARM_VCPU_PTRAUTH_ADDRESS, vcpu->arch.features) || > + test_bit(KVM_ARM_VCPU_PTRAUTH_GENERIC, vcpu->arch.features)) { > + /* Verify that KVM startup matches the conditions for ptrauth */ > + if (WARN_ON(!vcpu_has_ptrauth(vcpu))) > + return -EINVAL; > + } Could this hunk go in the previous patch that added the vcpu definitions? It would be good if the uapi defines, the documentation and this validation logic came as one patch, it makes future archaeology easier! Looks good! Thanks, James