Xen devel - Jul 2010 - [PATCH] Dont'' round-robin the callback interrupt

# HG changeset patch
# User Paul Durrant <paul.durrant@citrix.com>
# Date 1278948846 -3600
# Node ID 2dbd30d4027eeafc41fa46638679eb7e3e1bc951
# Parent  f12837d7a50e3e5f843bd1a7113bb329661c7dd0
Dont'' round-robin the callback interrupt.

Arrange that the event channel callback interrupt always goes to the
lowest vcpu with a matching local apic. This should, in most cases,
be VCPU0 (to which all event channels are bound for HVM guests) but
this cannot be guaranteed.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
CC: Tim Deegan <tim.deegan@citrix.com>

diff -r f12837d7a50e -r 2dbd30d4027e xen/arch/x86/hvm/vioapic.c
--- a/xen/arch/x86/hvm/vioapic.c	Mon Jul 12 10:48:34 2010 +0100
+++ b/xen/arch/x86/hvm/vioapic.c	Mon Jul 12 16:34:06 2010 +0100
@@ -276,6 +276,29 @@
     return pt_active(&pit->pt0);
 }
 
+static int is_callback_via(struct domain *d, int irq)
+{
+    struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq;
+
+    switch ( hvm_irq->callback_via_type )
+    {
+    case HVMIRQ_callback_gsi: {
+        unsigned int gsi = hvm_irq->callback_via.gsi;
+
+        return irq == gsi;
+    }
+    case HVMIRQ_callback_pci_intx: {
+        unsigned int pdev  = hvm_irq->callback_via.pci.dev;
+        unsigned int pintx = hvm_irq->callback_via.pci.intx;
+
+        return irq == hvm_pci_intx_gsi(pdev, pintx);
+    }
+    case HVMIRQ_callback_vector:
+    default:
+        return 0;
+    }
+}
+
 static void vioapic_deliver(struct hvm_hw_vioapic *vioapic, int irq)
 {
     uint16_t dest = vioapic->redirtbl[irq].fields.dest_id;
@@ -307,7 +330,11 @@
         }
         else
 #endif
+        if ( is_callback_via(d, irq) )
+            target = vlapic_lowest_vcpu(d, NULL, 0, dest, dest_mode);
+        else
             target = vlapic_lowest_prio(d, NULL, 0, dest, dest_mode);
+
         if ( target != NULL )
         {
             ioapic_inj_irq(vioapic, target, vector, trig_mode, delivery_mode);
diff -r f12837d7a50e -r 2dbd30d4027e xen/arch/x86/hvm/vlapic.c
--- a/xen/arch/x86/hvm/vlapic.c	Mon Jul 12 10:48:34 2010 +0100
+++ b/xen/arch/x86/hvm/vlapic.c	Mon Jul 12 16:34:06 2010 +0100
@@ -354,6 +354,27 @@
     if ( target != NULL )
         d->arch.hvm_domain.irq.round_robin_prev_vcpu             
vlapic_vcpu(target)->vcpu_id;
+
+    return target;
+}
+
+struct vlapic *vlapic_lowest_vcpu(
+    struct domain *d, struct vlapic *source,
+    int short_hand, uint8_t dest, uint8_t dest_mode)
+{
+    struct vlapic *vlapic, *target = NULL;
+    struct vcpu *v;
+
+    if ( unlikely(!d->vcpu) )
+        return NULL;
+
+    for ( v = d->vcpu[0]; v; v = v->next_in_list ) {
+        vlapic = vcpu_vlapic(v);
+        if ( vlapic_match_dest(vlapic, source, short_hand, dest, dest_mode)
&&
+             vlapic_enabled(vlapic) ) {
+            target = vlapic;
+        }
+    }
 
     return target;
 }
