On Mon, May 21, 2012 at 11:27 PM, Alexander Graf <agraf at suse.de> wrote: > > > 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 :). > yes, hold off on that. Let me know when you've tested my code and I will include it in the v8 patch series (hopefully later this week). > 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? it loops inside vcpu_run until there's signal_pending() (or need_resched) > > 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 ;). > > agreed :)
