From: Marc Zyngier <maz@kernel.org>
To: Andrew Scull <ascull@google.com>
Cc: kernel-team@android.com, kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH 06/37] KVM: arm64: Only check pending interrupts if it would trap
Date: Fri, 17 Jul 2020 17:21:42 +0100 [thread overview]
Message-ID: <87blkexgbt.wl-maz@kernel.org> (raw)
In-Reply-To: <20200715184438.1390996-7-ascull@google.com>
Hi Andrew,
On Wed, 15 Jul 2020 19:44:07 +0100,
Andrew Scull <ascull@google.com> wrote:
>
> Allow entry to a vcpu that can handle interrupts if there is an
> interrupts pending. Entry will still be aborted if the vcpu cannot
> handle interrupts.
This is pretty confusing. All vcpus can handle interrupts, it's just
that there are multiple classes of interrupts (physical or virtual).
Instead, this should outline *where* physical interrupt are taken.
Something like:
When entering a vcpu for which physical interrupts are not taken to
EL2, don't bother evaluating ISR_EL1 to work out whether we should
go back to EL2 early. Instead, just enter the guest without any
further ado. This is done by checking HCR_EL2.IMO bit.
>
> This allows use of __guest_enter to enter into the host.
>
> Signed-off-by: Andrew Scull <ascull@google.com>
> ---
> arch/arm64/kvm/hyp/entry.S | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index ee32a7743389..6a641fcff4f7 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -73,13 +73,17 @@ SYM_FUNC_START(__guest_enter)
> save_sp_el0 x1, x2
>
> // Now the host state is stored if we have a pending RAS SError it must
> - // affect the host. If any asynchronous exception is pending we defer
> - // the guest entry. The DSB isn't necessary before v8.2 as any SError
> - // would be fatal.
> + // affect the host. If physical IRQ interrupts are going to be trapped
> + // and there are already asynchronous exceptions pending then we defer
> + // the entry. The DSB isn't necessary before v8.2 as any SError would
> + // be fatal.
> alternative_if ARM64_HAS_RAS_EXTN
> dsb nshst
> isb
> alternative_else_nop_endif
> + mrs x1, hcr_el2
> + and x1, x1, #HCR_IMO
> + cbz x1, 1f
Do we really want to take the overhead of the above DSB/ISB when on
the host? We're not even evaluating ISR_EL1, so what is the gain?
This also assumes that IMO/FMO/AMO are all set together, which
deserves to be documented.
Another thing is that you are also restoring registers that the host
vcpu expects to be corrupted (the caller-saved registers, X0-17).
You probably should just zero them instead if leaking data from EL2 is
your concern. Yes, this is a departure from SMCCC 1.1, but I think
this is a valid one, as EL2 isn't a fully independent piece of SW.
Same goes on the __guest_exit() path. PtrAuth is another concern (I'm
pretty sure this doesn't do what we want, but I haven't tried on a model).
I've hacked the following patch together, which allowed me to claw
back about 10% of the performance loss. I'm pretty sure there are
similar places where you have introduced extra overhead, and we should
hunt them down.
Thanks,
M.
diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 6c3a6b27a96c..2d1a71bd7baa 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -33,6 +33,10 @@ SYM_FUNC_START(__guest_enter)
// Save the hyp's sp_el0
save_sp_el0 x1, x2
+ mrs x1, hcr_el2
+ and x1, x1, #HCR_IMO
+ cbz x1, 2f
+
// Now the hyp state is stored if we have a pending RAS SError it must
// affect the hyp. If physical IRQ interrupts are going to be trapped
// and there are already asynchronous exceptions pending then we defer
@@ -42,9 +46,6 @@ alternative_if ARM64_HAS_RAS_EXTN
dsb nshst
isb
alternative_else_nop_endif
- mrs x1, hcr_el2
- and x1, x1, #HCR_IMO
- cbz x1, 1f
mrs x1, isr_el1
cbz x1, 1f
mov x0, #ARM_EXCEPTION_IRQ
@@ -81,6 +82,31 @@ alternative_else_nop_endif
eret
sb
+2:
+ add x29, x0, #VCPU_CONTEXT
+
+ // Macro ptrauth_switch_to_guest format:
+ // ptrauth_switch_to_guest(guest cxt, tmp1, tmp2, tmp3)
+ // The below macro to restore guest keys is not implemented in C code
+ // as it may cause Pointer Authentication key signing mismatch errors
+ // when this feature is enabled for kernel code.
+ ptrauth_switch_to_guest x29, x0, x1, x2
+
+ // Restore the guest's sp_el0
+ restore_sp_el0 x29, x0
+
+ .irp n,4,5,6,7,8,9,10,11,12,13,14,15,16,17
+ mov x\n, xzr
+ .endr
+
+ ldp x0, x1, [x29, #CPU_XREG_OFFSET(0)]
+ ldp x2, x3, [x29, #CPU_XREG_OFFSET(2)]
+
+ // Restore guest regs x18-x29, lr
+ restore_callee_saved_regs x29
+ eret
+ sb
+
SYM_INNER_LABEL(__guest_exit, SYM_L_GLOBAL)
// x0: return code
// x1: vcpu
@@ -99,6 +125,11 @@ SYM_INNER_LABEL(__guest_exit, SYM_L_GLOBAL)
// Store the guest regs x0-x1 and x4-x17
stp x2, x3, [x1, #CPU_XREG_OFFSET(0)]
+
+ mrs x2, hcr_el2
+ and x2, x2, #HCR_IMO
+ cbz x2, 1f
+
stp x4, x5, [x1, #CPU_XREG_OFFSET(4)]
stp x6, x7, [x1, #CPU_XREG_OFFSET(6)]
stp x8, x9, [x1, #CPU_XREG_OFFSET(8)]
@@ -107,6 +138,7 @@ SYM_INNER_LABEL(__guest_exit, SYM_L_GLOBAL)
stp x14, x15, [x1, #CPU_XREG_OFFSET(14)]
stp x16, x17, [x1, #CPU_XREG_OFFSET(16)]
+1:
// Store the guest regs x18-x29, lr
save_callee_saved_regs x1
--
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
next prev parent reply other threads:[~2020-07-17 16:21 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-15 18:44 [PATCH 00/37] Transform the host into a vCPU Andrew Scull
2020-07-15 18:44 ` [PATCH 01/37] smccc: Make constants available to assembly Andrew Scull
2020-07-15 18:44 ` [PATCH 02/37] KVM: arm64: Move clearing of vcpu debug dirty bit Andrew Scull
2020-07-15 18:44 ` [PATCH 03/37] KVM: arm64: Track running vCPU outside of the CPU context Andrew Scull
2020-07-15 18:44 ` [PATCH 04/37] KVM: arm64: nVHE: Pass pointers consistently to hyp-init Andrew Scull
2020-07-15 18:44 ` [PATCH 05/37] KVM: arm64: nVHE: Break out of the hyp-init idmap Andrew Scull
2020-07-15 18:44 ` [PATCH 06/37] KVM: arm64: Only check pending interrupts if it would trap Andrew Scull
2020-07-17 16:21 ` Marc Zyngier [this message]
2020-07-15 18:44 ` [PATCH 07/37] KVM: arm64: Separate SError detection from VAXorcism Andrew Scull
2020-07-18 9:00 ` Marc Zyngier
2020-07-20 14:13 ` Andrew Scull
2020-07-20 14:56 ` Marc Zyngier
2020-07-23 0:59 ` FW: " Renters Cancellation Requests
2020-07-20 15:40 ` Andrew Scull
2020-07-20 15:57 ` Marc Zyngier
2020-07-15 18:44 ` [PATCH 08/37] KVM: arm64: nVHE: Introduce a hyp run loop for the host Andrew Scull
2020-07-15 18:44 ` [PATCH 09/37] smccc: Cast arguments to unsigned long Andrew Scull
2020-07-15 18:44 ` [PATCH 10/37] KVM: arm64: nVHE: Migrate hyp interface to SMCCC Andrew Scull
2020-07-15 18:44 ` [PATCH 11/37] KVM: arm64: nVHE: Migrate hyp-init " Andrew Scull
2020-07-15 18:44 ` [PATCH 12/37] KVM: arm64: nVHE: Fix pointers during SMCCC convertion Andrew Scull
2020-07-15 18:44 ` [PATCH 13/37] KVM: arm64: Rename workaround 2 helpers Andrew Scull
2020-07-15 18:44 ` [PATCH 14/37] KVM: arm64: nVHE: Use __kvm_vcpu_run for the host vcpu Andrew Scull
2020-07-15 18:44 ` [PATCH 15/37] KVM: arm64: Share some context save and restore macros Andrew Scull
2020-07-15 18:44 ` [PATCH 16/37] KVM: arm64: nVHE: Handle stub HVCs in the host loop Andrew Scull
2020-07-15 18:44 ` [PATCH 17/37] KVM: arm64: nVHE: Store host sysregs in host vcpu Andrew Scull
2020-07-15 18:44 ` [PATCH 18/37] KVM: arm64: nVHE: Access pmu_events directly in kvm_host_data Andrew Scull
2020-07-15 18:44 ` [PATCH 19/37] KVM: arm64: nVHE: Drop host_ctxt argument for context switching Andrew Scull
2020-07-15 18:44 ` [PATCH 20/37] KVM: arm64: nVHE: Use host vcpu context for host debug state Andrew Scull
2020-07-15 18:44 ` [PATCH 21/37] KVM: arm64: Move host debug state from vcpu to percpu Andrew Scull
2020-07-15 18:44 ` [PATCH 22/37] KVM: arm64: nVHE: Store host's mdcr_el2 and hcr_el2 in its vcpu Andrew Scull
2020-07-15 18:44 ` [PATCH 23/37] KVM: arm64: Skip __hyp_panic and go direct to hyp_panic Andrew Scull
2020-07-15 18:44 ` [PATCH 24/37] KVM: arm64: Break apart kvm_host_data Andrew Scull
2020-07-15 18:44 ` [PATCH 25/37] KVM: arm64: nVHE: Unify sysreg state saving paths Andrew Scull
2020-07-15 18:44 ` [PATCH 26/37] KVM: arm64: nVHE: Unify 32-bit sysreg " Andrew Scull
2020-07-15 18:44 ` [PATCH 27/37] KVM: arm64: nVHE: Unify vgic save and restore Andrew Scull
2020-07-15 18:44 ` [PATCH 28/37] KVM: arm64: nVHE: Unify fpexc32 saving paths Andrew Scull
2020-07-15 18:44 ` [PATCH 29/37] KVM: arm64: nVHE: Separate the save and restore of debug state Andrew Scull
2020-07-15 18:44 ` [PATCH 30/37] KVM: arm64: nVHE: Remove MMU assumption in speculative AT workaround Andrew Scull
2020-07-15 18:44 ` [PATCH 31/37] KVM: arm64: Move speculative AT ISBs into context Andrew Scull
2020-07-15 18:44 ` [PATCH 32/37] KVM: arm64: nVHE: Unify sysreg state restoration paths Andrew Scull
2020-07-15 18:44 ` [PATCH 33/37] KVM: arm64: Remove __activate_vm wrapper Andrew Scull
2020-07-15 18:44 ` [PATCH 34/37] KVM: arm64: nVHE: Unify timer restore paths Andrew Scull
2020-07-15 18:44 ` [PATCH 35/37] KVM: arm64: nVHE: Unify PMU event restoration paths Andrew Scull
2020-07-15 18:44 ` [PATCH 36/37] KVM: arm64: nVHE: Unify GIC PMR " Andrew Scull
2020-07-15 18:44 ` [PATCH 37/37] KVM: arm64: Separate save and restore of vcpu trap state Andrew Scull
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=87blkexgbt.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=ascull@google.com \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.cs.columbia.edu \
/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).