RE: [PATCH v4 2/5] x86/pv: allow reading FEATURE_CONTROL MSR

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> Sent: Monday, September 7, 2020 6:32 PM
> 
> Linux PV guests will attempt to read the FEATURE_CONTROL MSR, so move
> the handling done in VMX code into guest_rdmsr as it can be shared
> between PV and HVM guests that way.
> 
> Note that there's a slight behavior change and attempting to read the
> MSR when no features are available will result in a fault.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> ---
> Changes since v3:
>  - Only allow reading the MSR when there are bits available (different
>    than bit 0).
> 
> Changes from v1:
>  - Move the VMX implementation into guest_rdmsr.
> ---
>  xen/arch/x86/hvm/vmx/vmx.c |  8 +-------
>  xen/arch/x86/msr.c         | 12 ++++++++++++
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index c4b40bf3cb..709ea149d1 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2980,13 +2980,7 @@ static int vmx_msr_read_intercept(unsigned int
> msr, uint64_t *msr_content)
>      case MSR_IA32_DEBUGCTLMSR:
>          __vmread(GUEST_IA32_DEBUGCTL, msr_content);
>          break;
> -    case MSR_IA32_FEATURE_CONTROL:
> -        *msr_content = IA32_FEATURE_CONTROL_LOCK;
> -        if ( vmce_has_lmce(curr) )
> -            *msr_content |= IA32_FEATURE_CONTROL_LMCE_ON;
> -        if ( nestedhvm_enabled(curr->domain) )
> -            *msr_content |=
> IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX;
> -        break;
> +
>      case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_VMFUNC:
>          if ( !nvmx_msr_read_intercept(msr, msr_content) )
>              goto gp_fault;
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index 74bf7d9589..79fbb9e940 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -25,6 +25,7 @@
>  #include <xen/sched.h>
> 
>  #include <asm/debugreg.h>
> +#include <asm/hvm/nestedhvm.h>
>  #include <asm/hvm/viridian.h>
>  #include <asm/msr.h>
>  #include <asm/setup.h>
> @@ -197,6 +198,17 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr,
> uint64_t *val)
>          /* Not offered to guests. */
>          goto gp_fault;
> 
> +    case MSR_IA32_FEATURE_CONTROL:
> +        if ( !cp->basic.vmx && !vmce_has_lmce(v) )
> +            goto gp_fault;
> +
> +        *val = IA32_FEATURE_CONTROL_LOCK;
> +        if ( vmce_has_lmce(v) )
> +            *val |= IA32_FEATURE_CONTROL_LMCE_ON;
> +        if ( cp->basic.vmx )
> +            *val |= IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX;
> +        break;
> +
>      case MSR_IA32_PLATFORM_ID:
>          if ( !(cp->x86_vendor & X86_VENDOR_INTEL) ||
>               !(boot_cpu_data.x86_vendor & X86_VENDOR_INTEL) )
> --
> 2.28.0