Skip to content

Commit 3748613

Browse files
babumogerbonzini
authored andcommitted
KVM: x86: Fix pkru save/restore when guest CR4.PKE=0, move it to x86.c
Though rdpkru and wrpkru are contingent upon CR4.PKE, the PKRU resource isn't. It can be read with XSAVE and written with XRSTOR. So, if we don't set the guest PKRU value here(kvm_load_guest_xsave_state), the guest can read the host value. In case of kvm_load_host_xsave_state, guest with CR4.PKE clear could potentially use XRSTOR to change the host PKRU value. While at it, move pkru state save/restore to common code and the host_pkru field to kvm_vcpu_arch. This will let SVM support protection keys. Cc: [email protected] Reported-by: Jim Mattson <[email protected]> Signed-off-by: Babu Moger <[email protected]> Message-Id: <158932794619.44260.14508381096663848853.stgit@naples-babu.amd.com> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent 7d61123 commit 3748613

File tree

3 files changed

+18
-18
lines changed

3 files changed

+18
-18
lines changed

arch/x86/include/asm/kvm_host.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,7 @@ struct kvm_vcpu_arch {
578578
unsigned long cr4;
579579
unsigned long cr4_guest_owned_bits;
580580
unsigned long cr8;
581+
u32 host_pkru;
581582
u32 pkru;
582583
u32 hflags;
583584
u64 efer;

arch/x86/kvm/vmx/vmx.c

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,7 +1372,6 @@ void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
13721372

13731373
vmx_vcpu_pi_load(vcpu, cpu);
13741374

1375-
vmx->host_pkru = read_pkru();
13761375
vmx->host_debugctlmsr = get_debugctlmsr();
13771376
}
13781377

@@ -6564,11 +6563,6 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
65646563

65656564
kvm_load_guest_xsave_state(vcpu);
65666565

6567-
if (static_cpu_has(X86_FEATURE_PKU) &&
6568-
kvm_read_cr4_bits(vcpu, X86_CR4_PKE) &&
6569-
vcpu->arch.pkru != vmx->host_pkru)
6570-
__write_pkru(vcpu->arch.pkru);
6571-
65726566
pt_guest_enter(vmx);
65736567

65746568
if (vcpu_to_pmu(vcpu)->version)
@@ -6658,18 +6652,6 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
66586652

66596653
pt_guest_exit(vmx);
66606654

6661-
/*
6662-
* eager fpu is enabled if PKEY is supported and CR4 is switched
6663-
* back on host, so it is safe to read guest PKRU from current
6664-
* XSAVE.
6665-
*/
6666-
if (static_cpu_has(X86_FEATURE_PKU) &&
6667-
kvm_read_cr4_bits(vcpu, X86_CR4_PKE)) {
6668-
vcpu->arch.pkru = rdpkru();
6669-
if (vcpu->arch.pkru != vmx->host_pkru)
6670-
__write_pkru(vmx->host_pkru);
6671-
}
6672-
66736655
kvm_load_host_xsave_state(vcpu);
66746656

66756657
vmx->nested.nested_run_pending = 0;

arch/x86/kvm/x86.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -837,11 +837,25 @@ void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu)
837837
vcpu->arch.ia32_xss != host_xss)
838838
wrmsrl(MSR_IA32_XSS, vcpu->arch.ia32_xss);
839839
}
840+
841+
if (static_cpu_has(X86_FEATURE_PKU) &&
842+
(kvm_read_cr4_bits(vcpu, X86_CR4_PKE) ||
843+
(vcpu->arch.xcr0 & XFEATURE_MASK_PKRU)) &&
844+
vcpu->arch.pkru != vcpu->arch.host_pkru)
845+
__write_pkru(vcpu->arch.pkru);
840846
}
841847
EXPORT_SYMBOL_GPL(kvm_load_guest_xsave_state);
842848

843849
void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu)
844850
{
851+
if (static_cpu_has(X86_FEATURE_PKU) &&
852+
(kvm_read_cr4_bits(vcpu, X86_CR4_PKE) ||
853+
(vcpu->arch.xcr0 & XFEATURE_MASK_PKRU))) {
854+
vcpu->arch.pkru = rdpkru();
855+
if (vcpu->arch.pkru != vcpu->arch.host_pkru)
856+
__write_pkru(vcpu->arch.host_pkru);
857+
}
858+
845859
if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE)) {
846860

847861
if (vcpu->arch.xcr0 != host_xcr0)
@@ -3549,6 +3563,9 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
35493563

35503564
kvm_x86_ops.vcpu_load(vcpu, cpu);
35513565

3566+
/* Save host pkru register if supported */
3567+
vcpu->arch.host_pkru = read_pkru();
3568+
35523569
/* Apply any externally detected TSC adjustments (due to suspend) */
35533570
if (unlikely(vcpu->arch.tsc_offset_adjustment)) {
35543571
adjust_tsc_offset_host(vcpu, vcpu->arch.tsc_offset_adjustment);

0 commit comments

Comments
 (0)