diff -r f12837d7a50e -r 2dbd30d4027e xen/include/asm-x86/hvm/vlapic.h
--- a/xen/include/asm-x86/hvm/vlapic.h	Mon Jul 12 10:48:34 2010 +0100
+++ b/xen/include/asm-x86/hvm/vlapic.h	Mon Jul 12 16:34:06 2010 +0100
@@ -100,6 +100,10 @@
     struct domain *d, struct vlapic *source,
     int short_hand, uint8_t dest, uint8_t dest_mode);
 
+struct vlapic *vlapic_lowest_vcpu(
+    struct domain *d, struct vlapic *source,
+    int short_hand, uint8_t dest, uint8_t dest_mode);
+
 bool_t vlapic_match_dest(
     struct vlapic *target, struct vlapic *source,
     int short_hand, uint8_t dest, uint8_t dest_mode);

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

Oops. Sorry, don''t apply that... missing break statement. Will resend
shortly.

  Paul
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-
> bounces@lists.xensource.com] On Behalf Of Paul Durrant
> Sent: 12 July 2010 16:42
> To: xen-devel@lists.xensource.com
> Cc: Tim Deegan
> Subject: [Xen-devel] [PATCH] Dont'' round-robin the callback
> interrupt
> 
> # HG changeset patch
> # User Paul Durrant <paul.durrant@citrix.com>
> # Date 1278948846 -3600
> # Node ID 2dbd30d4027eeafc41fa46638679eb7e3e1bc951
> # Parent  f12837d7a50e3e5f843bd1a7113bb329661c7dd0
> Dont'' round-robin the callback interrupt.
> 
> Arrange that the event channel callback interrupt always goes to the
> lowest vcpu with a matching local apic. This should, in most cases,
> be VCPU0 (to which all event channels are bound for HVM guests) but
> this cannot be guaranteed.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> CC: Tim Deegan <tim.deegan@citrix.com>
> 
> diff -r f12837d7a50e -r 2dbd30d4027e xen/arch/x86/hvm/vioapic.c
> --- a/xen/arch/x86/hvm/vioapic.c	Mon Jul 12 10:48:34 2010 +0100
> +++ b/xen/arch/x86/hvm/vioapic.c	Mon Jul 12 16:34:06 2010 +0100
> @@ -276,6 +276,29 @@
>      return pt_active(&pit->pt0);
>  }
> 
> +static int is_callback_via(struct domain *d, int irq)
> +{
> +    struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq;
> +
> +    switch ( hvm_irq->callback_via_type )
> +    {
> +    case HVMIRQ_callback_gsi: {
> +        unsigned int gsi = hvm_irq->callback_via.gsi;
> +
> +        return irq == gsi;
> +    }
> +    case HVMIRQ_callback_pci_intx: {
> +        unsigned int pdev  = hvm_irq->callback_via.pci.dev;
> +        unsigned int pintx = hvm_irq->callback_via.pci.intx;
> +
> +        return irq == hvm_pci_intx_gsi(pdev, pintx);
> +    }
> +    case HVMIRQ_callback_vector:
> +    default:
> +        return 0;
> +    }
> +}
> +
>  static void vioapic_deliver(struct hvm_hw_vioapic *vioapic, int
> irq)
>  {
>      uint16_t dest = vioapic->redirtbl[irq].fields.dest_id;
> @@ -307,7 +330,11 @@
>          }
>          else
>  #endif
> +        if ( is_callback_via(d, irq) )
> +            target = vlapic_lowest_vcpu(d, NULL, 0, dest,
> dest_mode);
> +        else
>              target = vlapic_lowest_prio(d, NULL, 0, dest,
> dest_mode);
> +
>          if ( target != NULL )
>          {
>              ioapic_inj_irq(vioapic, target, vector, trig_mode,
> delivery_mode);
> diff -r f12837d7a50e -r 2dbd30d4027e xen/arch/x86/hvm/vlapic.c
> --- a/xen/arch/x86/hvm/vlapic.c	Mon Jul 12 10:48:34 2010 +0100
> +++ b/xen/arch/x86/hvm/vlapic.c	Mon Jul 12 16:34:06 2010 +0100
> @@ -354,6 +354,27 @@
>      if ( target != NULL )
>          d->arch.hvm_domain.irq.round_robin_prev_vcpu >             
vlapic_vcpu(target)->vcpu_id;
> +
> +    return target;
> +}
> +
> +struct vlapic *vlapic_lowest_vcpu(
> +    struct domain *d, struct vlapic *source,
> +    int short_hand, uint8_t dest, uint8_t dest_mode)
> +{
> +    struct vlapic *vlapic, *target = NULL;
> +    struct vcpu *v;
> +
> +    if ( unlikely(!d->vcpu) )
> +        return NULL;
> +
> +    for ( v = d->vcpu[0]; v; v = v->next_in_list ) {
> +        vlapic = vcpu_vlapic(v);
> +        if ( vlapic_match_dest(vlapic, source, short_hand, dest,
> dest_mode) &&
> +             vlapic_enabled(vlapic) ) {
> +            target = vlapic;
> +        }
> +    }
> 
>      return target;
>  }
> diff -r f12837d7a50e -r 2dbd30d4027e xen/include/asm-
> x86/hvm/vlapic.h
> --- a/xen/include/asm-x86/hvm/vlapic.h	Mon Jul 12 10:48:34 2010
> +0100
> +++ b/xen/include/asm-x86/hvm/vlapic.h	Mon Jul 12 16:34:06 2010
> +0100
> @@ -100,6 +100,10 @@
>      struct domain *d, struct vlapic *source,
>      int short_hand, uint8_t dest, uint8_t dest_mode);
> 
> +struct vlapic *vlapic_lowest_vcpu(
> +    struct domain *d, struct vlapic *source,
> +    int short_hand, uint8_t dest, uint8_t dest_mode);
> +
>  bool_t vlapic_match_dest(
>      struct vlapic *target, struct vlapic *source,
>      int short_hand, uint8_t dest, uint8_t dest_mode);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

