From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 2/3] x86/HVM: make vmsi_deliver() return proper error values Date: Tue, 3 Jun 2014 15:13:15 +0100 Message-ID: <538DD7FB.8050304@citrix.com> References: <538DEF42020000780001767A@mail.emea.novell.com> <538DF0ED0200007800017698@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4123334958456486973==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WrpTG-0002zH-CM for xen-devel@lists.xenproject.org; Tue, 03 Jun 2014 14:13:46 +0000 In-Reply-To: <538DF0ED0200007800017698@mail.emea.novell.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: Jan Beulich Cc: xen-devel , Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============4123334958456486973== Content-Type: multipart/alternative; boundary="------------020409020602040107080003" --------------020409020602040107080003 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit 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 Reviewed-by: Andrew Cooper 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 > > 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@lists.xen.org > http://lists.xen.org/xen-devel --------------020409020602040107080003 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
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@suse.com>

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

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@lists.xen.org
http://lists.xen.org/xen-devel

--------------020409020602040107080003-- --===============4123334958456486973== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============4123334958456486973==--