Re: [PATCH 3/3] x86/HVM: don't crash guest upon problems occurring in user mode

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4952 bytes --]

On 20/11/14 10:13, Jan Beulich wrote:
> This extends commit 5283b310 ("x86/HVM: only kill guest when unknown VM
> exit occurred in guest kernel mode") to further cases, including the
> failed VM entry one that XSA-110 was needed to be issued for.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -90,6 +90,15 @@ static bool_t amd_erratum383_found __rea
>  static uint64_t osvw_length, osvw_status;
>  static DEFINE_SPINLOCK(osvw_lock);
>  
> +/* Only crash the guest if the problem originates in kernel mode. */
> +static void svm_crash_or_gp(struct vcpu *v)
> +{
> +    if ( vmcb_get_cpl(v->arch.hvm_svm.vmcb) )
> +        hvm_inject_hw_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE);

This (and its VMX counterpart) should either deliver a #GP fault, or
have its name changed to not imply a #GP fault.

How about "crash_or_fault()" as an alternative which is slightly less
specific?

~Andrew

> +    else
> +        domain_crash(v->domain);
> +}
> +
>  void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len)
>  {
>      struct vcpu *curr = current;
> @@ -100,7 +109,7 @@ void __update_guest_eip(struct cpu_user_
>      if ( unlikely(inst_len > 15) )
>      {
>          gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len);
> -        domain_crash(curr->domain);
> +        svm_crash_or_gp(curr);
>          return;
>      }
>  
> @@ -1505,7 +1514,7 @@ static void svm_do_nested_pgfault(struct
>      gdprintk(XENLOG_ERR,
>           "SVM violation gpa %#"PRIpaddr", mfn %#lx, type %i\n",
>           gpa, mfn_x(mfn), p2mt);
> -    domain_crash(v->domain);
> +    svm_crash_or_gp(v);
>  }
>  
>  static void svm_fpu_dirty_intercept(void)
> @@ -2647,7 +2656,7 @@ void svm_vmexit_handler(struct cpu_user_
>              printk(XENLOG_G_ERR
>                     "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
>                     v, rc, vmcb->exitinfo2, vmcb->exitinfo1);
> -            domain_crash(v->domain);
> +            svm_crash_or_gp(v);
>          }
>          v->arch.hvm_svm.cached_insn_len = 0;
>          break;
> @@ -2680,11 +2689,7 @@ void svm_vmexit_handler(struct cpu_user_
>                   "exitinfo1 = %#"PRIx64", exitinfo2 = %#"PRIx64"\n",
>                   exit_reason, 
>                   (u64)vmcb->exitinfo1, (u64)vmcb->exitinfo2);
> -        if ( vmcb_get_cpl(vmcb) )
> -            hvm_inject_hw_exception(TRAP_invalid_op,
> -                                    HVM_DELIVER_NO_ERROR_CODE);
> -        else
> -            domain_crash(v->domain);
> +        svm_crash_or_gp(v);
>          break;
>      }
>  
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -134,6 +134,18 @@ static void vmx_vcpu_destroy(struct vcpu
>      passive_domain_destroy(v);
>  }
>  
> +/* Only crash the guest if the problem originates in kernel mode. */
> +static void vmx_crash_or_gp(struct vcpu *v)
> +{
> +    struct segment_register ss;
> +
> +    vmx_get_segment_register(v, x86_seg_ss, &ss);
> +    if ( ss.attr.fields.dpl )
> +        hvm_inject_hw_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE);
> +    else
> +        domain_crash(v->domain);
> +}
> +
>  static DEFINE_PER_CPU(struct vmx_msr_state, host_msr_state);
>  
>  static const u32 msr_index[] =
> @@ -2474,7 +2486,7 @@ static void ept_handle_violation(unsigne
>      if ( qualification & EPT_GLA_VALID )
>          gdprintk(XENLOG_ERR, " --- GLA %#lx\n", gla);
>  
> -    domain_crash(d);
> +    vmx_crash_or_gp(current);
>  }
>  
>  static void vmx_failed_vmentry(unsigned int exit_reason,
> @@ -2508,7 +2520,7 @@ static void vmx_failed_vmentry(unsigned 
>      vmcs_dump_vcpu(curr);
>      printk("**************************************\n");
>  
> -    domain_crash(curr->domain);
> +    vmx_crash_or_gp(curr);
>  }
>  
>  void vmx_enter_realmode(struct cpu_user_regs *regs)
> @@ -3161,19 +3173,8 @@ void vmx_vmexit_handler(struct cpu_user_
>      /* fall through */
>      default:
>      exit_and_crash:
> -        {
> -            struct segment_register ss;
> -
> -            gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n",
> -                     exit_reason);
> -
> -            vmx_get_segment_register(v, x86_seg_ss, &ss);
> -            if ( ss.attr.fields.dpl )
> -                hvm_inject_hw_exception(TRAP_invalid_op,
> -                                        HVM_DELIVER_NO_ERROR_CODE);
> -            else
> -                domain_crash(v->domain);
> -        }
> +        gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n", exit_reason);
> +        vmx_crash_or_gp(v);
>          break;
>      }
>  
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 5497 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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