# HG changeset patch
# User Paul Durrant <paul.durrant@citrix.com>
# Date 1278949925 -3600
# Node ID 0f3d7e651362dfe76a98c248d0f71795f624ed78
# Parent  f12837d7a50e3e5f843bd1a7113bb329661c7dd0
Dont'' round-robin the callback interrupt.

Arrange that the event channel callback interrupt always goes to the
lowest vcpu with a matching local apic. This should, in most cases,
be VCPU0 (to which all event channels are bound for HVM guests) but
this cannot be guaranteed.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
CC: Tim Deegan <tim.deegan@citrix.com>

diff -r f12837d7a50e -r 0f3d7e651362 xen/arch/x86/hvm/vioapic.c
--- a/xen/arch/x86/hvm/vioapic.c	Mon Jul 12 10:48:34 2010 +0100
+++ b/xen/arch/x86/hvm/vioapic.c	Mon Jul 12 16:52:05 2010 +0100
@@ -276,6 +276,29 @@
     return pt_active(&pit->pt0);
 }
 
+static int is_callback_via(struct domain *d, int irq)
+{
+    struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq;
+
+    switch ( hvm_irq->callback_via_type )
+    {
+    case HVMIRQ_callback_gsi: {
+        unsigned int gsi = hvm_irq->callback_via.gsi;
+
+        return irq == gsi;
+    }
+    case HVMIRQ_callback_pci_intx: {
+        unsigned int pdev  = hvm_irq->callback_via.pci.dev;
+        unsigned int pintx = hvm_irq->callback_via.pci.intx;
+
+        return irq == hvm_pci_intx_gsi(pdev, pintx);
+    }
+    case HVMIRQ_callback_vector:
+    default:
+        return 0;
+    }
+}
+
 static void vioapic_deliver(struct hvm_hw_vioapic *vioapic, int irq)
 {
     uint16_t dest = vioapic->redirtbl[irq].fields.dest_id;
@@ -307,7 +330,11 @@
         }
         else
 #endif
