Re: [Xen-devel] [PATCH v4 7/7] x86/msr: handle VMX MSRs with guest_rd/wrmsr()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.10.17 at 10:27, <sergey.dyasli@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -360,8 +360,10 @@ int init_vcpu_msr_policy(struct vcpu *v)
>  
>  int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
>  {
> +    const struct domain *d = v->domain;
>      const struct msr_domain_policy *dp = v->domain->arch.msr;
>      const struct msr_vcpu_policy *vp = v->arch.msr;
> +    const struct cpuid_policy *p = d->arch.cpuid;

"p" is clearly not a name helpful to the reader. Why not "cpuid"?

> @@ -379,6 +381,37 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
> uint64_t *val)
>                 _MSR_MISC_FEATURES_CPUID_FAULTING;
>          break;
>  
> +    case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMCS_ENUM:
> +        if ( !nestedhvm_enabled(d) || !p->basic.vmx )
> +            goto gp_fault;
> +        *val = dp->vmx.raw[msr - MSR_IA32_VMX_BASIC];
> +        break;
> +
> +    case MSR_IA32_VMX_PROCBASED_CTLS2:
> +        if ( !dp->vmx.procbased_ctls.allowed_1.activate_secondary_controls )

Here and ...

> +            goto gp_fault;
> +        *val = dp->vmx_procbased_ctls2.raw;
> +        break;
> +
> +    case MSR_IA32_VMX_EPT_VPID_CAP:
> +        if ( !(dp->vmx_procbased_ctls2.allowed_1.enable_ept ||
> +               dp->vmx_procbased_ctls2.allowed_1.enable_vpid) )

... here (and more below) again similar logic to what was there in an
earlier patch? Wouldn't it be better to generically express the
existence of an MSR in the policy (and then perhaps also its
writeability, albeit that may need to be a bitmap instead of a single
flag for the entire register)?

Jan


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