Hollis Blanchard
2006-Nov-16 15:55 UTC
[Xen-devel] Re: [Xen-changelog] [xen-unstable] [IA64] fix xencomm_handle_is_null().
On Wed, 2006-11-15 at 00:40 +0000, Xen patchbot-unstable wrote:> # HG changeset patch > # User awilliam@xenbuild.aw > # Node ID 3713ea43e6364bf2989a1cf0dc252a6a1393ce02 > # Parent 8533c59c5b40df0f6e6d03cbc8a74057dcbed5db > [IA64] fix xencomm_handle_is_null(). > > It checks only desc->address[0]. However xencomm_add_offset() may > sets XENCOMM_INVALID while there left consumable bytes. > In such a case xencomm_handle_is_null() returns true wrongly. > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > --- > xen/arch/ia64/xen/xencomm.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff -r 8533c59c5b40 -r 3713ea43e636 xen/arch/ia64/xen/xencomm.c > --- a/xen/arch/ia64/xen/xencomm.c Fri Nov 10 11:14:36 2006 -0700 > +++ b/xen/arch/ia64/xen/xencomm.c Fri Nov 10 11:14:42 2006 -0700 > @@ -382,6 +382,6 @@ xencomm_handle_is_null( > return 1; > > desc = (struct xencomm_desc *)desc_addr; > - return (desc->address[0] == XENCOMM_INVALID); > - } > -} > + return (desc->nr_addrs == 0); > + } > +}I think I''m missing something. Why did IA64 fork xencomm? I distinctly remember having conversations about sharing the code, which is obviously the right thing to do. -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Alex Williamson
2006-Nov-16 18:08 UTC
Re: [Xen-devel] Re: [Xen-changelog] [xen-unstable] [IA64] fix xencomm_handle_is_null().
On Thu, 2006-11-16 at 09:55 -0600, Hollis Blanchard wrote:> > I think I''m missing something. Why did IA64 fork xencomm? > > I distinctly remember having conversations about sharing the code, which > is obviously the right thing to do.Because Tristan believed the resulting amount of shared code would actually be very small (1 file) and we wanted to get his work into the tree before he left Bull. We can still work to share as much as possible even with the code split. Thanks, Alex -- Alex Williamson HP Open Source & Linux Org. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tristan Gingold
2006-Nov-16 20:44 UTC
Re: [Xen-ia64-devel] Re: [Xen-devel] Re: [Xen-changelog] [xen-unstable] [IA64] fix xencomm_handle_is_null().
On Thu, Nov 16, 2006 at 11:08:19AM -0700, Alex Williamson wrote:> On Thu, 2006-11-16 at 09:55 -0600, Hollis Blanchard wrote: > > > > I think I''m missing something. Why did IA64 fork xencomm? > > > > I distinctly remember having conversations about sharing the code, which > > is obviously the right thing to do. > > Because Tristan believed the resulting amount of shared code would > actually be very small (1 file) and we wanted to get his work into the > tree before he left Bull. We can still work to share as much as > possible even with the code split. Thanks,Indeed. Only asm/guest_access.h can be easily shared. Feel free to do it. For the other files, sharing can''t be easily done. We have slightly changed the interface (we now have 3 ways: inline, mini and area). Furthermore the translators are not very stable: bugs have been fixed recently. Tristan. _______________________________________________ Xen-ia64-devel mailing list Xen-ia64-devel@lists.xensource.com http://lists.xensource.com/xen-ia64-devel
On Thu, 2006-11-16 at 21:44 +0100, Tristan Gingold wrote:> On Thu, Nov 16, 2006 at 11:08:19AM -0700, Alex Williamson wrote: > > On Thu, 2006-11-16 at 09:55 -0600, Hollis Blanchard wrote: > > > > > > I think I''m missing something. Why did IA64 fork xencomm? > > > > > > I distinctly remember having conversations about sharing the code, which > > > is obviously the right thing to do. > > > > Because Tristan believed the resulting amount of shared code would > > actually be very small (1 file) and we wanted to get his work into the > > tree before he left Bull. We can still work to share as much as > > possible even with the code split. Thanks, > Indeed. Only asm/guest_access.h can be easily shared. Feel free to do it. > > For the other files, sharing can''t be easily done. We have slightly changed > the interface (we now have 3 ways: inline, mini and area). Furthermore > the translators are not very stable: bugs have been fixed recently.Sharing can''t easily be done *because* you''ve changed the interface, not the other way around. The patch I cited earlier today is a bug fix that applies to PowerPC as well, but the code is not shared, and the patch did not fix both architectures. -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Thu, 2006-11-16 at 11:08 -0700, Alex Williamson wrote:> On Thu, 2006-11-16 at 09:55 -0600, Hollis Blanchard wrote: > > > > I think I''m missing something. Why did IA64 fork xencomm? > > > > I distinctly remember having conversations about sharing the code, which > > is obviously the right thing to do. > > Because Tristan believed the resulting amount of shared code would > actually be very small (1 file) and we wanted to get his work into the > tree before he left Bull. We can still work to share as much as > possible even with the code split. Thanks,This is the first I''ve heard of that conclusion. I was working with Tristan to share the code, and the last time we spoke (in Sep) we were discussing how "mini" descriptors may still be needed (on all xencomm architectures!) because of Linux modules. -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Thu, 2006-11-16 at 15:52 -0600, Hollis Blanchard wrote:> > The patch I cited earlier today is a bug fix that applies to PowerPC > as well, but the code is not shared, and the patch did not fix both > architectures.First let me apologize for getting annoyed. I was wrong on a couple fronts: - Yamahata-san has in fact patched PowerPC''s usercopy.c to fix bugs along with IA64. - This particular patch was not needed on PowerPC at all. Also, I didn''t realize Tristan was leaving Bull, so I can understand the motivation to check something in short-term and sort it out later. This patch was a surprise to me because I hadn''t realized that IA64 had in fact forked xencomm, and I did a fair amount of work in the PPC tree to support IA64 and never got feedback on it. (That must have been about the time Tristan left.) Here is my original mail on the subject. (Strangely I can''t find it in the xen-ia64-devel archives.)> Subject: xencomm porting and "inline" handles > From: Hollis Blanchard <hollisb@us.ibm.com> > To: Tristan Gingold <Tristan.Gingold@bull.net> > Cc: xen-ia64-devel <xen-ia64-devel@lists.xensource.com> > Message-Id: <1158086273.14752.53.camel@basalt.austin.ibm.com> > Date: Tue, 12 Sep 2006 13:37:54 -0500[snip]> The Xen changes are committed to xenppc-unstable.hg . In Xen, > architectures need to provide: > - XENCOMM_INLINE_FLAG in arch-foo.h > - paddr_to_maddr(). I''m open to alternative names, but a standard > function seems useful outside xencomm. (I wish we had one for Linux.) > - asm/guest_access.h that just #includes xen/xencomm.h > > Once you have IA64-specific implementations for the above, I will > submit the whole thing to xen-devel. (Note that Linux doesn''t actually > have to use it right now; it won''t break anything.) > > The Linux are committed to linux-ppc-2.6.hg . I think for Linux > architectures just need to supply: > - XENCOMM_INLINE_FLAG in arch-foo.h > - xencomm_vtop()[snip] This work is still present in http://xenbits.xensource.com/ext/xenppc-unstable.hg , and I would appreciate comments about getting this code shared. -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel