Re: [PATCH] passthrough: streamline _hvm_dirq_assist()

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- 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