From: Amit Daniel Kachhap <amit.kachhap@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Jones <drjones@redhat.com>,
linux-kernel@vger.kernel.org,
Julien Thierry <julien.thierry@arm.com>,
Marc Zyngier <marc.zyngier@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
Christoffer Dall <christoffer.dall@arm.com>,
Kristina Martsenko <kristina.martsenko@arm.com>,
kvmarm@lists.cs.columbia.edu, James Morse <james.morse@arm.com>,
Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
Dave Martin <Dave.Martin@arm.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 3/6] arm64/kvm: context-switch ptrauth registers
Date: Thu, 28 Feb 2019 14:37:56 +0530 [thread overview]
Message-ID: <04f19012-3a18-60af-b43f-dc52139633bd@arm.com> (raw)
In-Reply-To: <20190221122942.GC33673@lakrids.cambridge.arm.com>
Hi Mark,
On 2/21/19 5:59 PM, Mark Rutland wrote:
> On Tue, Feb 19, 2019 at 02:54:28PM +0530, Amit Daniel Kachhap wrote:
>> From: Mark Rutland <mark.rutland@arm.com>
>>
>> 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 into CPU implementation so only VHE code
>> paths are modified.
>
> Nit: s/into/in the/
ok.
>
>>
>> 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 registers
>> are saved in vcpu load stage as they remain constant for each vcpu
>> schedule.
>>
>> 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.
>>
>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>> [Only VHE, key switch from from assembly, kvm_supports_ptrauth
>> checks, save host key in vcpu_load]
>
> Hmm, why do we need to do the key switch in assembly, given it's not
> used in-kernel right now?
>
> Is that in preparation for in-kernel pointer auth usage? If so, please
> call that out in the commit message.
ok sure.
>
> [...]
>
>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>> index 4e2fb87..5cac605 100644
>> --- a/arch/arm64/kernel/traps.c
>> +++ b/arch/arm64/kernel/traps.c
>> @@ -749,6 +749,7 @@ static const char *esr_class_str[] = {
>> [ESR_ELx_EC_CP14_LS] = "CP14 LDC/STC",
>> [ESR_ELx_EC_FP_ASIMD] = "ASIMD",
>> [ESR_ELx_EC_CP10_ID] = "CP10 MRC/VMRS",
>> + [ESR_ELx_EC_PAC] = "Pointer authentication trap",
>
> For consistency with the other strings, can we please make this "PAC"?
ok. It makes sense.
>
> [...]
>
>> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
>> index 82d1904..17cec99 100644
>> --- a/arch/arm64/kvm/hyp/Makefile
>> +++ b/arch/arm64/kvm/hyp/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_KVM_ARM_HOST) += switch.o
>> obj-$(CONFIG_KVM_ARM_HOST) += fpsimd.o
>> obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
>> obj-$(CONFIG_KVM_ARM_HOST) += hyp-entry.o
>> +obj-$(CONFIG_KVM_ARM_HOST) += ptrauth-sr.o
>
> Huh, so we're actually doing the switch in C code...
>
>> # KVM code is run at a different exception code with a different map, so
>> # compiler instrumentation that inserts callbacks or checks into the code may
>> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
>> index 675fdc1..b78cc15 100644
>> --- a/arch/arm64/kvm/hyp/entry.S
>> +++ b/arch/arm64/kvm/hyp/entry.S
>> @@ -64,6 +64,12 @@ ENTRY(__guest_enter)
>>
>> add x18, x0, #VCPU_CONTEXT
>>
>> +#ifdef CONFIG_ARM64_PTR_AUTH
>> + // Prepare parameter for __ptrauth_switch_to_guest(vcpu, host, guest).
>> + mov x2, x18
>> + bl __ptrauth_switch_to_guest
>> +#endif
>
> ... and conditionally *calling* that switch code from assembly ...
>
>> +
>> // Restore guest regs x0-x17
>> ldp x0, x1, [x18, #CPU_XREG_OFFSET(0)]
>> ldp x2, x3, [x18, #CPU_XREG_OFFSET(2)]
>> @@ -118,6 +124,17 @@ ENTRY(__guest_exit)
>>
>> get_host_ctxt x2, x3
>>
>> +#ifdef CONFIG_ARM64_PTR_AUTH
>> + // Prepare parameter for __ptrauth_switch_to_host(vcpu, guest, host).
>> + // Save x0, x2 which are used later in callee saved registers.
>> + mov x19, x0
>> + mov x20, x2
>> + sub x0, x1, #VCPU_CONTEXT
>> + ldr x29, [x2, #CPU_XREG_OFFSET(29)]
>> + bl __ptrauth_switch_to_host
>> + mov x0, x19
>> + mov x2, x20
>> +#endif
>
> ... which adds a load of boilerplate for no immediate gain.
Here some parameter optimizations may be possible as guest and host ctxt
can be derived from vcpu itself as James suggested in other review
comments. I thought about doing all save/restore in assembly but for
optimization now host keys are saved in vcpu_load stage in C so reused
those C codes here also.
Again all these codes are beneficial with in-kernel ptrauth and hence in
case of strong objection may revert to old way.
>
> Do we really need to do this in assembly today?
During the last patchset review [1], James provided a lot of supporting
arguments to have these switch routines called from assembly due to
function outlining between kvm_vcpu_run_vhe() and __kvm_vcpu_run_nvhe().
[1]: https://lkml.org/lkml/2019/1/31/662
Thanks,
Amit D
>
> Thanks,
> Mark.
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-02-28 9:08 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-19 9:24 [PATCH v6 0/6] Add ARMv8.3 pointer authentication for kvm guest Amit Daniel Kachhap
2019-02-19 9:24 ` [PATCH v6 1/6] arm64/kvm: preserve host HCR_EL2 value Amit Daniel Kachhap
2019-02-21 11:50 ` Mark Rutland
2019-02-25 18:09 ` Marc Zyngier
2019-02-28 6:43 ` Amit Daniel Kachhap
2019-02-21 15:49 ` Dave Martin
2019-03-01 5:56 ` Amit Daniel Kachhap
2019-02-25 17:39 ` James Morse
2019-02-26 10:06 ` James Morse
2019-03-02 11:09 ` Amit Daniel Kachhap
2019-02-19 9:24 ` [PATCH v6 2/6] arm64/kvm: preserve host MDCR_EL2 value Amit Daniel Kachhap
2019-02-21 11:57 ` Mark Rutland
2019-02-21 15:51 ` Dave Martin
2019-03-01 6:10 ` Amit Daniel Kachhap
2019-02-19 9:24 ` [PATCH v6 3/6] arm64/kvm: context-switch ptrauth registers Amit Daniel Kachhap
2019-02-21 12:29 ` Mark Rutland
2019-02-21 15:51 ` Dave Martin
2019-03-01 6:17 ` Amit Daniel Kachhap
2019-02-28 9:07 ` Amit Daniel Kachhap [this message]
2019-02-21 15:53 ` Dave Martin
2019-03-01 9:35 ` Amit Daniel Kachhap
2019-02-26 18:31 ` James Morse
2019-03-04 10:51 ` Amit Daniel Kachhap
2019-02-19 9:24 ` [PATCH v6 4/6] arm64/kvm: add a userspace option to enable pointer authentication Amit Daniel Kachhap
2019-02-21 12:34 ` Mark Rutland
2019-02-28 9:25 ` Amit Daniel Kachhap
2019-02-21 15:53 ` Dave Martin
2019-03-01 9:41 ` Amit Daniel Kachhap
2019-03-01 12:22 ` Dave P Martin
2019-02-26 18:33 ` James Morse
2019-03-04 10:56 ` Amit Daniel Kachhap
2019-02-19 9:24 ` [PATCH v6 5/6] arm64/kvm: control accessibility of ptrauth key registers Amit Daniel Kachhap
2019-02-21 15:53 ` Dave Martin
2019-02-26 18:34 ` James Morse
2019-02-19 9:24 ` [kvmtool PATCH v6 6/6] arm/kvm: arm64: Add a vcpu feature for pointer authentication Amit Daniel Kachhap
2019-02-21 15:54 ` Dave Martin
2019-03-01 10:37 ` Amit Daniel Kachhap
2019-03-01 11:24 ` Dave P Martin
2019-03-04 11:08 ` Amit Daniel Kachhap
2019-03-05 11:11 ` Dave Martin
2019-02-26 18:03 ` [PATCH v6 0/6] Add ARMv8.3 pointer authentication for kvm guest James Morse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=04f19012-3a18-60af-b43f-dc52139633bd@arm.com \
--to=amit.kachhap@arm.com \
--cc=Dave.Martin@arm.com \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@arm.com \
--cc=drjones@redhat.com \
--cc=james.morse@arm.com \
--cc=julien.thierry@arm.com \
--cc=kristina.martsenko@arm.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=mark.rutland@arm.com \
--cc=ramana.radhakrishnan@arm.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).