Jimi Xenidis
2006-Aug-11 16:36 UTC
[Xen-devel] [PATCH][XEN] Use a union to pack the dual-short combos in an endian neutral way.
The first to members of a grant entry are updated as a combined pair. The following union patch uses a union so updated can happen in an endian neutral fashion. Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com> diff -r 1f611b58729f xen/common/grant_table.c --- a/xen/common/grant_table.c Wed Aug 09 18:53:00 2006 -0400 +++ b/xen/common/grant_table.c Fri Aug 11 12:29:48 2006 -0400 @@ -32,6 +32,17 @@ #include <xen/guest_access.h> #include <acm/acm_hooks.h> +/* The first to members of a grant entry are updated as a combined + * pair. The following union allows that to happen in an endian + * neutral fashion. */ +union grant_combo { + uint32_t word; + struct { + uint16_t flags; + domid_t domid; + } shorts; +}; + #define PIN_FAIL(_lbl, _rc, _f, _a...) \ do { \ DPRINTK( _f, ## _a ); \ @@ -177,7 +188,7 @@ __gnttab_map_grant_ref( */ for ( ; ; ) { - u32 scombo, prev_scombo, new_scombo; + union grant_combo scombo, prev_scombo, new_scombo; if ( unlikely((sflags & GTF_type_mask) != GTF_permit_access) || unlikely(sdom != led->domain->domain_id) ) @@ -186,22 +197,25 @@ __gnttab_map_grant_ref( sflags, sdom, led->domain->domain_id); /* Merge two 16-bit values into a 32-bit combined update. */ - /* NB. Endianness! */ - scombo = ((u32)sdom << 16) | (u32)sflags; - - new_scombo = scombo | GTF_reading; + scombo.shorts.flags = sflags; + scombo.shorts.domid = sdom; + + new_scombo = scombo; + new_scombo.shorts.flags |= GTF_reading; + if ( !(op->flags & GNTMAP_readonly) ) { - new_scombo |= GTF_writing; + new_scombo.shorts.flags |= GTF_writing; if ( unlikely(sflags & GTF_readonly) ) PIN_FAIL(unlock_out, GNTST_general_error, "Attempt to write-pin a r/o grant entry.\n"); } - prev_scombo = cmpxchg((u32 *)&sha->flags, scombo, new_scombo); + prev_scombo.word = cmpxchg((u32 *)&sha->flags, + scombo.word, new_scombo.word); /* Did the combined update work (did we see what we expected?). */ - if ( likely(prev_scombo == scombo) ) + if ( likely(prev_scombo.word == scombo.word) ) break; if ( retries++ == 4 ) @@ -209,9 +223,8 @@ __gnttab_map_grant_ref( "Shared grant entry is unstable.\n"); /* Didn''t see what we expected. Split out the seen flags & dom. */ - /* NB. Endianness! */ - sflags = (u16)prev_scombo; - sdom = (u16)(prev_scombo >> 16); + sflags = prev_scombo.shorts.flags; + sdom = prev_scombo.shorts.domid; } if ( !act->pin ) @@ -532,7 +545,7 @@ gnttab_prepare_for_transfer( struct grant_entry *sha; domid_t sdom; u16 sflags; - u32 scombo, prev_scombo; + union grant_combo scombo, prev_scombo, tmp_scombo; int retries = 0; if ( unlikely((rgt = rd->grant_table) == NULL) || @@ -561,14 +574,16 @@ gnttab_prepare_for_transfer( } /* Merge two 16-bit values into a 32-bit combined update. */ - /* NB. Endianness! */ - scombo = ((u32)sdom << 16) | (u32)sflags; - - prev_scombo = cmpxchg((u32 *)&sha->flags, scombo, - scombo | GTF_transfer_committed); + scombo.shorts.flags = sflags; + scombo.shorts.domid = sdom; + + tmp_scombo = scombo; + tmp_scombo.shorts.flags |= GTF_transfer_committed; + prev_scombo.word = cmpxchg((u32 *)&sha->flags, + scombo.word, tmp_scombo.word); /* Did the combined update work (did we see what we expected?). */ - if ( likely(prev_scombo == scombo) ) + if ( likely(prev_scombo.word == scombo.word) ) break; if ( retries++ == 4 ) @@ -578,9 +593,8 @@ gnttab_prepare_for_transfer( } /* Didn''t see what we expected. Split out the seen flags & dom. */ - /* NB. Endianness! */ - sflags = (u16)prev_scombo; - sdom = (u16)(prev_scombo >> 16); + sflags = prev_scombo.shorts.flags; + sdom = prev_scombo.shorts.domid; } spin_unlock(&rgt->lock); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-Aug-13 17:46 UTC
Re: [Xen-devel] [PATCH][XEN] Use a union to pack the dual-short combos in an endian neutral way.
I think it''s an improvement in readability. Fine by me. -- Keir On 13/8/06 6:50 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:> Any comments on how this was done? > Shall I continue to apply this method to the new grant-table copy > operation? > -JX > On Aug 11, 2006, at 12:36 PM, Jimi Xenidis wrote: > >> >> The first to members of a grant entry are updated as a combined pair. >> The following union patch uses a union so updated can happen in an >> endian neutral fashion. >> >> Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jimi Xenidis
2006-Aug-13 17:50 UTC
Re: [Xen-devel] [PATCH][XEN] Use a union to pack the dual-short combos in an endian neutral way.
Any comments on how this was done? Shall I continue to apply this method to the new grant-table copy operation? -JX On Aug 11, 2006, at 12:36 PM, Jimi Xenidis wrote:> > The first to members of a grant entry are updated as a combined pair. > The following union patch uses a union so updated can happen in an > endian neutral fashion. > > Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com> > > > diff -r 1f611b58729f xen/common/grant_table.c > --- a/xen/common/grant_table.c Wed Aug 09 18:53:00 2006 -0400 > +++ b/xen/common/grant_table.c Fri Aug 11 12:29:48 2006 -0400 > @@ -32,6 +32,17 @@ > #include <xen/guest_access.h> > #include <acm/acm_hooks.h> > > +/* The first to members of a grant entry are updated as a combined > + * pair. The following union allows that to happen in an endian > + * neutral fashion. */ > +union grant_combo { > + uint32_t word; > + struct { > + uint16_t flags; > + domid_t domid; > + } shorts; > +}; > + > #define PIN_FAIL(_lbl, _rc, _f, _a...) \ > do { \ > DPRINTK( _f, ## _a ); \ > @@ -177,7 +188,7 @@ __gnttab_map_grant_ref( > */ > for ( ; ; ) > { > - u32 scombo, prev_scombo, new_scombo; > + union grant_combo scombo, prev_scombo, new_scombo; > > if ( unlikely((sflags & GTF_type_mask) != > GTF_permit_access) || > unlikely(sdom != led->domain->domain_id) ) > @@ -186,22 +197,25 @@ __gnttab_map_grant_ref( > sflags, sdom, led->domain->domain_id); > > /* Merge two 16-bit values into a 32-bit combined > update. */ > - /* NB. Endianness! */ > - scombo = ((u32)sdom << 16) | (u32)sflags; > - > - new_scombo = scombo | GTF_reading; > + scombo.shorts.flags = sflags; > + scombo.shorts.domid = sdom; > + > + new_scombo = scombo; > + new_scombo.shorts.flags |= GTF_reading; > + > if ( !(op->flags & GNTMAP_readonly) ) > { > - new_scombo |= GTF_writing; > + new_scombo.shorts.flags |= GTF_writing; > if ( unlikely(sflags & GTF_readonly) ) > PIN_FAIL(unlock_out, GNTST_general_error, > "Attempt to write-pin a r/o grant > entry.\n"); > } > > - prev_scombo = cmpxchg((u32 *)&sha->flags, scombo, > new_scombo); > + prev_scombo.word = cmpxchg((u32 *)&sha->flags, > + scombo.word, new_scombo.word); > > /* Did the combined update work (did we see what we > expected?). */ > - if ( likely(prev_scombo == scombo) ) > + if ( likely(prev_scombo.word == scombo.word) ) > break; > > if ( retries++ == 4 ) > @@ -209,9 +223,8 @@ __gnttab_map_grant_ref( > "Shared grant entry is unstable.\n"); > > /* Didn''t see what we expected. Split out the seen > flags & dom. */ > - /* NB. Endianness! */ > - sflags = (u16)prev_scombo; > - sdom = (u16)(prev_scombo >> 16); > + sflags = prev_scombo.shorts.flags; > + sdom = prev_scombo.shorts.domid; > } > > if ( !act->pin ) > @@ -532,7 +545,7 @@ gnttab_prepare_for_transfer( > struct grant_entry *sha; > domid_t sdom; > u16 sflags; > - u32 scombo, prev_scombo; > + union grant_combo scombo, prev_scombo, tmp_scombo; > int retries = 0; > > if ( unlikely((rgt = rd->grant_table) == NULL) || > @@ -561,14 +574,16 @@ gnttab_prepare_for_transfer( > } > > /* Merge two 16-bit values into a 32-bit combined update. */ > - /* NB. Endianness! */ > - scombo = ((u32)sdom << 16) | (u32)sflags; > - > - prev_scombo = cmpxchg((u32 *)&sha->flags, scombo, > - scombo | GTF_transfer_committed); > + scombo.shorts.flags = sflags; > + scombo.shorts.domid = sdom; > + > + tmp_scombo = scombo; > + tmp_scombo.shorts.flags |= GTF_transfer_committed; > + prev_scombo.word = cmpxchg((u32 *)&sha->flags, > + scombo.word, tmp_scombo.word); > > /* Did the combined update work (did we see what we > expected?). */ > - if ( likely(prev_scombo == scombo) ) > + if ( likely(prev_scombo.word == scombo.word) ) > break; > > if ( retries++ == 4 ) > @@ -578,9 +593,8 @@ gnttab_prepare_for_transfer( > } > > /* Didn''t see what we expected. Split out the seen flags & > dom. */ > - /* NB. Endianness! */ > - sflags = (u16)prev_scombo; > - sdom = (u16)(prev_scombo >> 16); > + sflags = prev_scombo.shorts.flags; > + sdom = prev_scombo.shorts.domid; > } > > spin_unlock(&rgt->lock); > > _______________________________________________ > 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