From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: [Patch v3] x86/traps: improvements to {rd, wr}msr_hypervisor_regs() Date: Mon, 7 Oct 2013 13:01:10 +0100 Message-ID: <1381147270-15890-1-git-send-email-andrew.cooper3@citrix.com> References: <5252924E.9020707@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5252924E.9020707@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Xen-devel Cc: Andrew Cooper , Keir Fraser , Jan Beulich List-Id: xen-devel@lists.xenproject.org Coverity ID: 1055249 1055250 Coverity was complaining that the switch statments contained dead code in their default statements. While this is quite minor, the code flow in wrmsr_hypervisor_regs() was sufficiently opaque that I felt it approprate to fix. Other improvements include: * not shadowing the function parameter 'idx'. * use of PAGE_SHIFT instead of opencoded numbers. * a more descriptive error message for attempting to write invalid indicies for hypercall pages. There is no behavioural change as a result. Signed-off-by: Andrew Cooper CC: Keir Fraser CC: Jan Beulich --- xen/arch/x86/traps.c | 44 ++++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 6c7bd99..3bb62f0 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -595,55 +595,50 @@ DO_ERROR_NOCODE(TRAP_copro_error, coprocessor_error) DO_ERROR( TRAP_alignment_check, alignment_check) DO_ERROR_NOCODE(TRAP_simd_error, simd_coprocessor_error) +/* + * Returns 0 if not handled, and non-0 for error. (The calling semantics are + * in need of some work) + */ int rdmsr_hypervisor_regs(uint32_t idx, uint64_t *val) { struct domain *d = current->domain; /* Optionally shift out of the way of Viridian architectural MSRs. */ uint32_t base = is_viridian_domain(d) ? 0x40000200 : 0x40000000; - idx -= base; - if ( idx > 0 ) - return 0; - - switch ( idx ) + switch ( idx - base ) { - case 0: + case 0: /* Write hypercall page. Reads are invalid. Hand a #GP back. */ { *val = 0; - break; + return 1; } - default: - BUG(); } - return 1; + return 0; } +/* Returns 1 if handled, 0 if not and -Exx for error. */ int wrmsr_hypervisor_regs(uint32_t idx, uint64_t val) { struct domain *d = current->domain; /* Optionally shift out of the way of Viridian architectural MSRs. */ uint32_t base = is_viridian_domain(d) ? 0x40000200 : 0x40000000; - idx -= base; - if ( idx > 0 ) - return 0; - - switch ( idx ) + switch ( idx - base ) { - case 0: + case 0: /* Write hypercall page */ { void *hypercall_page; - unsigned long gmfn = val >> 12; - unsigned int idx = val & 0xfff; + unsigned long gmfn = val >> PAGE_SHIFT; struct page_info *page; p2m_type_t t; - if ( idx > 0 ) + if ( val & 0xfff ) { + /* Bottom 12 bits are hypercall page index. Only 0 is valid. */ gdprintk(XENLOG_WARNING, - "Out of range index %u to MSR %08x\n", - idx, 0x40000000); + "wrmsr hypercall page index %#lx unsupported\n", + val & 0xfff); return 0; } @@ -671,14 +666,11 @@ int wrmsr_hypervisor_regs(uint32_t idx, uint64_t val) unmap_domain_page(hypercall_page); put_page_and_type(page); - break; + return 1; } - - default: - BUG(); } - return 1; + return 0; } int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx, -- 1.7.10.4