Re: [PATCH v3 7/8] x86/hvm: Disallow access to unknown MSRs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.09.2020 12:54, Roger Pau Monne wrote:
> @@ -3290,11 +3288,6 @@ static int vmx_msr_write_intercept(unsigned int msr, 
> uint64_t msr_content)
>          __vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
>          break;
>  
> -    case MSR_IA32_FEATURE_CONTROL:
> -    case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
> -        /* None of these MSRs are writeable. */
> -        goto gp_fault;

I understand Andrew did ask for this (and I didn't look closely
when I saw the comment), but ...

> @@ -3320,10 +3313,9 @@ static int vmx_msr_write_intercept(unsigned int msr, 
> uint64_t msr_content)
>               is_last_branch_msr(msr) )
>              break;
>  
> -        /* Match up with the RDMSR side; ultimately this should go away. */
> -        if ( rdmsr_safe(msr, msr_content) == 0 )
> -            break;
> -
> +        gdprintk(XENLOG_WARNING,
> +                 "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n",
> +                 msr, msr_content);
>          goto gp_fault;

... above from here is logic that handling of these MSRs now goes
through. I'm particularly worried about vmx_write_guest_msr(),
which blindly updates the value of any MSR it can find, i.e. if
any r/o MSR (from the set above, or even more generally) ever got
added to this vCPU-specific set, the r/o-ness would no longer be
maintained. Do we perhaps need to white-list MSRs for which
vmx_write_guest_msr() may get called here?

Jan