[Xen-devel] [PATCH] x86/vLAPIC: adjust types in internal read/write handling

["Jan Beulich" <JBeulich@xxxxxxxx>]

- use 32-bit types where possible (produces slightly better code)
- drop (now) unnecessary casts
- avoid indirection where not needed
- avoid duplicate log messages in vlapic_write()
- minor other cleanup

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -556,52 +556,42 @@ static void vlapic_set_tdcr(struct vlapi
                 "timer_divisor: %d", vlapic->hw.timer_divisor);
 }
 
-static void vlapic_read_aligned(
-    struct vlapic *vlapic, unsigned int offset, unsigned int *result)
+static uint32_t vlapic_read_aligned(struct vlapic *vlapic, unsigned int offset)
 {
     switch ( offset )
     {
     case APIC_PROCPRI:
-        *result = vlapic_get_ppr(vlapic);
-        break;
+        return vlapic_get_ppr(vlapic);
 
     case APIC_TMCCT: /* Timer CCR */
         if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
-        {
-            *result = 0;
             break;
-        }
-        *result = vlapic_get_tmcct(vlapic);
-        break;
+        return vlapic_get_tmcct(vlapic);
 
     case APIC_TMICT: /* Timer ICR */
         if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
-        {
-            *result = 0;
             break;
-        }
+        /* fall through */
     default:
-        *result = vlapic_get_reg(vlapic, offset);
-        break;
+        return vlapic_get_reg(vlapic, offset);
     }
+
+    return 0;
 }
 
 static int vlapic_read(
     struct vcpu *v, unsigned long address,
     unsigned long len, unsigned long *pval)
 {
-    unsigned int alignment;
-    unsigned int tmp;
-    unsigned long result = 0;
     struct vlapic *vlapic = vcpu_vlapic(v);
     unsigned int offset = address - vlapic_base_address(vlapic);
+    unsigned int alignment = offset & 3, tmp, result = 0;
 
     if ( offset > (APIC_TDCR + 0x3) )
         goto out;
 
-    alignment = offset & 0x3;
+    tmp = vlapic_read_aligned(vlapic, offset & ~3);
 
-    vlapic_read_aligned(vlapic, offset & ~0x3, &tmp);
     switch ( len )
     {
     case 1:
@@ -627,7 +617,7 @@ static int vlapic_read(
     }
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "offset %#x with length %#lx, "
-                "and the result is %#lx", offset, len, result);
+                "and the result is %#x", offset, len, result);
 
  out:
     *pval = result;
@@ -657,19 +647,17 @@ int hvm_x2apic_msr_read(struct vcpu *v, 
 #undef REGBLOCK
         };
     struct vlapic *vlapic = vcpu_vlapic(v);
-    uint32_t low, high = 0, reg = msr - MSR_IA32_APICBASE_MSR,
-        offset = reg << 4;
+    uint32_t high = 0, reg = msr - MSR_IA32_APICBASE_MSR, offset = reg << 4;
 
     if ( !vlapic_x2apic_mode(vlapic) ||
          (reg >= sizeof(readable) * 8) || !test_bit(reg, readable) )
         return X86EMUL_UNHANDLEABLE;
 
     if ( offset == APIC_ICR )
-        vlapic_read_aligned(vlapic, APIC_ICR2, &high);
-
-    vlapic_read_aligned(vlapic, offset, &low);
+        high = vlapic_read_aligned(vlapic, APIC_ICR2);
 
-    *msr_content = (((uint64_t)high) << 32) | low;
+    *msr_content = ((uint64_t)high << 32) |
+                   vlapic_read_aligned(vlapic, offset);
 
     return X86EMUL_OKAY;
 }
