On Thu, Feb 28, 2019 at 05:41:52PM +0800, Yang Weijiang wrote: > On Thu, Feb 28, 2019 at 08:32:49AM -0800, Sean Christopherson wrote: > > On Mon, Feb 25, 2019 at 09:27:16PM +0800, Yang Weijiang wrote: ... > > > static int __msr_io(struct kvm_vcpu *vcpu, struct kvm_msrs *msrs, > > > - struct kvm_msr_entry *entries, > > > + struct kvm_msr_entry *entries, bool read, > > > int (*do_msr)(struct kvm_vcpu *vcpu, > > > unsigned index, u64 *data)) > > > { > > > int i; > > > > > > - for (i = 0; i < msrs->nmsrs; ++i) > > > + for (i = 0; i < msrs->nmsrs; ++i) { > > > + /* If it comes to CET related MSRs, read them together. */ > > > + if (entries[i].index == MSR_IA32_U_CET) { > > > + i += do_cet_msrs(vcpu, i, entries, read) - 1; > > > > This wrong, not to mention horribly buggy. The correct way to handle > > MSRs that are switched via the VMCS is to read/write the VMCS in > > vmx_{get,set}_msr() as needed, e.g. vmcs_writel(GUEST_GS_BASE, data). > > > The code is barely for migration purpose since they're passed through > to guest, I have no intent to expose them just like normal MSRs. > I double checked the spec.: > MSR_IA32_U_CET 0x6a0 > MSR_IA32_PL0_SSP 0x6a4 > MSR_IA32_PL3_SSP 0x6a7 > should come from xsave area. > > MSR_IA32_S_CET 0x6a2 > MSR_IA32_INTR_SSP_TABL 0x6a8 > should come from VMCS guest fields. > > for the former, they're stored in guest fpu area, need > to restore them with xrstors temporarily before read, while the later is stored in > VMCS guest fields, I can read them out via vmcs_read() directly. > > I'd like to read them out as a whole(all or nothing) due to cost induced > by xsaves/xrstors, in Qemu I put these MSRs as sequential fields. > > what do you think of it? It doesn't need to be all or nothing, it should be simple enough to set a flag when we load guest state and use it to skip reloading guest state and to load host state before returning. I think the attached patch does the trick.