+        if ( is_callback_via(d, irq) )
+            target = vlapic_lowest_vcpu(d, NULL, 0, dest, dest_mode);
+        else
             target = vlapic_lowest_prio(d, NULL, 0, dest, dest_mode);
+
         if ( target != NULL )
         {
             ioapic_inj_irq(vioapic, target, vector, trig_mode, delivery_mode);
diff -r f12837d7a50e -r 0f3d7e651362 xen/arch/x86/hvm/vlapic.c
--- a/xen/arch/x86/hvm/vlapic.c	Mon Jul 12 10:48:34 2010 +0100
+++ b/xen/arch/x86/hvm/vlapic.c	Mon Jul 12 16:52:05 2010 +0100
@@ -354,6 +354,28 @@
     if ( target != NULL )
         d->arch.hvm_domain.irq.round_robin_prev_vcpu             
vlapic_vcpu(target)->vcpu_id;
+
+    return target;
+}
+
+struct vlapic *vlapic_lowest_vcpu(
+    struct domain *d, struct vlapic *source,
+    int short_hand, uint8_t dest, uint8_t dest_mode)
+{
+    struct vlapic *vlapic, *target = NULL;
+    struct vcpu *v;
+
+    if ( unlikely(!d->vcpu) )
+        return NULL;
+
+    for ( v = d->vcpu[0]; v; v = v->next_in_list ) {
+        vlapic = vcpu_vlapic(v);
+        if ( vlapic_match_dest(vlapic, source, short_hand, dest, dest_mode)
&&
+             vlapic_enabled(vlapic) ) {
+            target = vlapic;
+            break;
+        }
+    }
 
     return target;
 }
diff -r f12837d7a50e -r 0f3d7e651362 xen/include/asm-x86/hvm/vlapic.h
--- a/xen/include/asm-x86/hvm/vlapic.h	Mon Jul 12 10:48:34 2010 +0100
+++ b/xen/include/asm-x86/hvm/vlapic.h	Mon Jul 12 16:52:05 2010 +0100
@@ -100,6 +100,10 @@
     struct domain *d, struct vlapic *source,
     int short_hand, uint8_t dest, uint8_t dest_mode);
 
+struct vlapic *vlapic_lowest_vcpu(
+    struct domain *d, struct vlapic *source,
+    int short_hand, uint8_t dest, uint8_t dest_mode);
+
 bool_t vlapic_match_dest(
     struct vlapic *target, struct vlapic *source,
     int short_hand, uint8_t dest, uint8_t dest_mode);

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

On 12/07/2010 16:52, "Paul Durrant" <paul.durrant@citrix.com>
wrote:
> Dont'' round-robin the callback interrupt.
> 
> Arrange that the event channel callback interrupt always goes to the
> lowest vcpu with a matching local apic. This should, in most cases,
> be VCPU0 (to which all event channels are bound for HVM guests) but
> this cannot be guaranteed.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> CC: Tim Deegan <tim.deegan@citrix.com>
PV drivers should be handling callback on CPU != 0. The example Linux PV
drivers have done that for a very long time indeed. If a workaround is
needed for broken drivers then we need some way to gate it, and we should in
that case force delivery to VCPU0, as we do for some timer interrupts. The
forcing appears to do no harm even if not architecturally correct, and after
all we would be going to these lengths only because delivery to VCPU-not-0
*certainly* doesn''t work.

 -- Keir



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

