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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/10/16 20:47, Kyle Huey wrote:
> On HVM guests, the cpuid triggers a vm exit, so we can check the emulated
> faulting state in vmx_do_cpuid and inject a GP(0) if CPL > 0. Notably no
> hardware support for faulting on cpuid is necessary to emulate support with an
> HVM guest.
>
> On PV guests, hardware support is required so that userspace cpuid will trap
> to xen. Xen already enables cpuid faulting on supported CPUs for pv guests 
> (that

to Xen.

> aren't the control domain, see the comment in intel_ctxt_switch_levelling).
> Every PV guest cpuid will trap via a GP(0) to emulate_privileged_op (via
> do_general_protection). Once there we simply decline to emulate cpuid if the
> CPL > 0 and faulting is enabled, leaving the GP(0) for the guest kernel to
> handle.
>
> Signed-off-by: Kyle Huey <khuey@xxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/vmx/vmx.c   | 24 ++++++++++++++++++++++--
>  xen/arch/x86/traps.c         | 34 ++++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/domain.h |  3 +++
>  3 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index b9102ce..c038393 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2427,16 +2427,25 @@ static void vmx_cpuid_intercept(
>  
>      HVMTRACE_5D (CPUID, input, *eax, *ebx, *ecx, *edx);
>  }
>  
>  static int vmx_do_cpuid(struct cpu_user_regs *regs)
>  {
>      unsigned int eax, ebx, ecx, edx;
>      unsigned int leaf, subleaf;
> +    struct segment_register sreg;
> +    struct vcpu *v = current;
> +
> +    hvm_get_segment_register(v, x86_seg_ss, &sreg);
> +    if ( v->arch.cpuid_fault && sreg.attr.fields.dpl > 0 )
> +    {
> +        hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +        return 1; /* Don't advance the guest IP! */
> +    }

Thinking about it, the segment register query can be skipped in the
likely case that faulting isn't enabled.  Could this be re-arranged to:

if ( v->arch.cpuid_fault )
{
    struct segment_register sreg;

    hvm_get_segment_register(v, x86_seg_ss, &sreg);
    if ( sreg.attr.fields.dpl > 0 )
    {
        hvm_inject_hw_exception(TRAP_gp_fault, 0);
        return 1; /* Don't advance the guest IP! */
    }
}

With these two minor issues taken care of, Reviewed-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>

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