On 2017/8/24 17:19, Paolo Bonzini wrote:
On 24/08/2017 11:09, Yang Zhang wrote:
+ if (static_cpu_has(X86_FEATURE_OSPKE) &&
We expose protection key to VM without check whether OSPKE is enabled or
not. Why not check guest's cpuid here which also can avoid unnecessary
access to pkru?
Checking guest CPUID is pretty slow. We could check CR4.PKE though.
Also, using static_cpu_has with OSPKE is probably wrong. But if we do
check CR4.PKE, we can check X86_FEATURE_PKU instead, so something like
if (static_cpu_has(X86_FEATURE_PKU) &&
kvm_read_cr4_bits(vcpu, X86_CR4_PKE) &&
vcpu->arch.pkru != vmx->host_pkru)
... but then, kvm_read_cr4_bits is also pretty slow---and we don't
really need it, since all CR4 writes cause a vmexit. So for now I'd
stay with this patch, only s/static_cpu_has/boot_cpu_has/g.
Of course you can send improvements on top!
ok, since most OS distributions don't support protection key so far
which means vcpu->arch.pkru always 0 in it and i remember host_pkru will
be set to 55555554 be default. To avoid the unnecessary access to pkru,
how about the following change:
@@ -4338,6 +4331,9 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu,
unsigned long cr4)
return 1;
}
+ if (cr4 & X86_CR4_PKE)
+ to_vmx(vcpu)->guest_pkru_valid = true;
+
if (to_vmx(vcpu)->nested.vmxon && !nested_cr4_valid(vcpu, cr4))
return 1;
@@ -9020,8 +9016,10 @@ static void __noclone vmx_vcpu_run(struct
kvm_vcpu *vcpu)
if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
vmx_set_interrupt_shadow(vcpu, 0);
- if (vmx->guest_pkru_valid)
- __write_pkru(vmx->guest_pkru);
+ if (static_cpu_has(X86_FEATURE_OSPKE) &&
+ vmx->guest_pkru_valid &&
+ vcpu->arch.pkru != vmx->host_pkru)
+ __write_pkru(vcpu->arch.pkru);
atomic_switch_perf_msrs(vmx);
debugctlmsr = get_debugctlmsr();
@@ -9169,13 +9167,11 @@ static void __noclone vmx_vcpu_run(struct
kvm_vcpu *vcpu)
* back on host, so it is safe to read guest PKRU from current
* XSAVE.
*/
- if (boot_cpu_has(X86_FEATURE_OSPKE)) {
- vmx->guest_pkru = __read_pkru();
- if (vmx->guest_pkru != vmx->host_pkru) {
- vmx->guest_pkru_valid = true;
+ if (static_cpu_has(X86_FEATURE_OSPKE) &&
+ vmx->guest_pkru_valid) {
+ vcpu->arch.pkru = __read_pkru();
+ if (vcpu->arch.pkru != vmx->host_pkru)
__write_pkru(vmx->host_pkru);
- } else
- vmx->guest_pkru_valid = false;
}
/*
--
Yang
Alibaba Cloud Computing
