x86/APIC: don't make wrong implications on constants

For the physical APIC, oprofile code was abusing APIC_DM_NMI as a mask.

For the virtual APIC, a wrong assumption was made that LVTPC could be
programmed to only fixed or NMI delivery modes. While other modes are
invalid here, we still shouldn't inject an NMI into the guest in such
a case. Instead just do nothing.

In the course of adjusting this it became obvious that what value
vpmu_do_interrupt() returns on its various return paths was pretty
arbitrary. With its only caller ignoring the return value, simply make
the function's return type "void".

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
index 15d5b6fecfe6294e4dd3a0ee1b59f321358f312b..265fc0ecbb50148ce74945c9d4931c5a49ac7238 100644 (file)
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -82,7 +82,7 @@ int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
     return 0;
 }
 
-int vpmu_do_interrupt(struct cpu_user_regs *regs)
+void vpmu_do_interrupt(struct cpu_user_regs *regs)
 {
     struct vcpu *v = current;
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
@@ -91,25 +91,23 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
     {
         struct vlapic *vlapic = vcpu_vlapic(v);
         u32 vlapic_lvtpc;
-        unsigned char int_vec;
 
-        if ( !vpmu->arch_vpmu_ops->do_interrupt(regs) )
-            return 0;
-
-        if ( !is_vlapic_lvtpc_enabled(vlapic) )
-            return 1;
+        if ( !vpmu->arch_vpmu_ops->do_interrupt(regs) ||
+             !is_vlapic_lvtpc_enabled(vlapic) )
+            return;
 
         vlapic_lvtpc = vlapic_get_reg(vlapic, APIC_LVTPC);
-        int_vec = vlapic_lvtpc & APIC_VECTOR_MASK;
 
-        if ( GET_APIC_DELIVERY_MODE(vlapic_lvtpc) == APIC_MODE_FIXED )
-            vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
-        else
+        switch ( GET_APIC_DELIVERY_MODE(vlapic_lvtpc) )
+        {
+        case APIC_MODE_FIXED:
+            vlapic_set_irq(vlapic, vlapic_lvtpc & APIC_VECTOR_MASK, 0);
+            break;
+        case APIC_MODE_NMI:
             v->nmi_pending = 1;
-        return 1;
+            break;
+        }
     }
-
-    return 0;
 }
 
 void vpmu_do_cpuid(unsigned int input,

diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c
index eb12fcbc4682bfdb34200bac5ebade4055f87f3c..13534d4914054291f9cc53a0dbd7c33540dd25a4 100644 (file)
--- a/xen/arch/x86/oprofile/nmi_int.c
+++ b/xen/arch/x86/oprofile/nmi_int.c
@@ -274,7 +274,7 @@ static void nmi_cpu_stop(void * dummy)
         * power on apic lvt contain a zero vector nr which are legal only for
         * NMI delivery mode. So inhibit apic err before restoring lvtpc
         */
-       if ( !(apic_read(APIC_LVTPC) & APIC_DM_NMI)
+       if ( (apic_read(APIC_LVTPC) & APIC_MODE_MASK) != APIC_DM_NMI
             || (apic_read(APIC_LVTPC) & APIC_LVT_MASKED) )
        {
                printk("nmi_stop: APIC not good %ul\n", apic_read(APIC_LVTPC));

diff --git a/xen/include/asm-x86/hvm/vpmu.h b/xen/include/asm-x86/hvm/vpmu.h
index 9a5ac01619088560a8113e18ef2ede8f4d7e4c58..1f28bd85e14a93754c0a150445374a3ab6b1b069 100644 (file)
--- a/xen/include/asm-x86/hvm/vpmu.h
+++ b/xen/include/asm-x86/hvm/vpmu.h
@@ -89,7 +89,7 @@ struct vpmu_struct {
 
 int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported);
 int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content);
-int vpmu_do_interrupt(struct cpu_user_regs *regs);
+void vpmu_do_interrupt(struct cpu_user_regs *regs);
 void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
                                        unsigned int *ecx, unsigned int *edx);
 void vpmu_initialise(struct vcpu *v);