Re: [PATCH 2/2] KVM: SVM: Add support for MPK feature on AMD

[Sean Christopherson]

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
>