On Wed, Sep 12, 2018 at 12:17:04PM +0200, Christoffer Dall wrote: > On Wed, Sep 05, 2018 at 02:19:13PM +0100, Dave Martin wrote: > > Currently FPEXC32_EL2 is handled specially when context-switching. > > This made sense for arm, where FPEXC affects host execution > > (including VFP/SIMD register save/restore at Hyp). > > I think the point was actually to avoid saving/restoring FPEXC32_EL2 > when VFP was being trapped to EL2 With what purpose in mind? We saved neither the trap (since the VFP registers needed switching anyway) nor the FPEXC32_EL2 switch (since we had to do that at guest entry). I may have misunderstood something. I was somewhat guessing likely rationale, so if you can suggest a way to reword this that reflects the situation better I'd be happy to change it. Thinking about it, I don't see why FPEXC32_EL2 can't be handled via vcpu_load()/_put(). This register doesn't matter for the host, and I can't see why we would need to switch it when going through hyp. > > > > However, FPEXC has no architectural effect when running in AArch64. > > The only case where an arm64 host may execute in AArch32 is when > > running compat user code at EL0: the architecture explicitly > > documents FPEXC as having no effect in this case either when the > > kernel (EL2 in the VHE case; otherwise EL1) is AArch64. > > > > So, we can just handle FPEXC32_EL2 (which is the AArch64 alias for > > this register) as a regular AArch32-only system register and switch > > it without any special handling or synchronisation. > > > > This allows some gnarly special-case code to be eliminated from > > hyp. The extra isb() in __activate_traps_fpsimd32() is dropped > > too: for the reasons explained above arm64 never required this > > anyway. > > > > Cc: Marc Zyngier <marc.zyngier@xxxxxxx> > > Cc: Christoffer Dall <christoffer.dall@xxxxxxx> > > Cc: Alexander Graf <agraf@xxxxxxx> > > Signed-off-by: Dave Martin <Dave.Martin@xxxxxxx> > > --- > > > > I could do with some confirmation from someone that my interpretation of > > the architecture is in fact correct here. The CheckFPAdvSIMDEnabled64() > > and CheckAdvSIMDEnabled() pseudocode functions may be a reasonable > > starting point (this was my reference where the wordy text is unclear). > > Which aspect do you want confirmed here? I confess to a certain amount of "according to the spirit of the architecture it obviously ought to work this way" reasoning. Following the trail of documentation isn't trivial, so it would be good if someone could check that they can reach the same conclusion about FPEXC32_EL2 having no effect on the host. > > If Alexander could try this out, that would be good. > > > > This cleanup applies on top of the following previously pubished fix: > > [PULL 2/4] arm64: KVM: Only force FPEXC32_EL2.EN if trapping FPSIMD > > http://lists.infradead.org/pipermail/linux-arm-kernel/2018-September/599537.html > > > > We can queue this for the next merge window. > > Reviewed-by: Christoffer Dall <christoffer.dall@xxxxxxx> Thanks, modulo the question of maybe moving this to the vcpu_load()/ _put() path. Cheers ---Dave _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