On 12/07/2010 17:05, "Keir Fraser" <keir.fraser@eu.citrix.com>
wrote:
> On 12/07/2010 16:52, "Paul Durrant"
<paul.durrant@citrix.com> wrote:
> 
>> Dont'' round-robin the callback interrupt.
>> 
>> Arrange that the event channel callback interrupt always goes to the
>> lowest vcpu with a matching local apic. This should, in most cases,
>> be VCPU0 (to which all event channels are bound for HVM guests) but
>> this cannot be guaranteed.
>> 
>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>> CC: Tim Deegan <tim.deegan@citrix.com>
> 
> PV drivers should be handling callback on CPU != 0. The example Linux PV
> drivers have done that for a very long time indeed. If a workaround is
> needed for broken drivers then we need some way to gate it, and we should
in
> that case force delivery to VCPU0, as we do for some timer interrupts. The
> forcing appears to do no harm even if not architecturally correct, and
after
> all we would be going to these lengths only because delivery to VCPU-not-0
> *certainly* doesn''t work.
Actually given this is probably XS/XCP Win drivers we''re talking about,
aren''t they expected to be upgraded when moving to an upgraded host? In
which case the fix should be in the upgraded drivers?

 -- Keir




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

Keir,

  When we connect our platform driver interrupt, Windows is at liberty to
allocate vectors on as many or as few cpus as it wishes. I''ve seen
cases where it will *not* allocate us a vector on vcpu 0, so we cannot force
vcpu 0. Older frontends assume vcpu 0, but should function ok if the interrupt
does not move around.
  However, that''s not the motivation for this patch. In the windows
code, we only bind event channels to vcpu 0 since we cannot get callback
interrupts on multiple vcpus simultaneously, since the interrupt is level
sensitive. Thus round-robining is wasteful in terms of kicking certain data
structures between caches (assuming a reasonably constant vcpu -> pcpu
mapping).

  Paul
> -----Original Message-----
> From: Keir Fraser
> Sent: 12 July 2010 17:16
> To: Paul Durrant; xen-devel@lists.xensource.com
> Cc: Tim Deegan
> Subject: Re: [Xen-devel] [PATCH] Dont'' round-robin the callback
> interrupt
> 
> On 12/07/2010 17:05, "Keir Fraser"
<keir.fraser@eu.citrix.com>
> wrote:
> 
> > On 12/07/2010 16:52, "Paul Durrant"
<paul.durrant@citrix.com>
> wrote:
> >
> >> Dont'' round-robin the callback interrupt.
> >>
> >> Arrange that the event channel callback interrupt always goes to
> the
> >> lowest vcpu with a matching local apic. This should, in most
> cases,
> >> be VCPU0 (to which all event channels are bound for HVM guests)
> but
> >> this cannot be guaranteed.
> >>
> >> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >> CC: Tim Deegan <tim.deegan@citrix.com>
> >
> > PV drivers should be handling callback on CPU != 0. The example
> Linux PV
> > drivers have done that for a very long time indeed. If a
> workaround is
> > needed for broken drivers then we need some way to gate it, and we
> should in
> > that case force delivery to VCPU0, as we do for some timer
> interrupts. The
> > forcing appears to do no harm even if not architecturally correct,
> and after
> > all we would be going to these lengths only because delivery to
> VCPU-not-0
> > *certainly* doesn''t work.
> 
> Actually given this is probably XS/XCP Win drivers we''re talking
> about,
> aren''t they expected to be upgraded when moving to an upgraded
host?
> In
> which case the fix should be in the upgraded drivers?
> 
>  -- Keir
> 
> 

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

On 12/07/2010 17:36, "Paul Durrant" <Paul.Durrant@citrix.com>
wrote:
>   When we connect our platform driver interrupt, Windows is at liberty to
> allocate vectors on as many or as few cpus as it wishes. I''ve seen
cases where
> it will *not* allocate us a vector on vcpu 0, so we cannot force vcpu 0.
Older
> frontends assume vcpu 0, but should function ok if the interrupt does not
move
> around.
>   However, that''s not the motivation for this patch. In the
windows code, we
> only bind event channels to vcpu 0 since we cannot get callback interrupts
on
> multiple vcpus simultaneously, since the interrupt is level sensitive. Thus
> round-robining is wasteful in terms of kicking certain data structures
between
> caches (assuming a reasonably constant vcpu -> pcpu mapping).
Surely that argument can be made for any interrupt that is set up to
round-robin among multiple CPUs? Obviously in the PV drivers case the
event-channel IRQ is probably the only significant source of round-robin
interrupts. But I don''t see that it''s special in any other
way.

 -- Keir



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

