Re: [Xen-devel] [PATCH v6.5 19/26] x86/hvm: Permit guests direct access to MSR_{SPEC_CTRL, PRED_CMD}

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 01:15, <andrew.cooper3@xxxxxxxxxx> wrote:
> @@ -292,6 +301,16 @@ static int update_domain_cpuid_info(struct domain *d,
>              d->arch.pv_domain.cpuidmasks->e1cd = mask;
>          }
>          break;
> +
> +    case 0x80000008:
> +        /*
> +         * If the IBRB policy has changed, we need to recalculate the MSR
> +         * interception bitmaps.
> +         */
> +        call_policy_changed = (is_hvm_domain(d) &&
> +                               ((old_e8b ^ p->extd.raw[8].b) &
> +                                (cpufeat_mask(X86_FEATURE_IBPB))));

There's a stray pair of parentheses here.

> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -132,7 +132,8 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
> uint64_t *val)
>      case MSR_SPEC_CTRL:
>          if ( !cp->feat.ibrsb )
>              goto gp_fault;
> -        *val = vp->spec_ctrl.guest;
> +        *val = (vp->spec_ctrl.direct_access
> +                ? vp->spec_ctrl.host : vp->spec_ctrl.guest);
>          break;

To recap, I had asked whether this is valid ahead of later changes,
which you replied to saying this won't have any "by not permitting
the guest any access until patch 25". In which case at the very
least the patch title is misleading. Yet I don't even agree with what
you say - patch 25 only fiddles with CPUID bits. Did you perhaps
mean to say "By not permitting a well behaved guest any access
until patch 25," as one trying to access the MSRs without consulting
the CPUID bits would be able to starting with the patch here aiui?

I do realize that re-ordering the series may be impossible, so I'm
not necessarily asking for a code change. But at least the
description should explain the situation.

Jan


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