On Wed, May 06, 2020 at 05:02:21PM -0500, Babu Moger wrote:
> static __init int svm_hardware_setup(void)
> @@ -1300,6 +1304,8 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int
> cpu)
> indirect_branch_prediction_barrier();
> }
> avic_vcpu_load(vcpu, cpu);
> +
> + svm->host_pkru = read_pkru();
Move vcpu_vmx's host_prku to kvm_vcpu_arch instead of duplicating it to
SVM. And I'm 99% certain "vcpu->arch.host_pkru = read_pkru()" can be moved
to kvm_arch_vcpu_load(). The only direct calls to vmx_vcpu_load() are to
get the right VMCS loaded. Actually, those calls shouldn't be using
vmx_vcpu_load(), especially since that'll trigger IBPB. I'll send a patch
for that.
> }
>
> static void svm_vcpu_put(struct kvm_vcpu *vcpu)
> @@ -3318,6 +3324,12 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
> clgi();
> kvm_load_guest_xsave_state(vcpu);
>
> + /* Load the guest pkru state */
> + if (static_cpu_has(X86_FEATURE_PKU) &&
> + kvm_read_cr4_bits(vcpu, X86_CR4_PKE) &&
> + vcpu->arch.pkru != svm->host_pkru)
> + __write_pkru(vcpu->arch.pkru);
This and the restoration should be moved to common x86 helpers, at a glance
they look identical.
In short, pretty much all of this belongs in common x86.
> +
> if (lapic_in_kernel(vcpu) &&
> vcpu->arch.apic->lapic_timer.timer_advance_ns)
> kvm_wait_lapic_expire(vcpu);
> @@ -3371,6 +3383,14 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
> if (unlikely(svm->vmcb->control.exit_code == SVM_EXIT_NMI))
> kvm_before_interrupt(&svm->vcpu);
>
> + /* Save the guest pkru state and restore the host pkru state back */
> + if (static_cpu_has(X86_FEATURE_PKU) &&
> + kvm_read_cr4_bits(vcpu, X86_CR4_PKE)) {
> + vcpu->arch.pkru = rdpkru();
> + if (vcpu->arch.pkru != svm->host_pkru)
> + __write_pkru(svm->host_pkru);
> + }
> +
> kvm_load_host_xsave_state(vcpu);
> stgi();
>
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index df3474f4fb02..5d20a28c1b0e 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -158,6 +158,8 @@ struct vcpu_svm {
> u64 *avic_physical_id_cache;
> bool avic_is_running;
>
> + u32 host_pkru;
> +
> /*
> * Per-vcpu list of struct amd_svm_iommu_ir:
> * This is used mainly to store interrupt remapping information used
>
