From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] passthrough: streamline _hvm_dirq_assist()
Date: Wed, 17 Sep 2014 16:32:45 -0400 [thread overview]
Message-ID: <20140917203245.GA15018@laptop.dumpdata.com> (raw)
In-Reply-To: <5411BC630200007800033E1A@mail.emea.novell.com>
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);
>
> /*
next prev parent reply other threads:[~2014-09-17 20:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2014-09-18 7:49 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140917203245.GA15018@laptop.dumpdata.com \
--to=konrad.wilk@oracle.com \
--cc=JBeulich@suse.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.