Re: [Xen-devel] [PATCH 2/3] x86/HVM: make vmsi_deliver() return proper error values

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/06/14 14:59, Jan Beulich wrote:

... and propagate this from hvm_inject_msi(). In the course of this I
spotted further room for cleanup:
- vmsi_inj_irq()'s struct domain * parameter was unused
- vmsi_deliver() pointlessly passed on dest_ExtINT to vmsi_inj_irq()
  (which that one validly refused to handle)
- vmsi_inj_irq()'s sole caller guarantees a proper delivery mode (i.e.
  rather than printing an obscure message we can just BUG())
- some formatting and log message quirks

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

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Arguably under Xen style, the final addition in the final hunk should have a newline before return 0, but that's trivial.

--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -311,9 +311,7 @@ int hvm_inject_msi(struct domain *d, uin
         return -ERANGE;
     }
 
-    vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode);
-
-    return 0;
+    return vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode);
 }
 
 void hvm_set_callback_via(struct domain *d, uint64_t via)
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -43,14 +43,12 @@
 #include <asm/io_apic.h>
 
 static void vmsi_inj_irq(
-    struct domain *d,
     struct vlapic *target,
     uint8_t vector,
     uint8_t trig_mode,
     uint8_t delivery_mode)
 {
-    HVM_DBG_LOG(DBG_LEVEL_IOAPIC, "vmsi_inj_irq "
-                "irq %d trig %d delive mode %d\n",
+    HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "vmsi_inj_irq: vec %02x trig %d dm %d\n",
                 vector, trig_mode, delivery_mode);
 
     switch ( delivery_mode )
@@ -60,8 +58,7 @@ static void vmsi_inj_irq(
         vlapic_set_irq(target, vector, trig_mode);
         break;
     default:
-        gdprintk(XENLOG_WARNING, "error delivery mode %d\n", delivery_mode);
-        break;
+        BUG();
     }
 }
 
@@ -76,38 +73,31 @@ int vmsi_deliver(
     switch ( delivery_mode )
     {
     case dest_LowestPrio:
-    {
         target = vlapic_lowest_prio(d, NULL, 0, dest, dest_mode);
         if ( target != NULL )
-            vmsi_inj_irq(d, target, vector, trig_mode, delivery_mode);
-        else
-            HVM_DBG_LOG(DBG_LEVEL_IOAPIC, "null round robin: "
-                        "vector=%x delivery_mode=%x\n",
-                        vector, dest_LowestPrio);
-        break;
-    }
+        {
+            vmsi_inj_irq(target, vector, trig_mode, delivery_mode);
+            break;
+        }
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "null MSI round robin: vector=%02x\n",
+                    vector);
+        return -ESRCH;
 
     case dest_Fixed:
-    case dest_ExtINT:
-    {
         for_each_vcpu ( d, v )
             if ( vlapic_match_dest(vcpu_vlapic(v), NULL,
                                    0, dest, dest_mode) )
-                vmsi_inj_irq(d, vcpu_vlapic(v),
-                             vector, trig_mode, delivery_mode);
+                vmsi_inj_irq(vcpu_vlapic(v), vector,
+                             trig_mode, delivery_mode);
         break;
-    }
 
-    case dest_SMI:
-    case dest_NMI:
-    case dest_INIT:
-    case dest__reserved_2:
     default:
-        gdprintk(XENLOG_WARNING, "Unsupported delivery mode %d\n",
-                 delivery_mode);
-        break;
+        printk(XENLOG_G_WARNING
+               "%pv: Unsupported MSI delivery mode %d for Dom%d\n",
+               current, delivery_mode, d->domain_id);
+        return -EINVAL;
     }
-    return 1;
+    return 0;
 }
 
 void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci)

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

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