Re: [PATCH v2 5/6] x86/hvm: Forced Emulation Prefix for debug builds of Xen

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 09/23/2014 12:09 PM, Andrew Cooper wrote:
> Analysis of XSAs 105 and 106 show that is possible to force a race condition
> which causes any arbitrary instruction to be emulated.
>
> To aid testing, explicitly introduce the Forced Emulation Prefix for debug
> builds alone.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> CC: Jun Nakajima <jun.nakajima@intel.com>
> CC: Eddie Dong <eddie.dong@intel.com>
> CC: Kevin Tian <kevin.tian@intel.com>
>
> ---
> v2: (all suggested by Jan)
>   * Use hvm_fetch_from_guest_virt_nofault() in preference to copy_from_guest()
>   * Vastly reduce use of #ifndef NDEBUG
> ---
>   docs/misc/xen-command-line.markdown |   11 +++++++++++
>   xen/arch/x86/hvm/hvm.c              |    5 +++++
>   xen/arch/x86/hvm/svm/svm.c          |   13 +++++++++++++
>   xen/arch/x86/hvm/vmx/vmx.c          |   13 +++++++++++++
>   xen/include/asm-x86/hvm/hvm.h       |    7 +++++++
>   5 files changed, 49 insertions(+)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index af93e17..389701a 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -682,6 +682,17 @@ Bit 11 - MSR operation logging
>   
>   Recognized in debug builds of the hypervisor only.
>   
> +### hvm\_fep
> +> `= <boolean>`
> +
> +> Default: `false`
> +
> +Allow use of the Forced Emulation Prefix in HVM guests, to allow emulation of
> +arbitrary instructions.
> +
> +This option is intended for development purposes, and is only available in
> +debug builds of the hypervisor.
> +
>   ### hvm\_port80
>   > `= <boolean>`
>   
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 5c7e0a4..34f28d0 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -86,6 +86,11 @@ unsigned long __attribute__ ((__section__ (".bss.page_aligned")))
>   static bool_t __initdata opt_hap_enabled = 1;
>   boolean_param("hap", opt_hap_enabled);
>   
> +#ifndef opt_hvm_fep
> +bool_t opt_hvm_fep;
> +boolean_param("hvm_fep", opt_hvm_fep);
> +#endif
> +
>   static int cpu_callback(
>       struct notifier_block *nfb, unsigned long action, void *hcpu)
>   {
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index b6beefc..cda968b 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2118,6 +2118,19 @@ static void svm_vmexit_ud_intercept(struct cpu_user_regs *regs)
>       struct hvm_emulate_ctxt ctxt;
>       int rc;
>   
> +    if ( opt_hvm_fep )
> +    {
> +        char sig[5]; /* ud2; .ascii "xen" */
> +
> +        if ( (hvm_fetch_from_guest_virt_nofault(
> +                  sig, regs->eip, sizeof(sig), 0) == HVMCOPY_okay) &&
> +             (memcmp(sig, "\xf\xbxen", sizeof(sig)) == 0) )
> +        {
> +            regs->eip += sizeof(sig);
> +            regs->eflags &= ~X86_EFLAGS_RF;
> +        }
> +    }

This code is exactly the same for SVM and VMX. Can it be factored out?

-boris

> +
>       hvm_emulate_prepare(&ctxt, regs);
>   
>       rc = hvm_emulate_one(&ctxt);
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index addaa81..7f02ba2 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2499,6 +2499,19 @@ static void vmx_vmexit_ud_intercept(struct cpu_user_regs *regs)
>       struct hvm_emulate_ctxt ctxt;
>       int rc;
>   
> +    if ( opt_hvm_fep )
> +    {
> +        char sig[5]; /* ud2; .ascii "xen" */
> +
> +        if ( (hvm_fetch_from_guest_virt_nofault(
> +                  sig, regs->eip, sizeof(sig), 0) == HVMCOPY_okay) &&
> +             (memcmp(sig, "\xf\xbxen", sizeof(sig)) == 0) )
> +        {
> +            regs->eip += sizeof(sig);
> +            regs->eflags &= ~X86_EFLAGS_RF;
> +        }
> +    }
> +
>       hvm_emulate_prepare(&ctxt, regs);
>   
>       rc = hvm_emulate_one(&ctxt);
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 3e66276..c0fbc8b 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -517,6 +517,13 @@ bool_t nhvm_vmcx_hap_enabled(struct vcpu *v);
>   /* interrupt */
>   enum hvm_intblk nhvm_interrupt_blocked(struct vcpu *v);
>   
> +#ifndef NDEBUG
> +/* Permit use of the Forced Emulation Prefix in HVM guests */
> +extern bool_t opt_hvm_fep;
> +#else
> +#define opt_hvm_fep 0
> +#endif
> +
>   #endif /* __ASM_X86_HVM_HVM_H__ */
>   
>   /*