From: James Morse <james.morse@arm.com>
To: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
linux-kernel@vger.kernel.org,
Kristina Martsenko <kristina.martsenko@arm.com>,
Dave Martin <Dave.Martin@arm.com>,
Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 1/6] arm64/kvm: preserve host HCR_EL2 value
Date: Fri, 4 Jan 2019 17:50:38 +0000 [thread overview]
Message-ID: <47a81c5b-1b1b-2c87-8c90-af5ef63d1364@arm.com> (raw)
In-Reply-To: <1545119810-12182-2-git-send-email-amit.kachhap@arm.com>
Hi Amit,
On 18/12/2018 07:56, Amit Daniel Kachhap wrote:
> When restoring HCR_EL2 for the host, KVM uses HCR_HOST_VHE_FLAGS, which
> is a constant value. This works today, as the host HCR_EL2 value is
> always the same, but this will get in the way of supporting extensions
> that require HCR_EL2 bits to be set conditionally for the host.
>
> To allow such features to work without KVM having to explicitly handle
> every possible host feature combination, this patch has KVM save/restore
> the host HCR when switching to/from a guest HCR. The saving of the
> register is done once during cpu hypervisor initialization state and is
> just restored after switch from guest.
>
> For fetching HCR_EL2 during kvm initilisation, a hyp call is made using
(initialisation)
> kvm_call_hyp and is helpful in NHVE case.
>
> For the hyp TLB maintenance code, __tlb_switch_to_host_vhe() is updated
> to toggle the TGE bit with a RMW sequence, as we already do in
> __tlb_switch_to_guest_vhe().
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index aea01a0..25ac9fa 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -73,6 +73,8 @@ extern void __vgic_v3_init_lrs(void);
>
> extern u32 __kvm_get_mdcr_el2(void);
>
> +extern u64 __read_hyp_hcr_el2(void);
How come this isn't __kvm_get_hcr_el2() like mdcr?
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 52fbc82..1b9eed9 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -196,13 +196,17 @@ enum vcpu_sysreg {
>
> #define NR_COPRO_REGS (NR_SYS_REGS * 2)
>
> +struct kvm_cpu_init_host_regs {
> + u64 hcr_el2;
> +};
> +
> struct kvm_cpu_context {
> struct kvm_regs gp_regs;
> union {
> u64 sys_regs[NR_SYS_REGS];
> u32 copro[NR_COPRO_REGS];
> };
> -
> + struct kvm_cpu_init_host_regs init_regs;
> struct kvm_vcpu *__hyp_running_vcpu;
> };
Hmm, so we grow every vcpu's struct kvm_cpu_context with some host-only registers...
> @@ -211,7 +215,7 @@ typedef struct kvm_cpu_context kvm_cpu_context_t;
> struct kvm_vcpu_arch {
> struct kvm_cpu_context ctxt;
>
> - /* HYP configuration */
> + /* Guest HYP configuration */
> u64 hcr_el2;
> u32 mdcr_el2;
... but they aren't actually host-only.
I think it would be tidier to move these two into struct kvm_cpu_context (not as
some init_host state), as both host and vcpu's have these values.
You could then add the mdcr_el2 stashing to your __cpu_copy_host_registers()
too. This way they both work in the same way, otherwise one is per-cpu, the
other is in a special bit of only the host's kvm_cpu_context.
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index f6e02cc..85a2a5c 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -139,15 +139,15 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
> __activate_traps_nvhe(vcpu);
> }
>
> -static void deactivate_traps_vhe(void)
> +static void deactivate_traps_vhe(struct kvm_cpu_context *host_ctxt)
> {
> extern char vectors[]; /* kernel exception vectors */
> - write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> + write_sysreg(host_ctxt->init_regs.hcr_el2, hcr_el2);
> write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
> write_sysreg(vectors, vbar_el1);
> }
>
> -static void __hyp_text __deactivate_traps_nvhe(void)
> +static void __hyp_text __deactivate_traps_nvhe(struct kvm_cpu_context *host_ctxt)
> {
> u64 mdcr_el2 = read_sysreg(mdcr_el2);
>
> @@ -157,12 +157,15 @@ static void __hyp_text __deactivate_traps_nvhe(void)
> mdcr_el2 |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT;
>
> write_sysreg(mdcr_el2, mdcr_el2);
Strangely we try to rebuild the host's mdcr value here. If we had the host mdcr
value in host_ctxt we could restore it directly.
> - write_sysreg(HCR_HOST_NVHE_FLAGS, hcr_el2);
> + write_sysreg(host_ctxt->init_regs.hcr_el2, hcr_el2);
> write_sysreg(CPTR_EL2_DEFAULT, cptr_el2);
> }
> static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu)
> {
> + struct kvm_cpu_context *host_ctxt;
> +
> + host_ctxt = vcpu->arch.host_cpu_context;
> /*
> * If we pended a virtual abort, preserve it until it gets
> * cleared. See D1.14.3 (Virtual Interrupts) for details, but
> @@ -173,9 +176,9 @@ static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu)
> vcpu->arch.hcr_el2 = read_sysreg(hcr_el2);
>
> if (has_vhe())
> - deactivate_traps_vhe();
> + deactivate_traps_vhe(host_ctxt);
> else
> - __deactivate_traps_nvhe();
> + __deactivate_traps_nvhe(host_ctxt);
> }
(Alternatively each of these deactivate_traps() calls could retrieve the
host_ctxt directly as its a per-cpu variable, but as we have the struct vcpu
here, this is probably better.)
Thanks,
James
_______________________________________________
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-01-04 18:37 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-18 7:56 [PATCH v4 0/6] Add ARMv8.3 pointer authentication for kvm guest Amit Daniel Kachhap
2018-12-18 7:56 ` [PATCH v4 1/6] arm64/kvm: preserve host HCR_EL2 value Amit Daniel Kachhap
2019-01-04 17:50 ` James Morse [this message]
2019-01-08 5:16 ` Amit Daniel Kachhap
2018-12-18 7:56 ` [PATCH v4 2/6] arm64/kvm: context-switch ptrauth registers Amit Daniel Kachhap
2019-01-04 17:57 ` James Morse
2019-01-09 10:01 ` Amit Daniel Kachhap
2018-12-18 7:56 ` [PATCH v4 3/6] arm64/kvm: add a userspace option to enable pointer authentication Amit Daniel Kachhap
2019-01-04 17:57 ` James Morse
2019-01-09 10:13 ` Amit Daniel Kachhap
2019-01-31 16:19 ` James Morse
2018-12-18 7:56 ` [PATCH v4 4/6] arm64/kvm: enable pointer authentication cpufeature conditionally Amit Daniel Kachhap
2019-01-04 17:58 ` James Morse
2019-01-09 10:16 ` Amit Daniel Kachhap
2019-01-28 7:02 ` Amit Daniel Kachhap
2018-12-18 7:56 ` [PATCH v4 5/6] arm64/kvm: control accessibility of ptrauth key registers Amit Daniel Kachhap
2018-12-18 7:56 ` [PATCH v4 6/6] arm/kvm: arm64: Add a vcpu feature for pointer authentication Amit Daniel Kachhap
2019-01-04 17:59 ` 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=47a81c5b-1b1b-2c87-8c90-af5ef63d1364@arm.com \
--to=james.morse@arm.com \
--cc=Dave.Martin@arm.com \
--cc=amit.kachhap@arm.com \
--cc=catalin.marinas@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=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).