[Xen-devel] [Patch 2/2] x86/traps: improvements to {rd, wr}msr_hypervisor_regs()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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 and PAGE_MASK instead of opencoded numbers.
 * a more descriptive error message for attempts to write a hypercall page at
   an unaligned frame address.

There is no behavioural change as a result.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Keir Fraser <keir@xxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
---
 xen/arch/x86/traps.c |   38 +++++++++++++-------------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 6c7bd99..7aa4088 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -595,55 +595,45 @@ 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 1 if handled, 0 if not and -Exx for error. */
 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 */
     {
         *val = 0;
-        break;
+        return 1;
     }
     default:
-        BUG();
+        return 0;
     }
-
-    return 1;
 }
 
+/* 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 & PAGE_MASK )
         {
             gdprintk(XENLOG_WARNING,
-                     "Out of range index %u to MSR %08x\n",
-                     idx, 0x40000000);
+                     "Expected aligned frame for writing hypercall page\n");
             return 0;
         }
 
@@ -671,14 +661,12 @@ 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 0;
     }
-
-    return 1;
 }
 
 int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
-- 
1.7.10.4


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