Re: [XenPPC] multicast function invocations

[Jimi Xenidis <jimix@xxxxxxxxxxxxxx>]

On Nov 28, 2006, at 3:17 AM, Amos Waterland wrote:

> To summarize the situation, I found two problems.
>
> 1. Core Xen has a bug (I believe) in which they do not mark their cpu
>    mask volatile, so the compiler generates an infinite loop in  
> read_clocks.
>    I will send some patches upstream to resolve this issue.
This was PPc specific bug covered by:
# HG changeset patch
# User Jimi Xenidis <jimix@xxxxxxxxxxxxxx>
# Node ID a8e67a19c3255a6bbc0aaa5c9e1736f1ddd76f6c
# Parent  cc45282daf3d242fdcf6e858c0b18b7f1086a318
cpu_relax() needs to call barrier()

> 2. Xen/PPC has a problem in that its IPI callbacks (remote function
>    invocations) do not actually happen in parallel, which breaks the
>    design of read_clocks.  Our IPI callbacks are serialized by the
>    design we copied from Xen/x86, which is to acquire a per-vector  
> lock
>    very early in the EE handling path (see do_external).
>
>    I guess my real question is: will Xen/PPC ever in the future run  
> its
>    IPI remote function callbacks with EE enabled?  If the plan is to
>    keep things the way they are now, then we should remove the
>    per-vector lock entirely.
The lock protects the vector handler which theoretically could be  
reprogramed.
However we should not be calling the action with the lock held unless  
its per-cpu
> The following is a patch that implements the above two conclusions and
> which allows 'C-aC-aC-at' to work properly.  Comments?
>
> ---
>
>  arch/powerpc/external.c |    2 --
>  common/keyhandler.c     |    2 +-
>  include/xen/cpumask.h   |    2 +-
>  3 files changed, 2 insertions(+), 4 deletions(-)
>
> diff -r 305751a5281e xen/arch/powerpc/external.c
> --- a/xen/arch/powerpc/external.c       Wed Nov 22 16:29:25 2006 -0500
> +++ b/xen/arch/powerpc/external.c       Tue Nov 28 03:07:10 2006 -0500
> @@ -86,11 +86,9 @@ void do_external(struct cpu_user_regs *r
>          /* do_IRQ is fundamentally broken for reliable IPI  
> delivery.  */
>          irq_desc_t *desc = &irq_desc[vec];
>          regs->entry_vector = vec;
> -        spin_lock(&desc->lock);
>          desc->handler->ack(vec);
>          desc->action->handler(vector_to_irq(vec), desc->action- 
> >dev_id, regs);
>          desc->handler->end(vec);
> -        spin_unlock(&desc->lock);
Please add a:
   BUG_ON(!(desc->status & IRQ_PER_CPU));

If you trip this then we will need:
 - locking around the desc
 - tickle some status bits
 - unlock the action
 - lock again
 - tickle status bits.
...



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