From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
"Wei Liu" <wl@xen.org>, "Jan Beulich" <JBeulich@suse.com>,
"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [Xen-devel] [PATCH 2/4] x86/svm: Don't shadow variables in svm_vmexit_handler()
Date: Wed, 4 Dec 2019 09:43:33 +0000 [thread overview]
Message-ID: <20191204094335.24603-3-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20191204094335.24603-1-andrew.cooper3@citrix.com>
The local variable eventinj is set to the value of vmcb->exitintinfo which is
confusing considering that it isn't vmcb->eventinj. The variable isn't
necessary to begin with, so drop it to avoid confusion.
A local rc variable is shadowed in the CPUID, #DB and #BP handlers.
There is a mix of spelling of inst_len and insn_len, all of which are
logically the same value. Consolidate on insn_len which also matches the name
of the emulation functions for obtaining instruction lengths, and avoid
shadowing it in the CPUID and TASK_SWITCH handlers.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
xen/arch/x86/hvm/svm/svm.c | 63 ++++++++++++++++++++--------------------------
1 file changed, 27 insertions(+), 36 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 518eaefe68..c5ac03b0b1 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2480,8 +2480,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
uint64_t exit_reason;
struct vcpu *v = current;
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
- eventinj_t eventinj;
- int inst_len, rc;
+ int insn_len, rc;
vintr_t intr;
bool_t vcpu_guestmode = 0;
struct vlapic *vlapic = vcpu_vlapic(v);
@@ -2603,11 +2602,10 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
vmcb->cleanbits.bytes = cpu_has_svm_cleanbits ? ~0u : 0u;
/* Event delivery caused this intercept? Queue for redelivery. */
- eventinj = vmcb->exitintinfo;
- if ( unlikely(eventinj.fields.v) &&
- hvm_event_needs_reinjection(eventinj.fields.type,
- eventinj.fields.vector) )
- vmcb->eventinj = eventinj;
+ if ( unlikely(vmcb->exitintinfo.fields.v) &&
+ hvm_event_needs_reinjection(vmcb->exitintinfo.fields.type,
+ vmcb->exitintinfo.fields.vector) )
+ vmcb->eventinj = vmcb->exitintinfo;
switch ( exit_reason )
{
@@ -2630,63 +2628,60 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
case VMEXIT_EXCEPTION_DB:
if ( !v->domain->debugger_attached )
{
- int rc;
unsigned int trap_type;
if ( likely(exit_reason != VMEXIT_ICEBP) )
{
trap_type = X86_EVENTTYPE_HW_EXCEPTION;
- inst_len = 0;
+ insn_len = 0;
}
else
{
trap_type = X86_EVENTTYPE_PRI_SW_EXCEPTION;
- inst_len = svm_get_insn_len(v, INSTR_ICEBP);
+ insn_len = svm_get_insn_len(v, INSTR_ICEBP);
- if ( !inst_len )
+ if ( !insn_len )
break;
}
rc = hvm_monitor_debug(regs->rip,
HVM_MONITOR_DEBUG_EXCEPTION,
- trap_type, inst_len);
+ trap_type, insn_len);
if ( rc < 0 )
goto unexpected_exit_type;
if ( !rc )
hvm_inject_exception(TRAP_debug,
- trap_type, inst_len, X86_EVENT_NO_EC);
+ trap_type, insn_len, X86_EVENT_NO_EC);
}
else
domain_pause_for_debugger();
break;
case VMEXIT_EXCEPTION_BP:
- inst_len = svm_get_insn_len(v, INSTR_INT3);
+ insn_len = svm_get_insn_len(v, INSTR_INT3);
- if ( inst_len == 0 )
+ if ( insn_len == 0 )
break;
if ( v->domain->debugger_attached )
{
/* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
- __update_guest_eip(regs, inst_len);
+ __update_guest_eip(regs, insn_len);
current->arch.gdbsx_vcpu_event = TRAP_int3;
domain_pause_for_debugger();
}
else
{
- int rc;
-
rc = hvm_monitor_debug(regs->rip,
HVM_MONITOR_SOFTWARE_BREAKPOINT,
X86_EVENTTYPE_SW_EXCEPTION,
- inst_len);
+ insn_len);
if ( rc < 0 )
goto unexpected_exit_type;
if ( !rc )
hvm_inject_exception(TRAP_int3,
X86_EVENTTYPE_SW_EXCEPTION,
- inst_len, X86_EVENT_NO_EC);
+ insn_len, X86_EVENT_NO_EC);
}
break;
@@ -2757,7 +2752,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
case VMEXIT_TASK_SWITCH: {
enum hvm_task_switch_reason reason;
- int32_t errcode = -1, insn_len = -1;
+ int32_t errcode = -1;
/*
* All TASK_SWITCH intercepts have fault-like semantics. NRIP is
@@ -2769,6 +2764,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
* to distinguish interrupts/exceptions from instruction based
* switches.
*/
+ insn_len = -1;
if ( vmcb->exitintinfo.fields.v )
{
switch ( vmcb->exitintinfo.fields.type )
@@ -2818,22 +2814,17 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
}
case VMEXIT_CPUID:
- {
- unsigned int inst_len = svm_get_insn_len(v, INSTR_CPUID);
- int rc = 0;
-
- if ( inst_len == 0 )
+ if ( (insn_len = svm_get_insn_len(v, INSTR_CPUID)) == 0 )
break;
- rc = hvm_vmexit_cpuid(regs, inst_len);
+ rc = hvm_vmexit_cpuid(regs, insn_len);
if ( rc < 0 )
goto unexpected_exit_type;
if ( !rc )
- __update_guest_eip(regs, inst_len); /* Safe: CPUID */
-
+ __update_guest_eip(regs, insn_len);
break;
- }
+
case VMEXIT_HLT:
svm_vmexit_do_hlt(vmcb, regs);
break;
@@ -2875,20 +2866,20 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
break;
}
- if ( (inst_len = svm_get_insn_len(v, INSTR_INVLPGA)) == 0 )
+ if ( (insn_len = svm_get_insn_len(v, INSTR_INVLPGA)) == 0 )
break;
svm_invlpga_intercept(v, regs->rax, regs->ecx);
- __update_guest_eip(regs, inst_len);
+ __update_guest_eip(regs, insn_len);
break;
case VMEXIT_VMMCALL:
- if ( (inst_len = svm_get_insn_len(v, INSTR_VMCALL)) == 0 )
+ if ( (insn_len = svm_get_insn_len(v, INSTR_VMCALL)) == 0 )
break;
BUG_ON(vcpu_guestmode);
HVMTRACE_1D(VMMCALL, regs->eax);
if ( hvm_hypercall(regs) == HVM_HCALL_completed )
- __update_guest_eip(regs, inst_len);
+ __update_guest_eip(regs, insn_len);
break;
case VMEXIT_DR0_READ ... VMEXIT_DR7_READ:
@@ -2936,9 +2927,9 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
case VMEXIT_XSETBV:
if ( vmcb_get_cpl(vmcb) )
hvm_inject_hw_exception(TRAP_gp_fault, 0);
- else if ( (inst_len = svm_get_insn_len(v, INSTR_XSETBV)) &&
+ else if ( (insn_len = svm_get_insn_len(v, INSTR_XSETBV)) &&
hvm_handle_xsetbv(regs->ecx, msr_fold(regs)) == X86EMUL_OKAY )
- __update_guest_eip(regs, inst_len);
+ __update_guest_eip(regs, insn_len);
break;
case VMEXIT_NPF:
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-12-04 9:44 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-04 9:43 [Xen-devel] [PATCH 0/4] x86/svm: (Post TASK_SWITCH) cleanup Andrew Cooper
2019-12-04 9:43 ` [Xen-devel] [PATCH 1/4] x86/svm: Clean up construct_vmcb() Andrew Cooper
2019-12-04 10:06 ` Jan Beulich
2019-12-04 19:21 ` Andrew Cooper
2019-12-04 9:43 ` Andrew Cooper [this message]
2019-12-04 10:10 ` [Xen-devel] [PATCH 2/4] x86/svm: Don't shadow variables in svm_vmexit_handler() Jan Beulich
2019-12-04 9:43 ` [Xen-devel] [PATCH 3/4] x86/svm: Clean up intinfo_t variables Andrew Cooper
2019-12-04 10:19 ` Jan Beulich
2019-12-04 19:22 ` Andrew Cooper
2019-12-04 19:38 ` Andrew Cooper
2019-12-05 9:11 ` Jan Beulich
2019-12-05 12:33 ` Andrew Cooper
2019-12-04 9:43 ` [Xen-devel] [PATCH 4/4] x86/svm: Use named (bit)fields for task switch exit info Andrew Cooper
2019-12-04 10:24 ` Jan Beulich
2019-12-04 20:04 ` Andrew Cooper
2019-12-05 9:05 ` Jan Beulich
2019-12-05 10:51 ` [Xen-devel] [PATCH 5/4] x86/svm: Minor cleanup to start_svm() Andrew Cooper
2019-12-05 10:53 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191204094335.24603-3-andrew.cooper3@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=roger.pau@citrix.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).