xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] passthrough: streamline _hvm_dirq_assist()
@ 2014-09-11 13:14 Jan Beulich
  2014-09-11 14:53 ` Andrew Cooper
  2014-09-17 20:32 ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 4+ messages in thread
From: Jan Beulich @ 2014-09-11 13:14 UTC (permalink / raw)
  To: xen-devel

[-- Attachment #1: Type: text/plain, Size: 3774 bytes --]

The loop inside this function was calling two functions with loop-
invariable arguments which clearly don't need calling more than once:
send_guest_pirq() and __msi_pirq_eoi(). After moving these out of the
loop it further became apparent that folding the hvm_pci_msi_assert()
helper into the main function can further help readability.

In the course of this I noticed that __hvm_dpci_eoi() called
hvm_pci_intx_deassert() unconditionally, whereas hvm_pci_intx_assert()
(correctly) got called only when !hvm_domain_use_pirq(), so the former
is being made conditional now too.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -513,45 +513,39 @@ void hvm_dpci_msi_eoi(struct domain *d, 
     spin_unlock(&d->event_lock);
 }
 
-static void hvm_pci_msi_assert(
-    struct domain *d, struct hvm_pirq_dpci *pirq_dpci)
-{
-    struct pirq *pirq = dpci_pirq(pirq_dpci);
-
-    if ( hvm_domain_use_pirq(d, pirq) )
-        send_guest_pirq(d, pirq);
-    else
-        vmsi_deliver_pirq(d, pirq_dpci);
-}
-
 static int _hvm_dirq_assist(struct domain *d, struct hvm_pirq_dpci *pirq_dpci,
                             void *arg)
 {
     if ( test_and_clear_bool(pirq_dpci->masked) )
     {
+        struct pirq *pirq = dpci_pirq(pirq_dpci);
         const struct dev_intx_gsi_link *digl;
 
+        if ( hvm_domain_use_pirq(d, pirq) )
+        {
+            send_guest_pirq(d, pirq);
+
+            if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
+                return 0;
+        }
+
         if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
         {
-            hvm_pci_msi_assert(d, pirq_dpci);
+            vmsi_deliver_pirq(d, pirq_dpci);
             return 0;
         }
 
         list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
         {
-            struct pirq *info = dpci_pirq(pirq_dpci);
-
-            if ( hvm_domain_use_pirq(d, info) )
-                send_guest_pirq(d, info);
-            else
-                hvm_pci_intx_assert(d, digl->device, digl->intx);
+            hvm_pci_intx_assert(d, digl->device, digl->intx);
             pirq_dpci->pending++;
+        }
 
-            if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
-            {
-                /* for translated MSI to INTx interrupt, eoi as early as possible */
-                __msi_pirq_eoi(pirq_dpci);
-            }
+        if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
+        {
+            /* for translated MSI to INTx interrupt, eoi as early as possible */
+            __msi_pirq_eoi(pirq_dpci);
+            return 0;
         }
 
         /*
@@ -561,8 +555,8 @@ static int _hvm_dirq_assist(struct domai
          * guest will never deal with the irq, then the physical interrupt line
          * will never be deasserted.
          */
-        if ( pt_irq_need_timer(pirq_dpci->flags) )
-            set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
+        ASSERT(pt_irq_need_timer(pirq_dpci->flags));
+        set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
     }
 
     return 0;
@@ -583,12 +577,12 @@ static void __hvm_dpci_eoi(struct domain
                            const struct hvm_girq_dpci_mapping *girq,
                            const union vioapic_redir_entry *ent)
 {
-    struct pirq *pirq;
+    struct pirq *pirq = pirq_info(d, girq->machine_gsi);
     struct hvm_pirq_dpci *pirq_dpci;
 
-    hvm_pci_intx_deassert(d, girq->device, girq->intx);
+    if ( !hvm_domain_use_pirq(d, pirq) )
+        hvm_pci_intx_deassert(d, girq->device, girq->intx);
 
-    pirq = pirq_info(d, girq->machine_gsi);
     pirq_dpci = pirq_dpci(pirq);
 
     /*




[-- Attachment #2: pt-dirq-assist.patch --]
[-- Type: text/plain, Size: 3814 bytes --]

passthrough: streamline _hvm_dirq_assist()

The loop inside this function was calling two functions with loop-
invariable arguments which clearly don't need calling more than once:
send_guest_pirq() and __msi_pirq_eoi(). After moving these out of the
loop it further became apparent that folding the hvm_pci_msi_assert()
helper into the main function can further help readability.

In the course of this I noticed that __hvm_dpci_eoi() called
hvm_pci_intx_deassert() unconditionally, whereas hvm_pci_intx_assert()
(correctly) got called only when !hvm_domain_use_pirq(), so the former
is being made conditional now too.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -513,45 +513,39 @@ void hvm_dpci_msi_eoi(struct domain *d, 
     spin_unlock(&d->event_lock);
 }
 
-static void hvm_pci_msi_assert(
-    struct domain *d, struct hvm_pirq_dpci *pirq_dpci)
-{
-    struct pirq *pirq = dpci_pirq(pirq_dpci);
-
-    if ( hvm_domain_use_pirq(d, pirq) )
-        send_guest_pirq(d, pirq);
-    else
-        vmsi_deliver_pirq(d, pirq_dpci);
-}
-
 static int _hvm_dirq_assist(struct domain *d, struct hvm_pirq_dpci *pirq_dpci,
                             void *arg)
 {
     if ( test_and_clear_bool(pirq_dpci->masked) )
     {
+        struct pirq *pirq = dpci_pirq(pirq_dpci);
         const struct dev_intx_gsi_link *digl;
 
+        if ( hvm_domain_use_pirq(d, pirq) )
+        {
+            send_guest_pirq(d, pirq);
+
+            if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
+                return 0;
+        }
+
         if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
         {
-            hvm_pci_msi_assert(d, pirq_dpci);
+            vmsi_deliver_pirq(d, pirq_dpci);
             return 0;
         }
 
         list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
         {
-            struct pirq *info = dpci_pirq(pirq_dpci);
-
-            if ( hvm_domain_use_pirq(d, info) )
-                send_guest_pirq(d, info);
-            else
-                hvm_pci_intx_assert(d, digl->device, digl->intx);
+            hvm_pci_intx_assert(d, digl->device, digl->intx);
             pirq_dpci->pending++;
+        }
 
-            if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
-            {
-                /* for translated MSI to INTx interrupt, eoi as early as possible */
-                __msi_pirq_eoi(pirq_dpci);
-            }
+        if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
+        {
+            /* for translated MSI to INTx interrupt, eoi as early as possible */
+            __msi_pirq_eoi(pirq_dpci);
+            return 0;
         }
 
         /*
@@ -561,8 +555,8 @@ static int _hvm_dirq_assist(struct domai
          * guest will never deal with the irq, then the physical interrupt line
          * will never be deasserted.
          */
-        if ( pt_irq_need_timer(pirq_dpci->flags) )
-            set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
+        ASSERT(pt_irq_need_timer(pirq_dpci->flags));
+        set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
     }
 
     return 0;
@@ -583,12 +577,12 @@ static void __hvm_dpci_eoi(struct domain
                            const struct hvm_girq_dpci_mapping *girq,
                            const union vioapic_redir_entry *ent)
 {
-    struct pirq *pirq;
+    struct pirq *pirq = pirq_info(d, girq->machine_gsi);
     struct hvm_pirq_dpci *pirq_dpci;
 
-    hvm_pci_intx_deassert(d, girq->device, girq->intx);
+    if ( !hvm_domain_use_pirq(d, pirq) )
+        hvm_pci_intx_deassert(d, girq->device, girq->intx);
 
-    pirq = pirq_info(d, girq->machine_gsi);
     pirq_dpci = pirq_dpci(pirq);
 
     /*

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] passthrough: streamline _hvm_dirq_assist()
  2014-09-11 13:14 [PATCH] passthrough: streamline _hvm_dirq_assist() Jan Beulich
@ 2014-09-11 14:53 ` Andrew Cooper
  2014-09-17 20:32 ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2014-09-11 14:53 UTC (permalink / raw)
  To: Jan Beulich, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 4110 bytes --]

