On Tue, Oct 17, 2017 at 01:50:24PM +0200, Christoffer Dall wrote: > On Tue, Oct 10, 2017 at 07:38:39PM +0100, Dave Martin wrote: > > Until KVM has full SVE support, guests must not be allowed to > > execute SVE instructions. > > > > This patch enables the necessary traps, and also ensures that the > > traps are disabled again on exit from the guest so that the host > > can still use SVE if it wants to. > > > > This patch introduces another instance of > > __this_cpu_write(fpsimd_last_state, NULL), so this flush operation > > is abstracted out as a separate helper fpsimd_flush_cpu_state(). > > Other instances are ported appropriately. > > I don't understand this paragraph, beginning from ", so this...". > > > From reading the code, what I think is the reason for having to flush > the SVE state (and mark the host state invalid) is that even though we > disallow SVE usage in the guest, the guest can use the normal FP state, > and while we always fully preserve the host state, this could still > corrupt some additional SVE state not properly preserved for the host. > Is that correct? Yes, that's right: the guest can't touch the SVE-specific registers Pn/FFR, but FPSIMD accesses to Vn regs cause the high bits of the corresponding SVE Zn registers to be clobbered. In any case, the FPSIMD restore done by KVM after guest exit is sufficient to clobber those bits even if the guest didn't do it. This is a band-aid for not making the KVM world switch code properly SVE-aware yet. Does the following wording sound better: --8<-- On guest exit, high bits of the SVE Zn registers may have been clobbered as a side-effect the execution of FPSIMD instructions in the guest. The existing KVM host FPSIMD restore code is not sufficient to restore these bits, so this patch explicitly marks the CPU as not containing cached vector state for any task, this forcing a reload on the next return to userspace. This is an interim measure, in advance of adding full SVE awareness to KVM. Because of the duplication of this operation (__this_cpu_write(fpsimd_last_state, NULL)), it is factored out as a new helper fpsimd_flush_cpu_state() to make the purpose clearer. -->8-- > > > > As a side effect of this refactoring, a this_cpu_write() in > > fpsimd_cpu_pm_notifier() is changed to __this_cpu_write(). This > > should be fine, since cpu_pm_enter() is supposed to be called only > > with interrupts disabled. > > Otherwise the patch itself looks good to me. Thanks, let me know about the above wording change though. ---Dave _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
