From patchwork Wed Dec 4 09:43:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 11272591 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6210B1593 for ; Wed, 4 Dec 2019 09:44:58 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3215A20862 for ; Wed, 4 Dec 2019 09:44:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="U5gv69CR" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3215A20862 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=citrix.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1icRCL-000409-7M; Wed, 04 Dec 2019 09:43:53 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1icRCJ-0003z8-Op for xen-devel@lists.xenproject.org; Wed, 04 Dec 2019 09:43:51 +0000 X-Inumbo-ID: 908d9bba-167a-11ea-a0d2-bc764e2007e4 Received: from esa1.hc3370-68.iphmx.com (unknown [216.71.145.142]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 908d9bba-167a-11ea-a0d2-bc764e2007e4; Wed, 04 Dec 2019 09:43:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1575452626; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=WaRplA4Jo//vuo/88onMn4U2A7fRmaduLO/pNyr3jcg=; b=U5gv69CR7w/qe6yTZS71mNcGZsEeKeZvJTkCpCdtFr7U/gUOJK01Mxzz /Fgoi0CH+nxoOn77fwYPOhY9dN0q9o9StEK3tRDzrSKWpDY+O2vtsq2au yvvwECq+tj2Pw8DJoBXQIMTvcFlffsp77y5YL3rg2lwBBV/MFR/vygxP5 g=; Authentication-Results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=andrew.cooper3@citrix.com; spf=Pass smtp.mailfrom=Andrew.Cooper3@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of andrew.cooper3@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="andrew.cooper3@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa1.hc3370-68.iphmx.com: domain of Andrew.Cooper3@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="Andrew.Cooper3@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: m5V7+y5ldEWTDlx7NKSmAqX85dRx5FiE5Gp3HyyxjWFdNpopMp22Ceyj1JorIZiXRh2FfendGF 2CqOCWWZEB7FYxY/7nnV5FwM1dsJ44LSjAx0SwMx3AR0TCMmAN4OZurDSOfwe3TjJPSCr27JN1 gg+EF0fVltxQJ3glnfFfOQMfUNcGu5DzZqFfHGSqyH4juzoJ/JuYbkURg8CjISLcTYVpdxohCv mXzEhLqgO4lr0HLggexVBT6PEEjISraLDBX1yb/s0A9NS0yXUW84DHASmDFdHkrhu6w2L3W2mp dBQ= X-SBRS: 2.7 X-MesageID: 9301708 X-Ironport-Server: esa1.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.69,277,1571716800"; d="scan'208";a="9301708" From: Andrew Cooper To: Xen-devel Date: Wed, 4 Dec 2019 09:43:33 +0000 Message-ID: <20191204094335.24603-3-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20191204094335.24603-1-andrew.cooper3@citrix.com> References: <20191204094335.24603-1-andrew.cooper3@citrix.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH 2/4] x86/svm: Don't shadow variables in svm_vmexit_handler() X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Andrew Cooper , Wei Liu , Jan Beulich , =?utf-8?q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" 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 Reviewed-by: Jan Beulich --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné --- 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: