Re: [PATCH v6 03/11] x86/vmx: add IPT cpu feature

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jul 07, 2020 at 09:39:42PM +0200, Michał Leszczyński wrote:
> From: Michal Leszczynski <michal.leszczynski@xxxxxxx>
> 
> Check if Intel Processor Trace feature is supported by current
> processor. Define vmtrace_supported global variable.

IIRC there was some discussion in previous versions about whether
vmtrace_supported should be globally exposed to all arches, since I
see the symbol is defined in a common file I assume Arm maintainers
agree to this approach, and hence it would be helpful to add a note to
the commit message, ie:

"The vmtrace_supported global variable is defined in common code with
the expectation that other arches also supporting processor tracing
can make use of it."

> Signed-off-by: Michal Leszczynski <michal.leszczynski@xxxxxxx>

LGTM:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> ---
>  xen/arch/x86/hvm/vmx/vmcs.c                 | 15 ++++++++++++++-
>  xen/common/domain.c                         |  2 ++
>  xen/include/asm-x86/cpufeature.h            |  1 +
>  xen/include/asm-x86/hvm/vmx/vmcs.h          |  1 +
>  xen/include/public/arch-x86/cpufeatureset.h |  1 +
>  xen/include/xen/domain.h                    |  2 ++
>  6 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index ca94c2bedc..3a53553f10 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -291,6 +291,20 @@ static int vmx_init_vmcs_config(void)
>          _vmx_cpu_based_exec_control &=
>              ~(CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING);
>  
> +    rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
> +
> +    /* Check whether IPT is supported in VMX operation. */
> +    if ( !smp_processor_id() )
> +        vmtrace_supported = cpu_has_ipt &&
> +                            (_vmx_misc_cap & VMX_MISC_PROC_TRACE);

Andrew also suggested to set vmtrace_supported to the value of
cpu_has_ipt during CPU identification and then clear it here if VMX
doesn't support IPT. Since this implementation won't add support for
PV guests I'm not specially trilled and I think this approach is fine
for the time being. If/When PV support is added we will have to
re-arrange this a bit.

Thanks.