On Mon, 18 Oct 2010, Jan Beulich wrote:
> >>> On 15.10.10 at 19:03, Stefano Stabellini
> >>> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > @@ -1074,9 +1081,6 @@ static void __xen_evtchn_do_upcall(struct pt_regs
> > *regs)
> > int irq = evtchn_to_irq[port];
> > struct irq_desc *desc;
> >
> > - mask_evtchn(port);
> > - clear_evtchn(port);
> > -
> > if (irq != -1) {
> > desc = irq_to_desc(irq);
> > if (desc)
> > @@ -1202,7 +1206,7 @@ static void ack_dynirq(unsigned int irq)
> > move_masked_irq(irq);
> >
> > if (VALID_EVTCHN(evtchn))
> > - unmask_evtchn(evtchn);
> > + clear_evtchn(evtchn);
> > }
> >
> > static int retrigger_irq(unsigned int irq)
>
> These two hunks together don't look right, for two reasons: First,
> ack_dynirq() is used as both .eoi and .ack, but those certainly
> have different requirements (and this might have been a problem
> already before, though I didn't spend much thought on what may
> go wrong).
good point, see below for the fix
> Second, clearing the event channel in the .eoi handler
> after it wasn't masked while being handled has the potential of
> losing an event (if it got raised between the IRQ handler checking
> relevant state and the execution of clear_evtchn()).
This is only true for virqs because xen knows how to handle the case
when a pirq is already inflight.
Considering that this is the way the fasteoi handler is supposed to work
(no ack at the beginning, only at the end) I would keep fasteoi as pirq
handler and switch virqs to edge.
If you look at handle_edge_irq, the ack is always done at the beginning,
no eoi at the end but we don't need it for virqs. Mask and unmask are
done by the handler depending upon IRQ_INPROGRESS and IRQ_DISABLED.
It seems exactly the kind of thing we need as virq handler: we clear the
evtchn in ack and we mask and unmask the evtchn in mask and unmask.
The mapping of xen operations on the irq chip functions is very simple
and natural this way.
Following Jeremy's suggestion I reverted "xen/events: use
PHYSDEVOP_pirq_eoi_gmfn to get pirq need-EOI info"; I am attaching the
patch revert and the new version of this patch (also appended here for
easier read).
---
>From 47b52e85fb0b6edf458e079c604072370a3612f7 Mon Sep 17 00:00:00 2001
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Mon, 18 Oct 2010 15:18:50 +0100
Subject: [PATCH 2/2] xen: do not clear and mask evtchns in
__xen_evtchn_do_upcall
Do not clear and mask evtchns in __xen_evtchn_do_upcall, rely on the
fasteoi and edge handlers to do that for pirqs and virqs respectively.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
---
drivers/xen/events.c | 19 ++++++++-----------
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 5db8e98..3c1b7ae 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -436,12 +436,15 @@ static bool identity_mapped_irq(unsigned irq)
static void pirq_eoi(unsigned int irq)
{
struct physdev_eoi eoi = { .irq = irq };
+ int evtchn = evtchn_from_irq(irq);
if (pirq_needs_eoi(irq)) {
int rc = HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi);
WARN_ON(rc);
}
- unmask_irq(irq);
+
+ if (VALID_EVTCHN(evtchn))
+ clear_evtchn(evtchn);
}
static void pirq_query_unmask(int irq)
@@ -500,6 +503,7 @@ static unsigned int startup_pirq(unsigned int irq)
out:
pirq_eoi(irq);
+ unmask_irq(irq);
return 0;
}
@@ -738,7 +742,7 @@ int bind_evtchn_to_irq(unsigned int evtchn)
irq = find_unbound_irq();
set_irq_chip_and_handler_name(irq, &xen_dynamic_chip,
- handle_fasteoi_irq, "event");
+ handle_edge_irq, "event");
evtchn_to_irq[evtchn] = irq;
irq_info[irq] = mk_evtchn_info(evtchn);
@@ -1062,9 +1066,6 @@ static void __xen_evtchn_do_upcall(struct pt_regs *regs)
int irq = evtchn_to_irq[port];
struct irq_desc *desc;
- mask_evtchn(port);
- clear_evtchn(port);
-
if (irq != -1) {
desc = irq_to_desc(irq);
if (desc)
@@ -1190,7 +1191,7 @@ static void ack_dynirq(unsigned int irq)
move_masked_irq(irq);
if (VALID_EVTCHN(evtchn))
- unmask_evtchn(evtchn);
+ clear_evtchn(evtchn);
}
static int retrigger_irq(unsigned int irq)
@@ -1362,11 +1363,10 @@ void xen_irq_resume(void)
static struct irq_chip xen_dynamic_chip __read_mostly = {
.name = "xen-dyn",
- .disable = mask_irq,
.mask = mask_irq,
.unmask = unmask_irq,
+ .ack = ack_dynirq,
- .eoi = ack_dynirq,
.set_affinity = set_affinity_irq,
.retrigger = retrigger_irq,
};
@@ -1387,10 +1387,7 @@ static struct irq_chip xen_pirq_chip __read_mostly = {
.startup = startup_pirq,
.shutdown = shutdown_pirq,
- .enable = pirq_eoi,
.unmask = unmask_irq,
-
- .disable = mask_irq,
.mask = mask_irq,
.eoi = ack_pirq,
--
1.5.6.5
0001-Revert-xen-events-use-PHYSDEVOP_pirq_eoi_gmfn-to-g.patch
Description: Text Data
0002-xen-do-not-clear-and-mask-evtchns-in-__xen_evtchn_d.patch
Description: Text Data
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home