KVM ARM Archive on lore.kernel.org
 help / color / Atom feed
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 07/37] KVM: arm64: Separate SError detection from VAXorcism
Date: Sat, 18 Jul 2020 10:00:30 +0100
Message-ID: <87a6zxxknl.wl-maz@kernel.org> (raw)
In-Reply-To: <20200715184438.1390996-8-ascull@google.com>

Hi Andrew,

On Wed, 15 Jul 2020 19:44:08 +0100,
Andrew Scull <ascull@google.com> wrote:
> 
> When exiting a guest, just check whether there is an SError pending and
> set the bit in the exit code. The fixup then initiates the ceremony
> should it be required.
> 
> The separation allows for easier choices to be made as to whether the
> demonic consultation should proceed.

Such as?

> 
> Signed-off-by: Andrew Scull <ascull@google.com>
> ---
>  arch/arm64/include/asm/kvm_hyp.h        |  2 ++
>  arch/arm64/kvm/hyp/entry.S              | 27 +++++++++++++++++--------
>  arch/arm64/kvm/hyp/hyp-entry.S          |  1 -
>  arch/arm64/kvm/hyp/include/hyp/switch.h |  4 ++++
>  4 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 07745d9c49fc..50a774812761 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -91,6 +91,8 @@ void deactivate_traps_vhe_put(void);
>  
>  u64 __guest_enter(struct kvm_vcpu *vcpu, struct kvm_cpu_context *host_ctxt);
>  
> +void __vaxorcize_serror(void);

I think a VAXorsist reference in the comments is plenty. The code can
definitely stay architectural. Something like "__handle_SEI()" should
be good. I'm not *that* fun.

> +
>  void __noreturn hyp_panic(struct kvm_cpu_context *host_ctxt);
>  #ifdef __KVM_NVHE_HYPERVISOR__
>  void __noreturn __hyp_do_panic(unsigned long, ...);
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 6a641fcff4f7..dc4e3e7e7407 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -174,18 +174,31 @@ alternative_if ARM64_HAS_RAS_EXTN
>  	mrs_s	x2, SYS_DISR_EL1
>  	str	x2, [x1, #(VCPU_FAULT_DISR - VCPU_CONTEXT)]
>  	cbz	x2, 1f
> -	msr_s	SYS_DISR_EL1, xzr
>  	orr	x0, x0, #(1<<ARM_EXIT_WITH_SERROR_BIT)
> -1:	ret
> +	nop
> +1:
>  alternative_else
>  	dsb	sy		// Synchronize against in-flight ld/st
>  	isb			// Prevent an early read of side-effect free ISR
>  	mrs	x2, isr_el1
> -	tbnz	x2, #8, 2f	// ISR_EL1.A
> -	ret
> -	nop
> +	tbz	x2, #8, 2f	// ISR_EL1.A
> +	orr	x0, x0, #(1<<ARM_EXIT_WITH_SERROR_BIT)
>  2:
>  alternative_endif
> +
> +	ret
> +SYM_FUNC_END(__guest_enter)
> +
> +/*
> + * void __vaxorcize_serror(void);
> + */
> +SYM_FUNC_START(__vaxorcize_serror)
> +
> +alternative_if ARM64_HAS_RAS_EXTN
> +	msr_s	SYS_DISR_EL1, xzr
> +	ret
> +alternative_else_nop_endif
> +
>  	// We know we have a pending asynchronous abort, now is the
>  	// time to flush it out. From your VAXorcist book, page 666:
>  	// "Threaten me not, oh Evil one!  For I speak with
> @@ -193,7 +206,6 @@ alternative_endif
>  	mrs	x2, elr_el2
>  	mrs	x3, esr_el2
>  	mrs	x4, spsr_el2
> -	mov	x5, x0
>  
>  	msr	daifclr, #4	// Unmask aborts
>  
> @@ -217,6 +229,5 @@ abort_guest_exit_end:
>  	msr	elr_el2, x2
>  	msr	esr_el2, x3
>  	msr	spsr_el2, x4
> -	orr	x0, x0, x5
>  1:	ret
> -SYM_FUNC_END(__guest_enter)
> +SYM_FUNC_END(__vaxorcize_serror)
> diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
> index e727bee8e110..c441aabb8ab0 100644
> --- a/arch/arm64/kvm/hyp/hyp-entry.S
> +++ b/arch/arm64/kvm/hyp/hyp-entry.S
> @@ -177,7 +177,6 @@ el2_error:
>  	adr	x1, abort_guest_exit_end
>  	ccmp	x0, x1, #4, ne
>  	b.ne	__hyp_panic
> -	mov	x0, #(1 << ARM_EXIT_WITH_SERROR_BIT)
>  	eret
>  	sb
>  
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 0511af14dc81..14a774d1a35a 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -405,6 +405,10 @@ static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
>   */
>  static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  {
> +	/* Flush guest SErrors. */
> +	if (ARM_SERROR_PENDING(*exit_code))
> +		__vaxorcize_serror();
> +
>  	if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
>  		vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
>  
> -- 
> 2.27.0.389.gc38d7665816-goog
> 
> 

I'm not against this patch, but I wonder whether it actually helps
with anything. It spreads the handling across multiple paths, making
it harder to read. Could you explain the rational beyond "it's
easier"?

Thanks,

	M.

-- 
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

  reply index

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
2020-07-15 18:44 ` [PATCH 07/37] KVM: arm64: Separate SError detection from VAXorcism Andrew Scull
2020-07-18  9:00   ` Marc Zyngier [this message]
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=87a6zxxknl.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

KVM ARM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvmarm/0 kvmarm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvmarm kvmarm/ https://lore.kernel.org/kvmarm \
		kvmarm@lists.cs.columbia.edu
	public-inbox-index kvmarm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/edu.columbia.cs.lists.kvmarm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git