Re: [Xen-changelog] [xen-unstable] [IA64] fix xencomm_handle_is_null().

Re: [Xen-changelog] [xen-unstable] [IA64] fix xencomm_handle_is_null().

[Hollis Blanchard]

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

Re: Re: [Xen-changelog] [xen-unstable] [IA64] fix xencomm_handle_is_null().

[Alex Williamson]

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.

Re: Re: [Xen-devel] Re: [Xen-changelog] [xen-unstable] [IA64] fix xencomm_handle_is_null().

[Tristan Gingold]

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.

Re: consolidated xencomm

[Hollis Blanchard]

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

Re: [XenPPC] Re: consolidated xencomm

[Hollis Blanchard]

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

Re: consolidated xencomm

[Hollis Blanchard]

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