Re: [PATCH 1/2] KVM: x86: simplify handling of PKRU

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux