Messages in this thread | | | Date | Wed, 10 Nov 2021 08:56:41 +0800 | Subject | Re: [PATCH v5 3/7] KVM: X86: Expose IA32_PKRS MSR | From | Chenyi Qiang <> |
| |
On 11/9/2021 11:30 PM, Sean Christopherson wrote: > On Tue, Nov 09, 2021, Chenyi Qiang wrote: >> >> On 11/9/2021 1:44 AM, Sean Christopherson wrote: >>> Hrm. Ideally this would be open coded in vmx_set_msr(). Long term, the RESET/INIT >>> paths should really treat MSR updates as "normal" host_initiated writes instead of >>> having to manually handle every MSR. >>> >>> That would be a bit gross to handle in vmx_vcpu_reset() since it would have to >>> create a struct msr_data (because __kvm_set_msr() isn't exposed to vendor code), >>> but since vcpu->arch.pkrs is relevant to the MMU I think it makes sense to >>> initiate the write from common x86. >>> >>> E.g. this way there's not out-of-band special code, vmx_vcpu_reset() is kept clean, >>> and if/when SVM gains support for PKRS this particular path Just Works. And it would >>> be an easy conversion for my pipe dream plan of handling MSRs at RESET/INIT via a >>> list of MSRs+values. >>> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index ac83d873d65b..55881d13620f 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -11147,6 +11147,9 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) >>> kvm_set_rflags(vcpu, X86_EFLAGS_FIXED); >>> kvm_rip_write(vcpu, 0xfff0); >>> >>> + if (kvm_cpu_cap_has(X86_FEATURE_PKS)) >>> + __kvm_set_msr(vcpu, MSR_IA32_PKRS, 0, true); >>> + >> >> Got it. In addition, is it necessary to add on-INIT check? like: >> >> if (kvm_cpu_cap_has(X86_FEATURE_PKS) && !init_event) >> __kvm_set_msr(vcpu, MSR_IA32_PKRS, 0, true); >> >> PKRS should be preserved on INIT, not cleared. The SDM doesn't make this >> clear either. > > Hmm, but your cover letter says: > > To help patches review, one missing info in SDM is that PKSR will be > cleared on Powerup/INIT/RESET, which should be listed in Table 9.1 > "IA-32 and Intel 64 Processor States Following Power-up, Reset, or INIT" > > Which honestly makes me a little happy because I thought I was making stuff up > for a minute :-) > > So which is it?
Sorry about the confusion. PKRS is preserved on INIT. I tried to correct my statement in previous ping mail but seems not so obvious.
>
| |