From: Paul Durrant <paul.durrant@citrix.com>
To: xen-devel@lists.xenproject.org
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
Paul Durrant <paul.durrant@citrix.com>,
Jan Beulich <jbeulich@suse.com>
Subject: [PATCH] vlapic/viridian: abort existing APIC assist if any vector is pending in ISR
Date: Tue, 7 Mar 2017 14:58:04 +0000 [thread overview]
Message-ID: <1488898684-8105-1-git-send-email-paul.durrant@citrix.com> (raw)
The vlapic code already aborts an APIC assist if an interrupt is deferred
because a higher priority interrupt has already been delivered (and hence
its vector is pending in the ISR).
However, it is also necessary to abort an APIC assist in the case where a
higher priority is about to be delivered because, in either case, at least
two vectors will be pending in the ISR and hence an EOI is necessary.
Also, following on from the above reasoning, the decision to start a new
APIC assist should clearly be based upon whether any other vector is
pending in the ISR, regardless of whether it is lower or higher in
priority. (In fact the code in question cannot be reached if the
vector is lower in priority). Thus the single use of
vlapic_find_lowest_vector() can be replaced with a call to
vlapic_find_highest_isr() and the former function removed.
Without this patch, because the logic is flawed, a domain_crash() results
when an attempt is made to erroneously start a new APIC assist.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/hvm/vlapic.c | 54 +++++++++++++++++------------------------------
1 file changed, 19 insertions(+), 35 deletions(-)
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 3fa3727..14356a7 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -95,19 +95,6 @@ static int vlapic_find_highest_vector(const void *bitmap)
return (fls(word[word_offset*4]) - 1) + (word_offset * 32);
}
-static int vlapic_find_lowest_vector(const void *bitmap)
-{
- const uint32_t *word = bitmap;
- unsigned int word_offset;
-
- /* Work forwards through the bitmap (first 32-bit word in every four). */
- for ( word_offset = 0; word_offset < NR_VECTORS / 32; word_offset++)
- if ( word[word_offset * 4] )
- return (ffs(word[word_offset * 4]) - 1) + (word_offset * 32);
-
- return -1;
-}
-
/*
* IRR-specific bitmap update & search routines.
*/
@@ -1201,19 +1188,17 @@ int vlapic_has_pending_irq(struct vcpu *v)
vlapic_clear_vector(vector, &vlapic->regs->data[APIC_ISR]);
isr = vlapic_find_highest_isr(vlapic);
- isr = (isr != -1) ? isr : 0;
- if ( (isr & 0xf0) >= (irr & 0xf0) )
- {
- /*
- * There's already a higher priority vector pending so
- * we need to abort any previous APIC assist to ensure there
- * is an EOI.
- */
- viridian_abort_apic_assist(v);
- return -1;
- }
+ if ( isr == -1 )
+ return irr;
- return irr;
+ /*
+ * A vector is pending in the ISR so, regardless of whether the new
+ * vector in the IRR is lower or higher in priority, any pending
+ * APIC assist must be aborted to ensure an EOI.
+ */
+ viridian_abort_apic_assist(v);
+
+ return ((isr & 0xf0) < (irr & 0xf0)) ? irr : -1;
}
int vlapic_ack_pending_irq(struct vcpu *v, int vector, bool_t force_ack)
@@ -1230,16 +1215,15 @@ int vlapic_ack_pending_irq(struct vcpu *v, int vector, bool_t force_ack)
vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) )
goto done;
- isr = vlapic_find_lowest_vector(&vlapic->regs->data[APIC_ISR]);
- if ( isr >= 0 && isr < vector )
- goto done;
-
- /*
- * This vector is edge triggered and there are no lower priority
- * vectors pending, so we can use APIC assist to avoid exiting
- * for EOI.
- */
- viridian_start_apic_assist(v, vector);
+ isr = vlapic_find_highest_isr(vlapic);
+ if ( isr == -1 )
+ {
+ /*
+ * This vector is edge triggered and no other vectors are pending
+ * in the ISR so we can use APIC assist to avoid exiting for EOI.
+ */
+ viridian_start_apic_assist(v, vector);
+ }
done:
vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next reply other threads:[~2017-03-07 14:58 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-07 14:58 Paul Durrant [this message]
2017-03-07 16:27 ` [PATCH] vlapic/viridian: abort existing APIC assist if any vector is pending in ISR 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=1488898684-8105-1-git-send-email-paul.durrant@citrix.com \
--to=paul.durrant@citrix.com \
--cc=andrew.cooper3@citrix.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 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).