@@ -687,7 +675,7 @@ static void vlapic_tdt_pt_cb(struct vcpu
 }
 
 static int vlapic_reg_write(struct vcpu *v,
-                            unsigned int offset, unsigned long val)
+                            unsigned int offset, uint32_t val)
 {
     struct vlapic *vlapic = vcpu_vlapic(v);
     int rc = X86EMUL_OKAY;
@@ -797,8 +785,7 @@ static int vlapic_reg_write(struct vcpu 
             break;
         }
 
-        period = ((uint64_t)APIC_BUS_CYCLE_NS *
-                  (uint32_t)val * vlapic->hw.timer_divisor);
+        period = (uint64_t)APIC_BUS_CYCLE_NS * val * vlapic->hw.timer_divisor;
         TRACE_2_LONG_3D(TRC_HVM_EMUL_LAPIC_START_TIMER, TRC_PAR_LONG(period),
                  TRC_PAR_LONG(vlapic_lvtt_period(vlapic) ? period : 0LL),
                  vlapic->pt.irq);
@@ -811,7 +798,7 @@ static int vlapic_reg_write(struct vcpu 
 
         HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                     "bus cycle is %uns, "
-                    "initial count %lu, period %"PRIu64"ns",
+                    "initial count %u, period %"PRIu64"ns",
                     APIC_BUS_CYCLE_NS, val, period);
     }
     break;
@@ -847,47 +834,41 @@ static int vlapic_write(struct vcpu *v, 
      * According to the IA32 Manual, all accesses should be 32 bits.
      * Some OSes do 8- or 16-byte accesses, however.
      */
-    val = (uint32_t)val;
-    if ( len != 4 )
+    if ( unlikely(len != 4) )
     {
-        unsigned int tmp;
-        unsigned char alignment;
-
-        gdprintk(XENLOG_INFO, "Notice: Local APIC write with len = %lx\n",len);
-
-        alignment = offset & 0x3;
-        (void)vlapic_read_aligned(vlapic, offset & ~0x3, &tmp);
+        unsigned int tmp = vlapic_read_aligned(vlapic, offset & ~3);
+        unsigned char alignment = (offset & 3) * 8;
 
         switch ( len )
         {
         case 1:
-            val = ((tmp & ~(0xff << (8*alignment))) |
-                   ((val & 0xff) << (8*alignment)));
+            val = ((tmp & ~(0xff << alignment)) |
+                   ((val & 0xff) << alignment));
             break;
 
         case 2:
             if ( alignment & 1 )
                 goto unaligned_exit_and_crash;
-            val = ((tmp & ~(0xffff << (8*alignment))) |
-                   ((val & 0xffff) << (8*alignment)));
+            val = ((tmp & ~(0xffff << alignment)) |
+                   ((val & 0xffff) << alignment));
             break;
 
         default:
-            gdprintk(XENLOG_ERR, "Local APIC write with len = %lx, "
-                     "should be 4 instead\n", len);
+            gprintk(XENLOG_ERR, "LAPIC write with len %lx\n", len);
             goto exit_and_crash;
         }
+
+        gdprintk(XENLOG_INFO, "Notice: LAPIC write with len %lx\n", len);
+        offset &= ~3;
     }
-    else if ( (offset & 0x3) != 0 )
+    else if ( unlikely(offset & 3) )
         goto unaligned_exit_and_crash;
 
-    offset &= ~0x3;
-
     return vlapic_reg_write(v, offset, val);
 
  unaligned_exit_and_crash:
-    gdprintk(XENLOG_ERR, "Unaligned LAPIC write len=%#lx at offset=%#x.\n",
-             len, offset);
+    gprintk(XENLOG_ERR, "Unaligned LAPIC write: len=%#lx offset=%#x.\n",
+            len, offset);
  exit_and_crash:
     domain_crash(v->domain);
     return rc;
@@ -983,7 +964,7 @@ int hvm_x2apic_msr_write(struct vcpu *v,
             return X86EMUL_UNHANDLEABLE;
     }
 
-    return vlapic_reg_write(v, offset, (uint32_t)msr_content);
+    return vlapic_reg_write(v, offset, msr_content);
 }
 
 static int vlapic_range(struct vcpu *v, unsigned long addr)

Attachment: x86-vLAPIC-read-write-types.patch
Description: Text document

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