On 09.01.2012, at 22:48, Scott Wood wrote:
> On 01/09/2012 11:48 AM, Alexander Graf wrote:
>>
>> On 21.12.2011, at 02:34, Scott Wood wrote:
>>> +#ifdef CONFIG_PPC_FPU
>>> + /* Save userspace FPU state in stack */
>>> + enable_kernel_fp();
>>> + memcpy(fpr, current->thread.fpr, sizeof(current->thread.fpr));
>>> + fpscr = current->thread.fpscr.val;
>>> + fpexc_mode = current->thread.fpexc_mode;
>>> +
>>> + /* Restore guest FPU state to thread */
>>> + memcpy(current->thread.fpr, vcpu->arch.fpr, sizeof(vcpu->arch.fpr));
>>> + current->thread.fpscr.val = vcpu->arch.fpscr;
>>> +
>>> + /*
>>> + * Since we can't trap on MSR_FP in GS-mode, we consider the guest
>>> + * as always using the FPU. Kernel usage of FP (via
>>> + * enable_kernel_fp()) in this thread must not occur while
>>> + * vcpu->fpu_active is set.
>>> + */
>>> + vcpu->fpu_active = 1;
>>> +
>>> + kvmppc_load_guest_fp(vcpu);
>>> +#endif
>>
>> Do you think it's possible to combine this with the book3s_pr code, so we don't duplicate too much here?
>
> book3s_pr is a bit different in that it can trap when the guest sets
> MSR[FP].
Ah, there's no doorbell? So you always have to swap fpu registers? You still have to enable it manually when preempting in, right? IIRC ppc32 does lazy fpu activation.
> Maybe a few lines could be factored out (the first memcpy, fpscr,
> fpexc_mode). I'm not sure that it makes sense given the lack of
> isolation between what it's doing and what the rest of the code is doing.
Yeah, looking at the code it does look pretty different. Too bad - I would've hoped to throw the vmx code in as well so we could get vmx/vsx/whatever for free later.
>
>>> +/*
>>> + * Load up guest vcpu FP state if it's needed.
>>> + * It also set the MSR_FP in thread so that host know
>>> + * we're holding FPU, and then host can help to save
>>> + * guest vcpu FP state if other threads require to use FPU.
>>> + * This simulates an FP unavailable fault.
>>> + *
>>> + * It requires to be called with preemption disabled.
>>> + */
>>> +static inline void kvmppc_load_guest_fp(struct kvm_vcpu *vcpu)
>>> +{
>>> +#ifdef CONFIG_PPC_FPU
>>> + if (vcpu->fpu_active && !(current->thread.regs->msr & MSR_FP)) {
>>> + load_up_fpu();
>>> + current->thread.regs->msr |= MSR_FP;
>>
>> I'm having a hard time to grasp when shared->msr, shadow_msr and regs->msr is used in your code :).
>
> shadow_msr is the real MSR.
>
> shared->msr is the guest's view of MSR.
>
> current->thread.regs->msr is nominally userspace's MSR. In this case we
> use it to tell host Linux that FP is in use and must be saved on context
> switch. The actual userspace MSR_FP is known to be clear at this point
> because we called enable_kernel_fp(). It will be clear again when we
> return to userspace because we'll call giveup_fpu().
Ah, this is thread.regs, not vcpu.regs. Sorry, I misread that part. This way it obviously makes a lot more sense.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
