Re: [PATCH v3 8/8] KVM:X86: Add user-space read/write interface for CET MSRs.

[Sean Christopherson]

 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.