> > + * Simulate AltiVec unavailable fault to load guest state
> > + * from thread to AltiVec unit.
> > + * It requires to be called with preemption disabled.
> > + */
> > +static inline void kvmppc_load_guest_altivec(struct kvm_vcpu *vcpu)
> > +{
> > + if (kvmppc_supports_altivec()) {
> > + if (!(current->thread.regs->msr & MSR_VEC)) {
> > + load_up_altivec(NULL);
> > + current->thread.regs->msr |= MSR_VEC;
>
> Does this ensure that the kernel saves / restores all altivec state on
> task switch? Does it load it again when it schedules us in? Would
> definitely be worth a comment.
These units are _LAZY_ !!! Only SMP kernel save the state when it schedules out.
>
> > + }
> > + }
> > +}
> > +
> > +/*
> > * Helper function for "full" MSR writes. No need to call this if only
> > * EE/CE/ME/DE/RI are changing.
> > */
> > @@ -678,6 +706,12 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run,
> struct kvm_vcpu *vcpu)
> > u64 fpr[32];
> > #endif
> >
> > +#ifdef CONFIG_ALTIVEC
> > + vector128 vr[32];
> > + vector128 vscr;
> > + int used_vr = 0;
>
> bool
Why don't you ask first to change the type of used_vr member in
/include/asm/processor.h?
>
> > +#endif
> > +
> > if (!vcpu->arch.sane) {
> > kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> > return -EINVAL;
> > @@ -716,6 +750,22 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run,
> struct kvm_vcpu *vcpu)
> > kvmppc_load_guest_fp(vcpu);
> > #endif
> >
> > +#ifdef CONFIG_ALTIVEC
>
> /* Switch from user space Altivec to guest Altivec state */
>
> > + if (cpu_has_feature(CPU_FTR_ALTIVEC)) {
>
> Why not use your kvmppc_supports_altivec() helper here?
Give it a try ... because Linus guarded this members with CONFIG_ALTIVEC :)
>
> > + /* Save userspace VEC state in stack */
> > + enable_kernel_altivec();
>
> Can't you hide this in the load function? We only want to enable kernel
> altivec for a short time while we shuffle the registers around.
>
> > + memcpy(vr, current->thread.vr, sizeof(current->thread.vr));
>
> vr = current->thread.vr;
Are you kidding, be more careful with arrays :)
>
> > + vscr = current->thread.vscr;
> > + used_vr = current->thread.used_vr;
> > +
> > + /* Restore guest VEC state to thread */
> > + memcpy(current->thread.vr, vcpu->arch.vr, sizeof(vcpu-
> >arch.vr));
>
> current->thread.vr = vcpu->arch.vr;
>
> > + current->thread.vscr = vcpu->arch.vscr;
> > +
> > + kvmppc_load_guest_altivec(vcpu);
> > + }
>
> Do we need to do this even when the guest doesn't use Altivec? Can't we
> just load it on demand then once we fault? This code path really should
> only be a prefetch enable when MSR_VEC is already set in guest context.
No we can't, read 6/6.
>
> > +#endif
> > +
> > ret = __kvmppc_vcpu_run(kvm_run, vcpu);
> >
> > /* No need for kvm_guest_exit. It's done in handle_exit.
> > @@ -736,6 +786,23 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run,
> struct kvm_vcpu *vcpu)
> > current->thread.fpexc_mode = fpexc_mode;
> > #endif
> >
> > +#ifdef CONFIG_ALTIVEC
>
> /* Switch from guest Altivec to user space Altivec state */
>
> > + if (cpu_has_feature(CPU_FTR_ALTIVEC)) {
> > + /* Save AltiVec state to thread */
> > + if (current->thread.regs->msr & MSR_VEC)
> > + giveup_altivec(current);
> > +
> > + /* Save guest state */
> > + memcpy(vcpu->arch.vr, current->thread.vr, sizeof(vcpu-
> >arch.vr));
> > + vcpu->arch.vscr = current->thread.vscr;
> > +
> > + /* Restore userspace state */
> > + memcpy(current->thread.vr, vr, sizeof(current->thread.vr));
> > + current->thread.vscr = vscr;
> > + current->thread.used_vr = used_vr;
> > + }
>
> Same comments here. If the guest never touched Altivec state, don't
> bother restoring it, as it's still good.
LOL, the mighty guest kernel does whatever he wants with AltiVec and
doesn't inform us.
-Mike
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
