Re: [XenPPC] RFC: xencomm - linux side
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
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
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
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
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