On 22.05.2012, at 05:00, Rusty Russell <rusty.russell at linaro.org> wrote: > On Mon, 21 May 2012 11:41:31 +0200, Antonios Motakis <a.motakis at virtualopensystems.com> wrote: >> On 05/21/2012 05:49 AM, Rusty Russell wrote: >>> The guest may turn FPEXEC_EN off, but that doesn't mean the state isn't >>> important. In particular, a (non-SMP) Linux ARM guest will turn the >>> FPEXEC_EN bit off on context switch, without saving the contents. >>> >>> Similar logic applied to the host: we might have turned FPEXEC_EN off, >>> but that doesn't mean the contents of the registers aren't worth saving. >> >> My first attempted implementation tried to save/restore state regardless >> of FPEXC_EN. However, it turned out that when FPEXC_EN is 0, most VFP >> instructions will cause an undefined instruction trap (ARM arm p. >> B1-1225) so the code wouldn't work. Is the hardware supposed to preserve >> state even when the FPU extensions are disabled? > > Yes, in fact, the kernel relies on it; it's pretty standard on most > archs to be able to flip the FPU on and off to implement lazy FPU > switching. The ARM Linux implementation is classic here: > > On context switch: (vfp_notifier() in vfpmodule.c) > (SMP only): If FPEXEC_EN, save the state of the FPU to outgoing task. > Disable FPEXEC_EN. > > On trap: > Enable FPEXEC_EN. > If the FPU contents are up-to-date and from the current task, return. > (UP only): push the FPU state to previous task (if any) > Load FPU state from current task. > > Anyway, here's the patch I'm working on, based on yours. It compiles, > haven't run it yet :) > > It went through various complicated iterations yesterday, but I think > this is finally the simplest we can do. > > It's based on your idea about working in hyp mode: we use HCPTR to stop > the guest from using the FPU. If they trap, clear the HCPTR and save > host & restore guest FPU state, then return back to guest. > > On normal exit we check if HCPTR was clear, and if so, we save and > restore FPU state. > > This means that the FPU state is always saved and restored across > __kvm_vcpu_run. So we can simply enable & restore FPEXEC_EN across that > function: noone can see it. > > Note that in some cases, we could skip saving and restoring the host FPU > state. But that seems like a premature optimization, Yup, getting things work first and fast later is usually the better approach :). How does the ARM code work here? Are you guys doing it x86 style and exit vcpu_run for every intercept or are you going the ppc way to keep inside vcpu_run until we want to return to user space? The former benefits a lot from not saving/restoring the host fpu state. The latter not so much :). > and makes us > depend on the specific Linux kernel saving behavior. ... which should be a reasonable thing to do given that kvm is part of the Linux kernel ;). Alex