On 12/07/2010 18:17, "Keir Fraser" <keir.fraser@eu.citrix.com>
wrote:
>>   However, that''s not the motivation for this patch. In the
windows code, we
>> only bind event channels to vcpu 0 since we cannot get callback
interrupts on
>> multiple vcpus simultaneously, since the interrupt is level sensitive.
Thus
>> round-robining is wasteful in terms of kicking certain data structures
>> between
>> caches (assuming a reasonably constant vcpu -> pcpu mapping).
> 
> Surely that argument can be made for any interrupt that is set up to
> round-robin among multiple CPUs? Obviously in the PV drivers case the
> event-channel IRQ is probably the only significant source of round-robin
> interrupts. But I don''t see that it''s special in any
other way.
Further, the correct semantics for LowestPrio delivery was implemented by
Juergen Gross at Fujitsu for a reason. Cc''ing him. I suspect he will
say
that relaxing the delivery semantics will cause something he cares about to
break.

 -- Keir



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

Just to be clear. I''m only modifying the behavior of the irq being used
for callback, so this should not affect pass through devices.

  Paul
> -----Original Message-----
> From: Keir Fraser
> Sent: 12 July 2010 18:41
> To: Keir Fraser; Paul Durrant; xen-devel@lists.xensource.com
> Cc: Tim Deegan; Juergen Gross
> Subject: Re: [Xen-devel] [PATCH] Dont'' round-robin the callback
> interrupt
> 
> On 12/07/2010 18:17, "Keir Fraser"
<keir.fraser@eu.citrix.com>
> wrote:
> 
> >>   However, that''s not the motivation for this patch. In
the
> windows code, we
> >> only bind event channels to vcpu 0 since we cannot get callback
> interrupts on
> >> multiple vcpus simultaneously, since the interrupt is level
> sensitive. Thus
> >> round-robining is wasteful in terms of kicking certain data
> structures
> >> between
> >> caches (assuming a reasonably constant vcpu -> pcpu mapping).
> >
> > Surely that argument can be made for any interrupt that is set up
> to
> > round-robin among multiple CPUs? Obviously in the PV drivers case
> the
> > event-channel IRQ is probably the only significant source of
> round-robin
> > interrupts. But I don''t see that it''s special in any
other way.
> 
> Further, the correct semantics for LowestPrio delivery was
> implemented by
> Juergen Gross at Fujitsu for a reason. Cc''ing him. I suspect he
will
> say
> that relaxing the delivery semantics will cause something he cares
> about to
> break.
> 
>  -- Keir
> 

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

Ok, I can buy that argument to a degree, but in the case of the PV drivers *all*
our interrupts are multiplexed through a single irq, so we get no benefit from
round robining which systems using multiple irqs might.

  Paul
> -----Original Message-----
> From: Keir Fraser
> Sent: 12 July 2010 18:18
> To: Paul Durrant; xen-devel@lists.xensource.com
> Cc: Tim Deegan
> Subject: Re: [Xen-devel] [PATCH] Dont'' round-robin the callback
> interrupt
> 
> On 12/07/2010 17:36, "Paul Durrant"
<Paul.Durrant@citrix.com> wrote:
> 
> >   When we connect our platform driver interrupt, Windows is at
> liberty to
> > allocate vectors on as many or as few cpus as it wishes. I''ve
seen
> cases where
> > it will *not* allocate us a vector on vcpu 0, so we cannot force
> vcpu 0. Older
> > frontends assume vcpu 0, but should function ok if the interrupt
> does not move
> > around.
> >   However, that''s not the motivation for this patch. In the
> windows code, we
> > only bind event channels to vcpu 0 since we cannot get callback
> interrupts on
> > multiple vcpus simultaneously, since the interrupt is level
> sensitive. Thus
> > round-robining is wasteful in terms of kicking certain data
> structures between
> > caches (assuming a reasonably constant vcpu -> pcpu mapping).
> 
> Surely that argument can be made for any interrupt that is set up to
> round-robin among multiple CPUs? Obviously in the PV drivers case
> the
> event-channel IRQ is probably the only significant source of round-
> robin
> interrupts. But I don''t see that it''s special in any
other way.
> 
>  -- Keir
> 

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

