Re: [Xen-devel] [PATCH v8 08/17] x86/msr: Emulation of MSR_{SPEC_CTRL, PRED_CMD} for guests

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/01/18 11:10, David Woodhouse wrote:
> On Fri, 2018-01-12 at 18:00 +0000, Andrew Cooper wrote:
>> @@ -152,14 +163,38 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr,
>> uint64_t val)
>>  {
>>      const struct vcpu *curr = current;
>>      struct domain *d = v->domain;
>> +    const struct cpuid_policy *cp = d->arch.cpuid;
>>      struct msr_domain_policy *dp = d->arch.msr;
>>      struct msr_vcpu_policy *vp = v->arch.msr;
>>  
>>      switch ( msr )
>>      {
>>      case MSR_INTEL_PLATFORM_INFO:
>> +        /* Read-only */
>>          goto gp_fault;
>>  
>> +    case MSR_SPEC_CTRL:
>> +        if ( !cp->feat.ibrsb )
>> +            goto gp_fault; /* MSR available? */
>> +        if ( val & ~(SPEC_CTRL_IBRS |
>> +                     (cp->feat.stibp ? SPEC_CTRL_STIBP : 0)) )
> Intel defines the STIBP bit as non-faulting and ignored, even when
> STIBP isn't advertised. So you should probably just mask it out
> if !cp->feat.stibp.

Well - this published spec finally answers several several-month-old
outstanding questions of mine.

Time for some rewriting.  /sigh

>
>> +            goto gp_fault; /* Rsvd bit set? */
>> +        vp->spec_ctrl.guest = val;
>> +        vp->spec_ctrl.host  = val;
>> +        break;
>> +
> There's no actual wrmsr there. This is fine, because you're going to do
> it on the way back to the guest (albeit in a later patch in the
> series). But it probably warrants a comment?

Actually, IBPB being a wrmsr() here is going to be the rare
circustances.  Most of guest_wrmsr() will only be updating internal
hypervisor state, and/or the VMCS/VMCB when I move some more of the HVM
logic here.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel