Re: [XenPPC] RFC: xencomm - linux side

2006-09-08 Thread Hollis Blanchard
On Fri, 2006-08-25 at 09:02 +0200, Tristan Gingold wrote:
 Le Jeudi 24 Août 2006 17:43, Hollis Blanchard a écrit :
  On Thu, 2006-08-24 at 09:51 +0200, Tristan Gingold wrote:
My suggestion: have xencomm_create() test IS_KERNEL_ADDR() (in whatever
way is best for portability) and if it is, do the inline stuff. On
the free side, if the descriptor was inline, free can just return. That
would also make me happy because it removes the need to think about
whether callers can/should call create_inline or not; the code just
does the right thing.
  
   We definitly disagree here.  One whole point of xencomm_create_inline is
   it doesn't allocate memory and can't fail.  Because of that we don't need
   to worry about failure and freeing memory.  This makes the code a lot
   easier to write and to read.
 
  It would simplify the code even more to fold xencomm_create_inline()
  into xencomm_create(), as I suggest above. That way, the developer never
  needs to consider if the particular hypercall could ever be called
  before the page allocator works. Proving that assumption for some
  hypercall, and guaranteeing it will remain true in the future no matter
  what Linux changes occur, is a lot more difficult than remembering to
  call free() after create().
 Could you modify the ppc code, I will be happy to fetch directly the code for 
 this new idea.

I'm looking at this now, and I'm brought back to the fact that I don't
like the inline idea because practically speaking it requires that the
kernel stack is physically contiguous. That is true for Linux, but is
that really true for all OSs? Since we're defining a Xen interface, I
don't want to hardcode Linux assumptions.

Without that, an OS with a physically discontiguous stack would be
forced to do the equivalen of get_free_page() to do all communication
(including console output). Before the page allocator works that
wouldn't be possible, but I think we can assume a physically contiguous
stack early in the boot process before the page allocator works. So then
you're requiring a test:

hcall_console_write(char *str) {
if (page_allocator_done()) {
desc = (desc *)get_free_page();
xencomm_map(desc, str);
hcall(desc);
free_page(desc);
} else {
desc = xencomm_create_inline(str);
hcall(desc);
}
}

That seems lame. The mini xencomm stuff always works.

Actually... I guess the mini stuff will always work, and any OS that
needs it can use it. The inline stuff is an optimization that Linux
can take advantage of.

Summary: I've changed my mind, and I only send this email to illustrate
my thought process. :)

-- 
Hollis Blanchard
IBM Linux Technology Center


___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


Re: [XenPPC] RFC: xencomm - linux side

2006-08-22 Thread Tristan Gingold
Le Lundi 21 Août 2006 21:07, Hollis Blanchard a écrit :
 On Mon, 2006-08-21 at 17:18 +0200, Tristan Gingold wrote:
  I am posting the linux xencomm code for review.  I'd plan to submit soon
  unless comments/remarks.

 NAK. I'm still waiting to hear back about how you can use
 xencomm_inline() without worrying about page boundaries.
More elaborated answer:

On linux/ia64, kernel is linearly mapped into guest physical memory.  The same 
is true for process kernel stacks.  Therefore all kernels structure are 
linear in guest physical memory.

Kernel data may of course cross page boundaries.  But Xen can correctly handle 
this using only the guest physical address.

Is something wrong ?  Is something that don't apply on powerpc ?

Tristan.

___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


Re: [XenPPC] RFC: xencomm - linux side

2006-08-22 Thread Hollis Blanchard
On Tue, 2006-08-22 at 09:42 +0200, Tristan Gingold wrote:
 Le Lundi 21 Août 2006 21:07, Hollis Blanchard a écrit :
  On Mon, 2006-08-21 at 17:18 +0200, Tristan Gingold wrote:
   I am posting the linux xencomm code for review.  I'd plan to submit soon
   unless comments/remarks.
 
  NAK. I'm still waiting to hear back about how you can use
  xencomm_inline() without worrying about page boundaries.
 More elaborated answer:
 
 On linux/ia64, kernel is linearly mapped into guest physical memory.  The 
 same 
 is true for process kernel stacks.  Therefore all kernels structure are 
 linear in guest physical memory.
 
 Kernel data may of course cross page boundaries.  But Xen can correctly 
 handle 
 this using only the guest physical address.

I see, and you handle this by breaking up copies at page granularity in
xencomm_copy_from_user().

I have to say I don't really like the code complexity, or the fact that
there are now two very different ways to access guest handles. That
being said, it sure would be nice to get rid of that mini stack-based
stuff, so it's OK with me. I would be surprised if there were any
performance difference, however.

I'll send some comments to your original patch.

-- 
Hollis Blanchard
IBM Linux Technology Center


___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


Re: [XenPPC] RFC: xencomm - linux side

2006-08-22 Thread Hollis Blanchard
I apologize for my mailer line-wrapping the patch as I quote it below.

On Mon, 2006-08-21 at 17:18 +0200, Tristan Gingold wrote:
 diff -r b7db009d622c linux-2.6-xen-sparse/drivers/xen/Kconfig
 --- a/linux-2.6-xen-sparse/drivers/xen/Kconfig  Mon Aug 21 09:41:24
 2006 +0200
 +++ b/linux-2.6-xen-sparse/drivers/xen/Kconfig  Mon Aug 21 15:04:32
 2006 +0200
 @@ -257,4 +257,7 @@ config XEN_SMPBOOT
 default y
 depends on SMP
  
 +config XEN_XENCOMM
 +   bool
 +   default n
  endif

Shouldn't IA64 select XEN_XENCOMM? Or is your kernel in a separate
tree?

 diff -r b7db009d622c linux-2.6-xen-sparse/drivers/xen/Makefile
 --- a/linux-2.6-xen-sparse/drivers/xen/Makefile Mon Aug 21 09:41:24
 2006 +0200
 +++ b/linux-2.6-xen-sparse/drivers/xen/Makefile Mon Aug 21 15:04:32
 2006 +0200
 @@ -1,10 +1,10 @@ obj-y += core/
  obj-y  += core/
  obj-y  += console/
  obj-y  += evtchn/
 -obj-y  += privcmd/
  obj-y  += xenbus/
  
  obj-$(CONFIG_XEN_UTIL) += util.o
 +obj-$(CONFIG_XEN_PRIVCMD)  += privcmd/
  obj-$(CONFIG_XEN_BALLOON)  += balloon/
  obj-$(CONFIG_XEN_DEVMEM)   += char/
  obj-$(CONFIG_XEN_BLKDEV_BACKEND)   += blkback/

Not really part of this patch.

 diff -r b7db009d622c linux-2.6-xen-sparse/drivers/xen/core/Makefile
 --- a/linux-2.6-xen-sparse/drivers/xen/core/MakefileMon Aug 21
 09:41:24 2006 +0200
 +++ b/linux-2.6-xen-sparse/drivers/xen/core/MakefileMon Aug 21
 15:04:32 2006 +0200
 @@ -11,3 +11,4 @@ obj-$(CONFIG_XEN_SKBUFF)  += skbuff.o
  obj-$(CONFIG_XEN_SKBUFF)   += skbuff.o
  obj-$(CONFIG_XEN_REBOOT)   += reboot.o
  obj-$(CONFIG_XEN_SMPBOOT)  += smpboot.o
 +obj-$(CONFIG_XEN_XENCOMM)  += xencomm.o xencomm_hcall.o
 diff -r b7db009d622c linux-2.6-xen-sparse/drivers/xen/privcmd/Makefile
 --- a/linux-2.6-xen-sparse/drivers/xen/privcmd/Makefile Mon Aug 21
 09:41:24 2006 +0200
 +++ b/linux-2.6-xen-sparse/drivers/xen/privcmd/Makefile Mon Aug 21
 15:04:32 2006 +0200
 @@ -1,2 +1,3 @@
 +obj-y  := privcmd.o
  
 -obj-$(CONFIG_XEN_PRIVCMD)  := privcmd.o
 +obj-$(CONFIG_XEN_XENCOMM)  += xencomm.o

I agree with the CONFIG_XEN_PRIVCMD stuff, but I think that should be a
separate patch.

 diff -r b7db009d622c
 linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c
 --- a/linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.cMon
 Aug 21 09:41:24 2006 +0200
 +++ b/linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.cMon
 Aug 21 15:04:32 2006 +0200
 @@ -34,6 +34,10 @@
  
  static struct proc_dir_entry *privcmd_intf;
  static struct proc_dir_entry *capabilities_intf;
 +
 +#ifdef CONFIG_XEN_XENCOMM
 +extern int xencomm_privcmd_hypercall(privcmd_hypercall_t *hypercall);
 +#endif
  
  #define NR_HYPERCALLS 64
  static DECLARE_BITMAP(hypercall_permission_map, NR_HYPERCALLS);
 @@ -91,19 +95,8 @@ static int privcmd_ioctl(struct inode *i
 g ((unsigned long)hypercall.arg[4])
 : r8, r10, memory );
 }
 -#elif defined (__ia64__)
 -   __asm__ __volatile__ (
 -   ;; mov r14=%2; mov r15=%3; 
 -   mov r16=%4; mov r17=%5; mov r18=%6;
 -   mov r2=%1; break 0x1000;; mov %0=r8 ;;
 -   : =r (ret)
 -   : r (hypercall.op),
 -   r (hypercall.arg[0]),
 -   r (hypercall.arg[1]),
 -   r (hypercall.arg[2]),
 -   r (hypercall.arg[3]),
 -   r (hypercall.arg[4])
 -   :
 r14,r15,r16,r17,r18,r2,r8,memory);
 +#elif defined (CONFIG_XEN_XENCOMM)
 +   ret = xencomm_privcmd_hypercall (hypercall);
  #endif
 }
 break;

Move all the #ifdef stuff into appropriate header files, then have every
arch unconditionally call arch_privcmd_hypercall().

 diff -r b7db009d622c linux-2.6-xen-sparse/drivers/xen/core/xencomm.c
 --- /dev/null   Thu Jan 01 00:00:00 1970 +
 +++ b/linux-2.6-xen-sparse/drivers/xen/core/xencomm.c   Mon Aug 21
 15:04:32 2006 +0200
 @@ -0,0 +1,213 @@
 +/*
 + * Copyright (C) 2006 Hollis Blanchard [EMAIL PROTECTED], IBM
 Corporation
 + *
 + * This program is free software; you can redistribute it and/or
 modify
 + * it under the terms of the GNU General Public License as published
 by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + * 
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + * 
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
 02111-1307 USA
 + */
 +
 +#include 

Re: [XenPPC] RFC: xencomm - linux side

2006-08-21 Thread Hollis Blanchard
On Mon, 2006-08-21 at 17:18 +0200, Tristan Gingold wrote:
 I am posting the linux xencomm code for review.  I'd plan to submit soon 
 unless comments/remarks.

NAK. I'm still waiting to hear back about how you can use
xencomm_inline() without worrying about page boundaries.

-- 
Hollis Blanchard
IBM Linux Technology Center


___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel