Hi James, On 04/06/2019 15:45, James Morse wrote: > On systems with v8.2 we switch the 'vaxorcism' of guest SError with an > alternative sequence that uses the ESB-instruction, then reads DISR_EL1. > This saves the unmasking and re-masking of asynchronous exceptions. > > We do this after we've saved the guest registers and restored the > host's. Any SError that becomes pending due to this will be accounted > to the guest, when it actually occurred during host-execution. > > Move the ESB-instruction as early as possible. Any guest SError > will become pending due to this ESB-instruction and then consumed to > DISR_EL1 before the host touches anything. > Since you're moving the ESB from a HAS_RAS alternative location to a normal location, it might be worth noting in the commit message that the ESB is a NOP when RAS is not implemented, to clarify that we are not uselessly adding a barrier (or potentially undefined instruction). > This lets us account for host/guest SError precisely on the guest > exit exception boundary. > > Signed-off-by: James Morse <james.morse@xxxxxxx> > --- > N.B. ESB-instruction is a nop on CPUs that don't support it. > > arch/arm64/include/asm/kvm_asm.h | 2 +- > arch/arm64/kvm/hyp/entry.S | 5 ++--- > arch/arm64/kvm/hyp/hyp-entry.S | 2 ++ > 3 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index 9170c43b332f..5c9548ae8fa7 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -45,7 +45,7 @@ > * Size of the HYP vectors preamble. kvm_patch_vector_branch() generates code > * that jumps over this. > */ > -#define KVM_VECTOR_PREAMBLE 4 > +#define KVM_VECTOR_PREAMBLE 8 > > #ifndef __ASSEMBLY__ > > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S > index 93ba3d7ef027..7863ec5266e2 100644 > --- a/arch/arm64/kvm/hyp/entry.S > +++ b/arch/arm64/kvm/hyp/entry.S > @@ -138,8 +138,8 @@ ENTRY(__guest_exit) > > alternative_if ARM64_HAS_RAS_EXTN > // If we have the RAS extensions we can consume a pending error > - // without an unmask-SError and isb. > - esb > + // without an unmask-SError and isb. The ESB-instruction consumed any > + // pending guest error when we took the exception from the guest. > mrs_s x2, SYS_DISR_EL1 > str x2, [x1, #(VCPU_FAULT_DISR - VCPU_CONTEXT)] > cbz x2, 1f > @@ -157,7 +157,6 @@ alternative_else > mov x5, x0 > > dsb sy // Synchronize against in-flight ld/st > - nop > msr daifclr, #4 // Unmask aborts > alternative_endif > > diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S > index 914036e6b6d7..b8d37a987b34 100644 > --- a/arch/arm64/kvm/hyp/hyp-entry.S > +++ b/arch/arm64/kvm/hyp/hyp-entry.S > @@ -230,6 +230,7 @@ ENDPROC(\label) > .macro valid_vect target > .align 7 > 661: > + esb Having said the above, if the kernel is built without RAS support (you have to disable some of options enabled by default to get to that) but runs on a CPU that does have the RAS extention, should we execute a nop instead of an esb (so have an alternative here)? Also, when we have the smccc workaround installed we do two esb, is that intentional/necessary? Could we have only one esb at the start of hyp_ventry (and "only" 26 nops after it) for KVM_INDIRECT_VECTORS? Or does this not affect performance that much to be of interest? > stp x0, x1, [sp, #-16]! > 662: > b \target > @@ -320,6 +321,7 @@ ENTRY(__bp_harden_hyp_vecs_end) > .popsection > > ENTRY(__smccc_workaround_1_smc_start) > + esb > sub sp, sp, #(8 * 4) > stp x2, x3, [sp, #(8 * 0)] > stp x0, x1, [sp, #(8 * 2)] > Thanks, -- Julien Thierry _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
