From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hollis Blanchard Subject: Re: [patch] make evtchn_upcall_pending arch-specific type Date: Tue, 13 Jun 2006 15:02:23 -0500 Message-ID: <1150228944.31548.50.camel@basalt.austin.ibm.com> References: <200603301313.44384.hollisb@us.ibm.com> <5764fe373f0ae77a75e8a9a8e2826900@cl.cam.ac.uk> <5BD25DCF-992A-4C95-B349-622F213EA4F4@watson.ibm.com> <7fbc6e4ae20a33508b54720a1a8821cc@cl.cam.ac.uk> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <7fbc6e4ae20a33508b54720a1a8821cc@cl.cam.ac.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: Jimi Xenidis , xen-devel@lists.xensource.com, xen-ppc-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On Sat, 2006-04-01 at 10:30 +0100, Keir Fraser wrote: > On 1 Apr 2006, at 04:55, Jimi Xenidis wrote: > > >> Before going down this route, what about just casting the field to > >> long, since it is actually aligned on a suitable boundary, as it > >> happens? > > > > We tried that, but to get the right bit we would have to use 56 not 0. > > Actually, evtchn_upcall_pending is touched in very few places in Xen > common code. Using bit 56 is not very pretty but should be easy to hide > behind a macro? You can hide the cast to long inside the same macro. > You already need to arch-dep the event_pending() macro, for your MSR.EE > check, and that's one of the main ways in which common code accesses > the pending flag. For reference, here's the PPC implementation: /* HACK: evtchn_upcall_pending is only a byte, but our atomic instructions only * store in 4/8 byte quantities. However, because evtchn_upcall_pending is part * of the guest ABI, we can't change its size without breaking backwards * compatibility. In this particular case, struct vcpu_info is big enough that * we can safely store a full long into it. However, note that bit 0 of that * byte is bit 56 when cast to a long. */ static inline int evtchn_test_and_set_pending(struct vcpu *v) { unsigned long *l = (unsigned long *)&v->vcpu_info->evtchn_upcall_pending; return test_and_set_bit(56, l); } You'll notice I added an alignment attribute to that field because it really really needs to be aligned for us. This has only been build-tested on x86! If it's acceptable, please apply. Hide evtchn_upcall_pending test-and-set accesses behind a wrapper. Signed-off-by: Hollis Blanchard diff -r aa088739693a xen/common/event_channel.c --- a/xen/common/event_channel.c Tue Jun 13 10:48:08 2006 -0500 +++ b/xen/common/event_channel.c Tue Jun 13 14:52:05 2006 -0500 @@ -494,7 +494,7 @@ void evtchn_set_pending(struct vcpu *v, if ( !test_bit (port, s->evtchn_mask) && !test_and_set_bit(port / BITS_PER_LONG, &v->vcpu_info->evtchn_pending_sel) && - !test_and_set_bit(0, &v->vcpu_info->evtchn_upcall_pending) ) + !evtchn_test_and_set_pending(v) ) { evtchn_notify(v); } @@ -683,7 +683,7 @@ static long evtchn_unmask(evtchn_unmask_ test_bit (port, s->evtchn_pending) && !test_and_set_bit (port / BITS_PER_LONG, &v->vcpu_info->evtchn_pending_sel) && - !test_and_set_bit (0, &v->vcpu_info->evtchn_upcall_pending) ) + !evtchn_test_and_set_pending(v) ) { evtchn_notify(v); } diff -r aa088739693a xen/include/asm-ia64/event.h --- a/xen/include/asm-ia64/event.h Tue Jun 13 10:48:08 2006 -0500 +++ b/xen/include/asm-ia64/event.h Tue Jun 13 14:52:05 2006 -0500 @@ -74,4 +74,9 @@ static inline int arch_virq_is_global(in return rc; } +static inline int evtchn_test_and_set_pending(struct vcpu *v) +{ + return test_and_set_bit(0, &v->vcpu_info->evtchn_upcall_pending); +} + #endif diff -r aa088739693a xen/include/asm-x86/event.h --- a/xen/include/asm-x86/event.h Tue Jun 13 10:48:08 2006 -0500 +++ b/xen/include/asm-x86/event.h Tue Jun 13 14:52:05 2006 -0500 @@ -55,4 +55,9 @@ static inline int arch_virq_is_global(in return 1; } +static inline int evtchn_test_and_set_pending(struct vcpu *v) +{ + return test_and_set_bit(0, &v->vcpu_info->evtchn_upcall_pending); +} + #endif diff -r aa088739693a xen/include/public/xen.h --- a/xen/include/public/xen.h Tue Jun 13 10:48:08 2006 -0500 +++ b/xen/include/public/xen.h Tue Jun 13 14:52:05 2006 -0500 @@ -363,7 +363,7 @@ struct vcpu_info { * an upcall activation. The mask is cleared when the VCPU requests * to block: this avoids wakeup-waiting races. */ - uint8_t evtchn_upcall_pending; + uint8_t evtchn_upcall_pending __attribute__((aligned(sizeof(long)))); uint8_t evtchn_upcall_mask; unsigned long evtchn_pending_sel; struct arch_vcpu_info arch; -- Hollis Blanchard IBM Linux Technology Center