Re: [Xen-devel] [PATCH] x86/Intel: virtualize support for cpuid faulting

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.10.16 at 06:07, <me@xxxxxxxxxxxx> wrote:
> ---

In addition to what Andrew said: Please version your patch, and please
add a short summary of what changed compared to the previous version
here.

> @@ -2931,6 +2941,11 @@ static int vmx_msr_write_intercept(unsigned int msr, 
> uint64_t msr_content)
>               rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
>              goto gp_fault;
>          break;
> +    case MSR_INTEL_MISC_FEATURES_ENABLES:

Blank line ahead of this one please.

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -2945,6 +2945,16 @@ static int emulate_privileged_op(struct cpu_user_regs 
> *regs)
>                   rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
>                  goto fail;
>              break;
> +        case MSR_INTEL_MISC_FEATURES_ENABLES:

Same here. Plus you need to re-base. The code you modify has
changed quite a bit.

> +            if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> +                 rdmsr_safe(MSR_INTEL_MISC_FEATURES_ENABLES, val) ||
> +                 msr_content & ~MSR_MISC_FEATURES_CPUID_FAULTING )

Please parenthesize the operands of &. Also I think the msr_content
check would better go ahead of the rdmsr_safe().

> +                goto fail;
> +            if ( msr_content == MSR_MISC_FEATURES_CPUID_FAULTING &&

Please use & instead of ==, so the line won't need to change once
further bits here get emulated.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel