Re: [PATCH] xen: ensure that all event channels start off bound to VCPU 0

[Ian Campbell <ijc@hellion.org.uk>]

Jeremy,

I think its a bit late for 2.6.36 for this fix. Even though the fix is
pretty obvious and the issue is quite serious if you are unlucky enough
to hit it there seems to have been no reports of anyone seeing it in a
guest domain so I don't think we need to push it in at the last minute.

I plan to send the following to Linus for the 2.6.37 merge window and
request that the fix goes into the various stable trees. Sound good to
you?

The following changes since commit cb655d0f3d57c23db51b981648e452988c0223f9:
  Linus Torvalds (1):
        Linux 2.6.36-rc7

are available in the git repository at:

  git://xenbits.xen.org/people/ianc/linux-2.6.git for-linus/bugfixes

Ian Campbell (1):
      xen: ensure that all event channels start off bound to VCPU 0

 drivers/xen/events.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Thanks,
Ian.

I fleshed out the description a bit since the original post:

On Fri, 2010-10-08 at 17:00 +0100, Ian Campbell wrote:

xen: ensure that all event channels start off bound to VCPU 0

All event channels startbound to VCPU 0 so ensure that cpu_evtchn_mask
is initialised to reflect this. Otherwise there is a race after registering an
event channel but before the affinity is explicitly set where the event channel
can be delivered. If this happens then the event channel remains pending in the
L1 (evtchn_pending) array but is cleared in L2 (evtchn_pending_sel), this means
the event channel cannot be reraised until another event channel happens to
trigger the same L2 entry on that VCPU.
    
sizeof(cpu_evtchn_mask(0))==sizeof(unsigned long*) which is not correct, and
causes only the first 32 or 64 event channels (depending on architecture) to be
initially bound to VCPU0. Use sizeof(struct cpu_evtchn_s) instead.

> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> Cc: stable@kernel.org
> ---
>  drivers/xen/events.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 075e709..175e931 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -290,7 +290,7 @@ static void init_evtchn_cpu_bindings(void)
>  	}
>  #endif
>  
> -	memset(cpu_evtchn_mask(0), ~0, sizeof(cpu_evtchn_mask(0)));
> +	memset(cpu_evtchn_mask(0), ~0, sizeof(struct cpu_evtchn_s));
>  }
>  
>  static inline void clear_evtchn(int port)

-- 
Ian Campbell
Current Noise: Sabbat - I For An Eye

The New England Journal of Medicine reports that 9 out of 10 doctors agree
that 1 out of 10 doctors is an idiot.