Hi Marc, On 10/06/2019 17:58, Marc Zyngier wrote: > On 10/06/2019 17:30, James Morse wrote: >> During __guest_exit() we need to consume any SError left pending by the >> guest so it doesn't contaminate the host. With v8.2 we use the >> ESB-instruction. For systems without v8.2, we use dsb+isb and unmask >> SError. We do this on every guest exit. >> >> Use the same dsb+isr_el1 trick, this lets us know if an SError is pending >> after the dsb, allowing us to skip the isb and self-synchronising PSTATE >> write if its not. >> >> This means SError remains masked during KVM's world-switch, so any SError >> that occurs during this time is reported by the host, instead of causing >> a hyp-panic. > > Ah, that'd be pretty good.
I'll add a patch to re-mask it so this intent is clear, and the behaviour/performance stuff is done in separate patches. >> If you give gcc likely()/unlikely() hints in an if() condition, it >> shuffles the generated assembly so that the likely case is immediately >> after the branch. Lets do the same here. >> >> Signed-off-by: James Morse <james.mo...@arm.com> >> --- >> This patch was previously posted as part of: >> [v1] >> https://lore.kernel.org/linux-arm-kernel/20190604144551.188107-1-james.mo...@arm.com/ >> >> arch/arm64/kvm/hyp/entry.S | 14 ++++++++++---- >> 1 file changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S >> index a5a4254314a1..c2de1a1faaf4 100644 >> --- a/arch/arm64/kvm/hyp/entry.S >> +++ b/arch/arm64/kvm/hyp/entry.S >> @@ -161,18 +161,24 @@ alternative_if ARM64_HAS_RAS_EXTN >> orr x0, x0, #(1<<ARM_EXIT_WITH_SERROR_BIT) >> 1: ret >> alternative_else >> - // If we have a pending asynchronous abort, now is the >> - // time to find out. From your VAXorcist book, page 666: >> + dsb sy // Synchronize against in-flight ld/st >> + mrs x2, isr_el1 > > The CPU is allowed to perform a system register access before the DSB > completes if it doesn't have a side effect. Reading ISR_EL1 doesn't have > such side effect, so you could end-up missing the abort. An ISB after > DSB should cure that, ... bother ... > but you'll need to verify that it doesn't make > things much worse than what we already have. Retested with isb in both patches, and Robin's better assembly. This still saves the self-synchronising pstate modification, (of which we'd need two if we want to keep SError masked over the rest of world-switch) On Xgene: | 5.2.0-rc2-00006-g9b94314 mean:3215 stddev:45 | 5.2.0-rc2-00007-g5d37b0b mean:3176 stddev:30 | with this patch 1.23% faster On Seattle: | 5.2.0-rc2-00006-g9b9431445730 mean:4474 stddev:10 | 5.2.0-rc2-00007-g5d37b0b5dd65 mean:4410 stddev:27 | with this patch: 1.44% faster Thanks, James _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
