* [PATCH] x86/HVM: prevent infinite VM entry retries
@ 2014-11-27 12:39 Jan Beulich
2014-11-27 12:46 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2014-11-27 12:39 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Tim Deegan, Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 4502 bytes --]
This reverts the VMX side of commit 28b4baac ("x86/HVM: don't crash
guest upon problems occurring in user mode") and gets SVM in line with
the resulting VMX behavior. This is because Andrew validly says
"A failed vmentry is overwhelmingly likely to be caused by corrupt
VMC[SB] state. As a result, injecting a fault and retrying the the
vmentry is likely to fail in the same way."
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Tim Deegan <tim@xen.org>
---
v2: Replace SVM change by what Tim suggested.
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2371,8 +2371,8 @@ void svm_vmexit_handler(struct cpu_user_
goto out;
case NESTEDHVM_VMEXIT_FATALERROR:
gdprintk(XENLOG_ERR, "unexpected nestedsvm_vmexit() error\n");
- goto exit_and_crash;
-
+ domain_crash(v->domain);
+ goto out;
default:
BUG();
case NESTEDHVM_VMEXIT_ERROR:
@@ -2385,18 +2385,21 @@ void svm_vmexit_handler(struct cpu_user_
case NESTEDHVM_VMEXIT_FATALERROR:
gdprintk(XENLOG_ERR,
"unexpected nestedsvm_check_intercepts() error\n");
- goto exit_and_crash;
+ domain_crash(v->domain);
+ goto out;
default:
gdprintk(XENLOG_INFO, "nestedsvm_check_intercepts() returned %i\n",
nsret);
- goto exit_and_crash;
+ domain_crash(v->domain);
+ goto out;
}
}
if ( unlikely(exit_reason == VMEXIT_INVALID) )
{
svm_vmcb_dump(__func__, vmcb);
- goto exit_and_crash;
+ domain_crash(v->domain);
+ goto out;
}
perfc_incra(svmexits, exit_reason);
@@ -2431,13 +2434,13 @@ void svm_vmexit_handler(struct cpu_user_
case VMEXIT_EXCEPTION_DB:
if ( !v->domain->debugger_attached )
- goto exit_and_crash;
+ goto unexpected_exit_type;
domain_pause_for_debugger();
break;
case VMEXIT_EXCEPTION_BP:
if ( !v->domain->debugger_attached )
- goto exit_and_crash;
+ goto unexpected_exit_type;
/* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
break;
@@ -2684,7 +2687,7 @@ void svm_vmexit_handler(struct cpu_user_
break;
default:
- exit_and_crash:
+ unexpected_exit_type:
gdprintk(XENLOG_ERR, "unexpected VMEXIT: exit reason = %#"PRIx64", "
"exitinfo1 = %#"PRIx64", exitinfo2 = %#"PRIx64"\n",
exit_reason,
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -134,18 +134,6 @@ 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_fault(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[] =
@@ -2520,7 +2508,7 @@ static void vmx_failed_vmentry(unsigned
vmcs_dump_vcpu(curr);
printk("**************************************\n");
- vmx_crash_or_fault(curr);
+ domain_crash(curr->domain);
}
void vmx_enter_realmode(struct cpu_user_regs *regs)
@@ -3173,8 +3161,19 @@ void vmx_vmexit_handler(struct cpu_user_
/* fall through */
default:
exit_and_crash:
- gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n", exit_reason);
- vmx_crash_or_fault(v);
+ {
+ 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);
+ }
break;
}
[-- Attachment #2: x86-HVM-prevent-infinite-VMentry-loop.patch --]
[-- Type: text/plain, Size: 4544 bytes --]
x86/HVM: prevent infinite VM entry retries
This reverts the VMX side of commit 28b4baac ("x86/HVM: don't crash
guest upon problems occurring in user mode") and gets SVM in line with
the resulting VMX behavior. This is because Andrew validly says
"A failed vmentry is overwhelmingly likely to be caused by corrupt
VMC[SB] state. As a result, injecting a fault and retrying the the
vmentry is likely to fail in the same way."
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Tim Deegan <tim@xen.org>
---
v2: Replace SVM change by what Tim suggested.
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2371,8 +2371,8 @@ void svm_vmexit_handler(struct cpu_user_
goto out;
case NESTEDHVM_VMEXIT_FATALERROR:
gdprintk(XENLOG_ERR, "unexpected nestedsvm_vmexit() error\n");
- goto exit_and_crash;
-
+ domain_crash(v->domain);
+ goto out;
default:
BUG();
case NESTEDHVM_VMEXIT_ERROR:
@@ -2385,18 +2385,21 @@ void svm_vmexit_handler(struct cpu_user_
case NESTEDHVM_VMEXIT_FATALERROR:
gdprintk(XENLOG_ERR,
"unexpected nestedsvm_check_intercepts() error\n");
- goto exit_and_crash;
+ domain_crash(v->domain);
+ goto out;
default:
gdprintk(XENLOG_INFO, "nestedsvm_check_intercepts() returned %i\n",
nsret);
- goto exit_and_crash;
+ domain_crash(v->domain);
+ goto out;
}
}
if ( unlikely(exit_reason == VMEXIT_INVALID) )
{
svm_vmcb_dump(__func__, vmcb);
- goto exit_and_crash;
+ domain_crash(v->domain);
+ goto out;
}
perfc_incra(svmexits, exit_reason);
@@ -2431,13 +2434,13 @@ void svm_vmexit_handler(struct cpu_user_
case VMEXIT_EXCEPTION_DB:
if ( !v->domain->debugger_attached )
- goto exit_and_crash;
+ goto unexpected_exit_type;
domain_pause_for_debugger();
break;
case VMEXIT_EXCEPTION_BP:
if ( !v->domain->debugger_attached )
- goto exit_and_crash;
+ goto unexpected_exit_type;
/* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
break;
@@ -2684,7 +2687,7 @@ void svm_vmexit_handler(struct cpu_user_
break;
default:
- exit_and_crash:
+ unexpected_exit_type:
gdprintk(XENLOG_ERR, "unexpected VMEXIT: exit reason = %#"PRIx64", "
"exitinfo1 = %#"PRIx64", exitinfo2 = %#"PRIx64"\n",
exit_reason,
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -134,18 +134,6 @@ 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_fault(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[] =
@@ -2520,7 +2508,7 @@ static void vmx_failed_vmentry(unsigned
vmcs_dump_vcpu(curr);
printk("**************************************\n");
- vmx_crash_or_fault(curr);
+ domain_crash(curr->domain);
}
void vmx_enter_realmode(struct cpu_user_regs *regs)
@@ -3173,8 +3161,19 @@ void vmx_vmexit_handler(struct cpu_user_
/* fall through */
default:
exit_and_crash:
- gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n", exit_reason);
- vmx_crash_or_fault(v);
+ {
+ 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);
+ }
break;
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] x86/HVM: prevent infinite VM entry retries
2014-11-27 12:39 [PATCH] x86/HVM: prevent infinite VM entry retries Jan Beulich
@ 2014-11-27 12:46 ` Andrew Cooper
2014-11-27 12:58 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2014-11-27 12:46 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Tim Deegan, Keir Fraser
On 27/11/14 12:39, Jan Beulich wrote:
> This reverts the VMX side of commit 28b4baac ("x86/HVM: don't crash
> guest upon problems occurring in user mode") and gets SVM in line with
> the resulting VMX behavior. This is because Andrew validly says
>
> "A failed vmentry is overwhelmingly likely to be caused by corrupt
> VMC[SB] state. As a result, injecting a fault and retrying the the
> vmentry is likely to fail in the same way."
>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Tim Deegan <tim@xen.org>
> ---
> v2: Replace SVM change by what Tim suggested.
>
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2371,8 +2371,8 @@ void svm_vmexit_handler(struct cpu_user_
> goto out;
> case NESTEDHVM_VMEXIT_FATALERROR:
> gdprintk(XENLOG_ERR, "unexpected nestedsvm_vmexit() error\n");
> - goto exit_and_crash;
> -
> + domain_crash(v->domain);
> + goto out;
> default:
> BUG();
> case NESTEDHVM_VMEXIT_ERROR:
> @@ -2385,18 +2385,21 @@ void svm_vmexit_handler(struct cpu_user_
> case NESTEDHVM_VMEXIT_FATALERROR:
> gdprintk(XENLOG_ERR,
> "unexpected nestedsvm_check_intercepts() error\n");
> - goto exit_and_crash;
> + domain_crash(v->domain);
> + goto out;
> default:
> gdprintk(XENLOG_INFO, "nestedsvm_check_intercepts() returned %i\n",
> nsret);
> - goto exit_and_crash;
> + domain_crash(v->domain);
> + goto out;
> }
> }
>
> if ( unlikely(exit_reason == VMEXIT_INVALID) )
> {
I think it would be good to retain the printk here from previous
versions of the patch, specifically identifies the below vmcb dump as
caused by a failed vmentry. As it stands, it is a vmcb dump with no
other context.
Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> svm_vmcb_dump(__func__, vmcb);
> - goto exit_and_crash;
> + domain_crash(v->domain);
> + goto out;
> }
>
> perfc_incra(svmexits, exit_reason);
> @@ -2431,13 +2434,13 @@ void svm_vmexit_handler(struct cpu_user_
>
> case VMEXIT_EXCEPTION_DB:
> if ( !v->domain->debugger_attached )
> - goto exit_and_crash;
> + goto unexpected_exit_type;
> domain_pause_for_debugger();
> break;
>
> case VMEXIT_EXCEPTION_BP:
> if ( !v->domain->debugger_attached )
> - goto exit_and_crash;
> + goto unexpected_exit_type;
> /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
> if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
> break;
> @@ -2684,7 +2687,7 @@ void svm_vmexit_handler(struct cpu_user_
> break;
>
> default:
> - exit_and_crash:
> + unexpected_exit_type:
> gdprintk(XENLOG_ERR, "unexpected VMEXIT: exit reason = %#"PRIx64", "
> "exitinfo1 = %#"PRIx64", exitinfo2 = %#"PRIx64"\n",
> exit_reason,
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -134,18 +134,6 @@ 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_fault(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[] =
> @@ -2520,7 +2508,7 @@ static void vmx_failed_vmentry(unsigned
> vmcs_dump_vcpu(curr);
> printk("**************************************\n");
>
> - vmx_crash_or_fault(curr);
> + domain_crash(curr->domain);
> }
>
> void vmx_enter_realmode(struct cpu_user_regs *regs)
> @@ -3173,8 +3161,19 @@ void vmx_vmexit_handler(struct cpu_user_
> /* fall through */
> default:
> exit_and_crash:
> - gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n", exit_reason);
> - vmx_crash_or_fault(v);
> + {
> + 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);
> + }
> break;
> }
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] x86/HVM: prevent infinite VM entry retries
2014-11-27 12:46 ` Andrew Cooper
@ 2014-11-27 12:58 ` Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2014-11-27 12:58 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Tim Deegan, Keir Fraser
>>> On 27.11.14 at 13:46, <andrew.cooper3@citrix.com> wrote:
> On 27/11/14 12:39, Jan Beulich wrote:
>> if ( unlikely(exit_reason == VMEXIT_INVALID) )
>> {
>
> I think it would be good to retain the printk here from previous
> versions of the patch, specifically identifies the below vmcb dump as
> caused by a failed vmentry. As it stands, it is a vmcb dump with no
> other context.
>
> Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
I meant to do so and then forgot. Added to the to be committed
version.
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-27 12:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-27 12:39 [PATCH] x86/HVM: prevent infinite VM entry retries Jan Beulich
2014-11-27 12:46 ` Andrew Cooper
2014-11-27 12:58 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.