On 07/12/2010 07:41 PM, Keir Fraser wrote:
> On 12/07/2010 18:17, "Keir
Fraser"<keir.fraser@eu.citrix.com>  wrote:
>
>>>    However, that''s not the motivation for this patch. In
the windows code, we
>>> only bind event channels to vcpu 0 since we cannot get callback
interrupts on
>>> multiple vcpus simultaneously, since the interrupt is level
sensitive. Thus
>>> round-robining is wasteful in terms of kicking certain data
structures
>>> between
>>> caches (assuming a reasonably constant vcpu ->  pcpu mapping).
>>
>> Surely that argument can be made for any interrupt that is set up to
>> round-robin among multiple CPUs? Obviously in the PV drivers case the
>> event-channel IRQ is probably the only significant source of
round-robin
>> interrupts. But I don''t see that it''s special in any
other way.
>
> Further, the correct semantics for LowestPrio delivery was implemented by
> Juergen Gross at Fujitsu for a reason. Cc''ing him. I suspect he
will say
> that relaxing the delivery semantics will cause something he cares about to
> break.
Thanks for CC''ing me, Keir.
Selecting different CPUs gives at least our BS2000 system a performance win of
a few percent. As Keir already said, that''s the reason I implemented
the LPP
delivery of interrupts.
If you really need a different interrupt delivery behaviour I would at least
recommend a per-domain parameter for violating the correct semantics using the
LPP delivery as default.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

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

No, I think that''s getting a little too ugly. It looks like I
can''t have a solution that both caters for older
''broken'' frontends and new ones too. I guess we''ll
just keep the patch in XS for the next release and I''ll fix the
frontends to affinitize to only one vcpu for subsequent releases.

  Paul
> -----Original Message-----
> From: Juergen Gross [mailto:juergen.gross@ts.fujitsu.com]
> Sent: 13 July 2010 06:00
> To: Keir Fraser
> Cc: Paul Durrant; xen-devel@lists.xensource.com; Tim Deegan
> Subject: Re: [Xen-devel] [PATCH] Dont'' round-robin the callback
> interrupt
> 
> On 07/12/2010 07:41 PM, Keir Fraser wrote:
> > On 12/07/2010 18:17, "Keir
Fraser"<keir.fraser@eu.citrix.com>
> wrote:
> >
> >>>    However, that''s not the motivation for this patch.
In the
> windows code, we
> >>> only bind event channels to vcpu 0 since we cannot get
callback
> interrupts on
> >>> multiple vcpus simultaneously, since the interrupt is level
> sensitive. Thus
> >>> round-robining is wasteful in terms of kicking certain data
> structures
> >>> between
> >>> caches (assuming a reasonably constant vcpu ->  pcpu
mapping).
> >>
> >> Surely that argument can be made for any interrupt that is set up
> to
> >> round-robin among multiple CPUs? Obviously in the PV drivers case
> the
> >> event-channel IRQ is probably the only significant source of
> round-robin
> >> interrupts. But I don''t see that it''s special in
any other way.
> >
> > Further, the correct semantics for LowestPrio delivery was
> implemented by
> > Juergen Gross at Fujitsu for a reason. Cc''ing him. I suspect
he
> will say
> > that relaxing the delivery semantics will cause something he cares
> about to
> > break.
> 
> Thanks for CC''ing me, Keir.
> Selecting different CPUs gives at least our BS2000 system a
> performance win of
> a few percent. As Keir already said, that''s the reason I
implemented
> the LPP
> delivery of interrupts.
> If you really need a different interrupt delivery behaviour I would
> at least
> recommend a per-domain parameter for violating the correct semantics
> using the
> LPP delivery as default.
> 
> 
> Juergen
> 
> --
> Juergen Gross                 Principal Developer Operating Systems
> TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222
> 2967
> Fujitsu Technology Solutions              e-mail:
> juergen.gross@ts.fujitsu.com
> Domagkstr. 28                           Internet: ts.fujitsu.com
> D-80807 Muenchen                 Company details:
> ts.fujitsu.com/imprint.html
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yes, if it''s possible to make this optimisation in the PV driver
itself,
then I think that is the better path.

 -- Keir