On 11/09/14 14:14, Jan Beulich wrote:
> The loop inside this function was calling two functions with loop-
> invariable arguments which clearly don't need calling more than once:
> send_guest_pirq() and __msi_pirq_eoi(). After moving these out of the
> loop it further became apparent that folding the hvm_pci_msi_assert()
> helper into the main function can further help readability.
>
> In the course of this I noticed that __hvm_dpci_eoi() called
> hvm_pci_intx_deassert() unconditionally, whereas hvm_pci_intx_assert()
> (correctly) got called only when !hvm_domain_use_pirq(), so the former
> is being made conditional now too.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

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

>
> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -513,45 +513,39 @@ void hvm_dpci_msi_eoi(struct domain *d, 
>      spin_unlock(&d->event_lock);
>  }
>  
> -static void hvm_pci_msi_assert(
> -    struct domain *d, struct hvm_pirq_dpci *pirq_dpci)
> -{
> -    struct pirq *pirq = dpci_pirq(pirq_dpci);
> -
> -    if ( hvm_domain_use_pirq(d, pirq) )
> -        send_guest_pirq(d, pirq);
> -    else
> -        vmsi_deliver_pirq(d, pirq_dpci);
> -}
> -
>  static int _hvm_dirq_assist(struct domain *d, struct hvm_pirq_dpci *pirq_dpci,
>                              void *arg)
>  {
>      if ( test_and_clear_bool(pirq_dpci->masked) )
>      {
> +        struct pirq *pirq = dpci_pirq(pirq_dpci);
>          const struct dev_intx_gsi_link *digl;
>  
> +        if ( hvm_domain_use_pirq(d, pirq) )
> +        {
> +            send_guest_pirq(d, pirq);
> +
> +            if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
> +                return 0;
> +        }
> +
>          if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
>          {
> -            hvm_pci_msi_assert(d, pirq_dpci);
> +            vmsi_deliver_pirq(d, pirq_dpci);
>              return 0;
>          }
>  
>          list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
>          {
> -            struct pirq *info = dpci_pirq(pirq_dpci);
> -
> -            if ( hvm_domain_use_pirq(d, info) )
> -                send_guest_pirq(d, info);
> -            else
> -                hvm_pci_intx_assert(d, digl->device, digl->intx);
> +            hvm_pci_intx_assert(d, digl->device, digl->intx);
>              pirq_dpci->pending++;
> +        }
>  
> -            if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
> -            {
> -                /* for translated MSI to INTx interrupt, eoi as early as possible */
> -                __msi_pirq_eoi(pirq_dpci);
> -            }
> +        if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
> +        {
> +            /* for translated MSI to INTx interrupt, eoi as early as possible */
> +            __msi_pirq_eoi(pirq_dpci);
> +            return 0;
>          }
>  
>          /*
> @@ -561,8 +555,8 @@ static int _hvm_dirq_assist(struct domai
>           * guest will never deal with the irq, then the physical interrupt line
>           * will never be deasserted.
>           */
> -        if ( pt_irq_need_timer(pirq_dpci->flags) )
> -            set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
> +        ASSERT(pt_irq_need_timer(pirq_dpci->flags));
> +        set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
>      }
>  
>      return 0;
> @@ -583,12 +577,12 @@ static void __hvm_dpci_eoi(struct domain
>                             const struct hvm_girq_dpci_mapping *girq,
>                             const union vioapic_redir_entry *ent)
>  {
> -    struct pirq *pirq;
> +    struct pirq *pirq = pirq_info(d, girq->machine_gsi);
>      struct hvm_pirq_dpci *pirq_dpci;
>  
> -    hvm_pci_intx_deassert(d, girq->device, girq->intx);
> +    if ( !hvm_domain_use_pirq(d, pirq) )
> +        hvm_pci_intx_deassert(d, girq->device, girq->intx);
>  
> -    pirq = pirq_info(d, girq->machine_gsi);
>      pirq_dpci = pirq_dpci(pirq);
>  
>      /*
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 4923 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] passthrough: streamline _hvm_dirq_assist()
  2014-09-11 13:14 [PATCH] passthrough: streamline _hvm_dirq_assist() Jan Beulich
  2014-09-11 14:53 ` Andrew Cooper
@ 2014-09-17 20:32 ` Konrad Rzeszutek Wilk
  2014-09-18  7:49   ` Jan Beulich
  1 sibling, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-09-17 20:32 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

On Thu, Sep 11, 2014 at 02:14:43PM +0100, Jan Beulich wrote:
> The loop inside this function was calling two functions with loop-
> invariable arguments which clearly don't need calling more than once:
> send_guest_pirq() and __msi_pirq_eoi(). After moving these out of the
> loop it further became apparent that folding the hvm_pci_msi_assert()
> helper into the main function can further help readability.
> 
> In the course of this I noticed that __hvm_dpci_eoi() called
> hvm_pci_intx_deassert() unconditionally, whereas hvm_pci_intx_assert()
> (correctly) got called only when !hvm_domain_use_pirq(), so the former
> is being made conditional now too.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thought I would also like to test it first, so if you are OK with
waiting until I send out an 'Tested-by' I would appreciate it.
> 
> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -513,45 +513,39 @@ void hvm_dpci_msi_eoi(struct domain *d, 
>      spin_unlock(&d->event_lock);
>  }
>  
> -static void hvm_pci_msi_assert(
> -    struct domain *d, struct hvm_pirq_dpci *pirq_dpci)
> -{
> -    struct pirq *pirq = dpci_pirq(pirq_dpci);
> -
> -    if ( hvm_domain_use_pirq(d, pirq) )
> -        send_guest_pirq(d, pirq);
> -    else
> -        vmsi_deliver_pirq(d, pirq_dpci);
> -}
> -
>  static int _hvm_dirq_assist(struct domain *d, struct hvm_pirq_dpci *pirq_dpci,
>                              void *arg)
>  {
>      if ( test_and_clear_bool(pirq_dpci->masked) )
>      {
> +        struct pirq *pirq = dpci_pirq(pirq_dpci);
>          const struct dev_intx_gsi_link *digl;
>  
> +        if ( hvm_domain_use_pirq(d, pirq) )
> +        {
> +            send_guest_pirq(d, pirq);
> +
> +            if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
> +                return 0;
> +        }
> +
>          if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
>          {
> -            hvm_pci_msi_assert(d, pirq_dpci);
> +            vmsi_deliver_pirq(d, pirq_dpci);
>              return 0;
>          }
>  
>          list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
>          {
> -            struct pirq *info = dpci_pirq(pirq_dpci);
> -
> -            if ( hvm_domain_use_pirq(d, info) )
> -                send_guest_pirq(d, info);
> -            else
> -                hvm_pci_intx_assert(d, digl->device, digl->intx);
> +            hvm_pci_intx_assert(d, digl->device, digl->intx);
>              pirq_dpci->pending++;
> +        }
>  
> -            if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
> -            {
> -                /* for translated MSI to INTx interrupt, eoi as early as possible */
> -                __msi_pirq_eoi(pirq_dpci);
> -            }
> +        if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
> +        {
> +            /* for translated MSI to INTx interrupt, eoi as early as possible */
> +            __msi_pirq_eoi(pirq_dpci);
> +            return 0;
>          }
>  
>          /*
> @@ -561,8 +555,8 @@ static int _hvm_dirq_assist(struct domai
>           * guest will never deal with the irq, then the physical interrupt line
>           * will never be deasserted.
>           */
> -        if ( pt_irq_need_timer(pirq_dpci->flags) )
> -            set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
> +        ASSERT(pt_irq_need_timer(pirq_dpci->flags));
> +        set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
>      }
>  
>      return 0;
> @@ -583,12 +577,12 @@ static void __hvm_dpci_eoi(struct domain
>                             const struct hvm_girq_dpci_mapping *girq,
>                             const union vioapic_redir_entry *ent)
>  {
> -    struct pirq *pirq;
> +    struct pirq *pirq = pirq_info(d, girq->machine_gsi);
>      struct hvm_pirq_dpci *pirq_dpci;
>  
> -    hvm_pci_intx_deassert(d, girq->device, girq->intx);
> +    if ( !hvm_domain_use_pirq(d, pirq) )
> +        hvm_pci_intx_deassert(d, girq->device, girq->intx);
>  
> -    pirq = pirq_info(d, girq->machine_gsi);
>      pirq_dpci = pirq_dpci(pirq);
>  
>      /*
> 
> 
> 

> passthrough: streamline _hvm_dirq_assist()
> 
> The loop inside this function was calling two functions with loop-
> invariable arguments which clearly don't need calling more than once:
> send_guest_pirq() and __msi_pirq_eoi(). After moving these out of the
> loop it further became apparent that folding the hvm_pci_msi_assert()
> helper into the main function can further help readability.
> 
> In the course of this I noticed that __hvm_dpci_eoi() called
> hvm_pci_intx_deassert() unconditionally, whereas hvm_pci_intx_assert()
> (correctly) got called only when !hvm_domain_use_pirq(), so the former
> is being made conditional now too.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -513,45 +513,39 @@ void hvm_dpci_msi_eoi(struct domain *d, 
>      spin_unlock(&d->event_lock);
>  }
>  
> -static void hvm_pci_msi_assert(
> -    struct domain *d, struct hvm_pirq_dpci *pirq_dpci)
> -{
> -    struct pirq *pirq = dpci_pirq(pirq_dpci);
> -
> -    if ( hvm_domain_use_pirq(d, pirq) )
> -        send_guest_pirq(d, pirq);
> -    else
> -        vmsi_deliver_pirq(d, pirq_dpci);
> -}
> -
>  static int _hvm_dirq_assist(struct domain *d, struct hvm_pirq_dpci *pirq_dpci,
>                              void *arg)
>  {
>      if ( test_and_clear_bool(pirq_dpci->masked) )
>      {
> +        struct pirq *pirq = dpci_pirq(pirq_dpci);
>          const struct dev_intx_gsi_link *digl;
>  
> +        if ( hvm_domain_use_pirq(d, pirq) )
> +        {
> +            send_guest_pirq(d, pirq);
> +
> +            if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
> +                return 0;
> +        }
> +
>          if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI )
>          {
> -            hvm_pci_msi_assert(d, pirq_dpci);
> +            vmsi_deliver_pirq(d, pirq_dpci);
>              return 0;
>          }
>  
>          list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
>          {
> -            struct pirq *info = dpci_pirq(pirq_dpci);
> -
> -            if ( hvm_domain_use_pirq(d, info) )
> -                send_guest_pirq(d, info);
> -            else
> -                hvm_pci_intx_assert(d, digl->device, digl->intx);
> +            hvm_pci_intx_assert(d, digl->device, digl->intx);
>              pirq_dpci->pending++;
> +        }
>  
> -            if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
> -            {
> -                /* for translated MSI to INTx interrupt, eoi as early as possible */
> -                __msi_pirq_eoi(pirq_dpci);
> -            }
> +        if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE )
> +        {
> +            /* for translated MSI to INTx interrupt, eoi as early as possible */
> +            __msi_pirq_eoi(pirq_dpci);
> +            return 0;
>          }
>  
>          /*
> @@ -561,8 +555,8 @@ static int _hvm_dirq_assist(struct domai
>           * guest will never deal with the irq, then the physical interrupt line
>           * will never be deasserted.
>           */
> -        if ( pt_irq_need_timer(pirq_dpci->flags) )
> -            set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
> +        ASSERT(pt_irq_need_timer(pirq_dpci->flags));
> +        set_timer(&pirq_dpci->timer, NOW() + PT_IRQ_TIME_OUT);
>      }
>  
>      return 0;
> @@ -583,12 +577,12 @@ static void __hvm_dpci_eoi(struct domain
>                             const struct hvm_girq_dpci_mapping *girq,
>                             const union vioapic_redir_entry *ent)
>  {
> -    struct pirq *pirq;
> +    struct pirq *pirq = pirq_info(d, girq->machine_gsi);
>      struct hvm_pirq_dpci *pirq_dpci;
>  
> -    hvm_pci_intx_deassert(d, girq->device, girq->intx);
> +    if ( !hvm_domain_use_pirq(d, pirq) )
> +        hvm_pci_intx_deassert(d, girq->device, girq->intx);
>  
> -    pirq = pirq_info(d, girq->machine_gsi);
>      pirq_dpci = pirq_dpci(pirq);
>  
>      /*

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] passthrough: streamline _hvm_dirq_assist()
  2014-09-17 20:32 ` Konrad Rzeszutek Wilk
@ 2014-09-18  7:49   ` Jan Beulich
  0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2014-09-18  7:49 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel

>>> On 17.09.14 at 22:32, <konrad.wilk@oracle.com> wrote:
> On Thu, Sep 11, 2014 at 02:14:43PM +0100, Jan Beulich wrote:
>> The loop inside this function was calling two functions with loop-
>> invariable arguments which clearly don't need calling more than once:
>> send_guest_pirq() and __msi_pirq_eoi(). After moving these out of the
>> loop it further became apparent that folding the hvm_pci_msi_assert()
>> helper into the main function can further help readability.
>> 
>> In the course of this I noticed that __hvm_dpci_eoi() called
>> hvm_pci_intx_deassert() unconditionally, whereas hvm_pci_intx_assert()
>> (correctly) got called only when !hvm_domain_use_pirq(), so the former
>> is being made conditional now too.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Thought I would also like to test it first, so if you are OK with
> waiting until I send out an 'Tested-by' I would appreciate it.

Too late - it passed the push gate yesterday already.

Jan

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-09-18  7:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-11 13:14 [PATCH] passthrough: streamline _hvm_dirq_assist() Jan Beulich
2014-09-11 14:53 ` Andrew Cooper
2014-09-17 20:32 ` Konrad Rzeszutek Wilk
2014-09-18  7:49   ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).