On 13/07/2010 09:02, "Paul Durrant" <Paul.Durrant@citrix.com>
wrote:
> No, I think that''s getting a little too ugly. It looks like I
can''t have a
> solution that both caters for older ''broken'' frontends
and new ones too. I
> guess we''ll just keep the patch in XS for the next release and
I''ll fix the
> frontends to affinitize to only one vcpu for subsequent releases.
> 
>   Paul
> 
>> -----Original Message-----
>> From: Juergen Gross [mailto:juergen.gross@ts.fujitsu.com]
>> Sent: 13 July 2010 06:00
>> To: Keir Fraser
>> Cc: Paul Durrant; xen-devel@lists.xensource.com; Tim Deegan
>> Subject: Re: [Xen-devel] [PATCH] Dont'' round-robin the
callback
>> interrupt
>> 
>> On 07/12/2010 07:41 PM, Keir Fraser wrote:
>>> On 12/07/2010 18:17, "Keir
Fraser"<keir.fraser@eu.citrix.com>
>> wrote:
>>> 
>>>>>    However, that''s not the motivation for this
patch. In the
>> windows code, we
>>>>> only bind event channels to vcpu 0 since we cannot get
callback
>> interrupts on
>>>>> multiple vcpus simultaneously, since the interrupt is level
>> sensitive. Thus
>>>>> round-robining is wasteful in terms of kicking certain data
>> structures
>>>>> between
>>>>> caches (assuming a reasonably constant vcpu ->  pcpu
mapping).
>>>> 
>>>> Surely that argument can be made for any interrupt that is set
up
>> to
>>>> round-robin among multiple CPUs? Obviously in the PV drivers
case
>> the
>>>> event-channel IRQ is probably the only significant source of
>> round-robin
>>>> interrupts. But I don''t see that it''s special
in any other way.
>>> 
>>> Further, the correct semantics for LowestPrio delivery was
>> implemented by
>>> Juergen Gross at Fujitsu for a reason. Cc''ing him. I
suspect he
>> will say
>>> that relaxing the delivery semantics will cause something he cares
>> about to
>>> break.
>> 
>> Thanks for CC''ing me, Keir.
>> Selecting different CPUs gives at least our BS2000 system a
>> performance win of
>> a few percent. As Keir already said, that''s the reason I
implemented
>> the LPP
>> delivery of interrupts.
>> If you really need a different interrupt delivery behaviour I would
>> at least
>> recommend a per-domain parameter for violating the correct semantics
>> using the
>> LPP delivery as default.
>> 
>> 
>> Juergen
>> 
>> --
>> Juergen Gross                 Principal Developer Operating Systems
>> TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222
>> 2967
>> Fujitsu Technology Solutions              e-mail:
>> juergen.gross@ts.fujitsu.com
>> Domagkstr. 28                           Internet: ts.fujitsu.com
>> D-80807 Muenchen                 Company details:
>> ts.fujitsu.com/imprint.html


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