[XenPPC] [PATCH] [RFC] Fix xenminicom optimizations to work for module
First patch to attempt to re introduce older code written by Hollis. If a structure that does not fit within 1 page is pointed to, we need to create a structure on the stack to represent this structure for the hypervisor. This code affects kernel module addresses which are do not have physically contiguous memory. This patch isn't perfect yet. But comments would be nice. diff -r bbf2db4ddf54 arch/powerpc/platforms/xen/gnttab.c --- a/arch/powerpc/platforms/xen/gnttab.c Tue Dec 19 09:22:37 2006 -0500 +++ b/arch/powerpc/platforms/xen/gnttab.c Wed Jan 10 00:50:24 2007 -0600 @@ -264,7 +264,7 @@ int HYPERVISOR_grant_table_op(unsigned i argsize = sizeof(setup); frame_list = xencomm_create_inline( - xen_guest_handle(setup.frame_list)); + xen_guest_handle(setup.frame_list), 0); set_xen_guest_handle(setup.frame_list, frame_list); memcpy(op, setup, sizeof(setup)); @@ -286,7 +286,7 @@ int HYPERVISOR_grant_table_op(unsigned i return -ENOSYS; } - desc = xencomm_create_inline(op); + desc = xencomm_create_inline(op, 0); ret = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_grant_table_op), cmd, desc, count); diff -r bbf2db4ddf54 arch/powerpc/platforms/xen/hcall.c --- a/arch/powerpc/platforms/xen/hcall.c Tue Dec 19 09:22:37 2006 -0500 +++ b/arch/powerpc/platforms/xen/hcall.c Wed Jan 10 10:30:08 2007 -0600 @@ -50,11 +50,31 @@ * In general, we need a xencomm descriptor to cover the top-level data * structure (e.g. the domctl op), plus another for every embedded pointer to * another data structure (i.e. for every GUEST_HANDLE). + * + * Some hypercalls are made before the memory subsystem is up, so instead of + * calling xencomm_create(), we allocate XENCOMM_MINI_AREA bytes from the stack + * to hold the xencomm descriptor. */ int HYPERVISOR_console_io(int cmd, int count, char *str) { - void *desc = xencomm_create_inline(str); + char xc_area[XENCOMM_MINI_AREA]; + struct xencomm_desc *x_desc; + int rc; + + void *desc = xencomm_create_inline(str, count); + + if (desc == NULL) { + rc = xencomm_create_mini(xc_area, XENCOMM_MINI_AREA, str, count, + x_desc); + if (rc) + return rc; + + rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_console_io), + cmd, count, __pa(x_desc)); + + return rc; + } return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_console_io), cmd, count, desc); @@ -63,7 +83,22 @@ EXPORT_SYMBOL(HYPERVISOR_console_io); int HYPERVISOR_event_channel_op(int cmd, void *op) { - void *desc = xencomm_create_inline(op); + char xc_area[XENCOMM_MINI_AREA]; + struct xencomm_desc *x_desc; + int rc; + + void *desc = xencomm_create_inline(op, sizeof(evtchn_op_t)); + + if (desc == NULL) { + rc = xencomm_create_mini(xc_area, XENCOMM_MINI_AREA, + op, sizeof(evtchn_op_t), x_desc); + if (rc) + return rc; + + rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_event_channel_op), + cmd, __pa(x_desc)); + return rc; + } return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_event_channel_op), cmd, desc); @@ -121,7 +156,7 @@ int HYPERVISOR_xen_version(int cmd, void int HYPERVISOR_xen_version(int cmd, void *arg) { if (is_kernel_addr((unsigned long)arg)) { - void *desc = xencomm_create_inline(arg); + void *desc = xencomm_create_inline(arg, 0); return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_xen_version), cmd, desc); } return HYPERVISOR_xen_version_userspace(cmd, arg); @@ -129,7 +164,7 @@ int HYPERVISOR_xen_version(int cmd, void int HYPERVISOR_physdev_op(int cmd, void *op) { - void *desc = xencomm_create_inline(op); + void *desc = xencomm_create_inline(op, sizeof(physdev_op_t)); return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_physdev_op), cmd, desc); @@ -154,7 +189,7 @@ int HYPERVISOR_sched_op(int cmd, void *a memcpy(sched_poll, arg, sizeof(sched_poll)); ports = xencomm_create_inline( - xen_guest_handle(sched_poll.ports)); + xen_guest_handle(sched_poll.ports), 0); set_xen_guest_handle(sched_poll.ports, ports); memcpy(arg, sched_poll, sizeof(sched_poll)); @@ -168,7 +203,7 @@ int HYPERVISOR_sched_op(int cmd, void *a return -ENOSYS; } - desc = xencomm_create_inline(arg); + desc = xencomm_create_inline(arg, 0); return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op), cmd, desc); diff -r bbf2db4ddf54 drivers/xen/core/xencomm.c --- a/drivers/xen/core/xencomm.c Tue Dec 19 09:22:37 2006 -0500 +++ b/drivers/xen/core/xencomm.c Wed Jan 10 01:15:50 2007 -0600 @@ -119,13 +119,59 @@ int xencomm_create(void *buffer, unsigne return 0; } -void *xencomm_create_inline(void *ptr) +void *xencomm_create_inline(void *ptr, unsigned long bytes) { unsigned long paddr; - - BUG_ON(!is_kernel_addr((unsigned long)ptr)); - + unsigned long first; + unsigned long last; + + first = xencomm_vtop(ptr) PAGE_MASK; + last = xencomm_vtop(ptr+bytes) PAGE_MASK; + + if (first != last) + return NULL; + paddr = (unsigned long)xencomm_pa(ptr); BUG_ON(paddr XENCOMM_INLINE_FLAG); return (void *)(paddr | XENCOMM_INLINE_FLAG); } +
Re: [XenPPC] [PATCH] [RFC] Fix xenminicom optimizations to work for module
On Wed, 2007-01-10 at 11:51 -0600, Jerone Young wrote: diff -r bbf2db4ddf54 arch/powerpc/platforms/xen/gnttab.c --- a/arch/powerpc/platforms/xen/gnttab.c Tue Dec 19 09:22:37 2006 -0500 +++ b/arch/powerpc/platforms/xen/gnttab.c Wed Jan 10 00:50:24 2007 -0600 @@ -264,7 +264,7 @@ int HYPERVISOR_grant_table_op(unsigned i argsize = sizeof(setup); frame_list = xencomm_create_inline( - xen_guest_handle(setup.frame_list)); + xen_guest_handle(setup.frame_list), 0); set_xen_guest_handle(setup.frame_list, frame_list); memcpy(op, setup, sizeof(setup)); @@ -286,7 +286,7 @@ int HYPERVISOR_grant_table_op(unsigned i return -ENOSYS; } - desc = xencomm_create_inline(op); + desc = xencomm_create_inline(op, 0); ret = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_grant_table_op), cmd, desc, count); Throughout your entire patch you're using a size of 0. That can't be right. diff -r bbf2db4ddf54 arch/powerpc/platforms/xen/hcall.c --- a/arch/powerpc/platforms/xen/hcall.cTue Dec 19 09:22:37 2006 -0500 +++ b/arch/powerpc/platforms/xen/hcall.cWed Jan 10 10:30:08 2007 -0600 @@ -63,7 +83,22 @@ EXPORT_SYMBOL(HYPERVISOR_console_io); int HYPERVISOR_event_channel_op(int cmd, void *op) { - void *desc = xencomm_create_inline(op); + char xc_area[XENCOMM_MINI_AREA]; + struct xencomm_desc *x_desc; + int rc; + + void *desc = xencomm_create_inline(op, sizeof(evtchn_op_t)); + + if (desc == NULL) { + rc = xencomm_create_mini(xc_area, XENCOMM_MINI_AREA, + op, sizeof(evtchn_op_t), x_desc); + if (rc) + return rc; + + rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_event_channel_op), + cmd, __pa(x_desc)); + return rc; + } return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_event_channel_op), cmd, desc); You don't need both desc and x_desc. Just remove x_desc. diff -r bbf2db4ddf54 drivers/xen/core/xencomm.c --- a/drivers/xen/core/xencomm.cTue Dec 19 09:22:37 2006 -0500 +++ b/drivers/xen/core/xencomm.cWed Jan 10 01:15:50 2007 -0600 @@ -119,13 +119,59 @@ int xencomm_create(void *buffer, unsigne return 0; } -void *xencomm_create_inline(void *ptr) +void *xencomm_create_inline(void *ptr, unsigned long bytes) { unsigned long paddr; - - BUG_ON(!is_kernel_addr((unsigned long)ptr)); - + unsigned long first; + unsigned long last; + + first = xencomm_vtop(ptr) PAGE_MASK; + last = xencomm_vtop(ptr+bytes) PAGE_MASK; Rename first and last to something like first_phys_page and last_phys_page. Does this code actually work? It seemed like the problem with your other patch was that xencomm_vtop() doesn't work early. A simpler but overly-broad test could work here: first_page = ptr PAGE_MASK; last_page = (ptr + bytes) PAGE_MASK; + if (first != last) + return NULL; paddr = (unsigned long)xencomm_pa(ptr); BUG_ON(paddr XENCOMM_INLINE_FLAG); return (void *)(paddr | XENCOMM_INLINE_FLAG); } How has this patch been tested? -- 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] [PATCH] [RFC] Fix xenminicom optimizations to work for module
On Wed, 2007-01-10 at 12:45 -0600, Hollis Blanchard wrote: On Wed, 2007-01-10 at 11:51 -0600, Jerone Young wrote: diff -r bbf2db4ddf54 arch/powerpc/platforms/xen/gnttab.c --- a/arch/powerpc/platforms/xen/gnttab.c Tue Dec 19 09:22:37 2006 -0500 +++ b/arch/powerpc/platforms/xen/gnttab.c Wed Jan 10 00:50:24 2007 -0600 @@ -264,7 +264,7 @@ int HYPERVISOR_grant_table_op(unsigned i argsize = sizeof(setup); frame_list = xencomm_create_inline( - xen_guest_handle(setup.frame_list)); + xen_guest_handle(setup.frame_list), 0); set_xen_guest_handle(setup.frame_list, frame_list); memcpy(op, setup, sizeof(setup)); @@ -286,7 +286,7 @@ int HYPERVISOR_grant_table_op(unsigned i return -ENOSYS; } - desc = xencomm_create_inline(op); + desc = xencomm_create_inline(op, 0); ret = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_grant_table_op), cmd, desc, count); Throughout your entire patch you're using a size of 0. That can't be right. Glad you pointed this out. Actually, in these cases I use 0 (why the patch isn't perfect) to ensure that we are not returned a NULL pointer. Since this is code that has just been added. Since the check is not needed in theses cases, but perhaps it will always pass and this is not going to be a worry. diff -r bbf2db4ddf54 arch/powerpc/platforms/xen/hcall.c --- a/arch/powerpc/platforms/xen/hcall.cTue Dec 19 09:22:37 2006 -0500 +++ b/arch/powerpc/platforms/xen/hcall.cWed Jan 10 10:30:08 2007 -0600 @@ -63,7 +83,22 @@ EXPORT_SYMBOL(HYPERVISOR_console_io); int HYPERVISOR_event_channel_op(int cmd, void *op) { - void *desc = xencomm_create_inline(op); + char xc_area[XENCOMM_MINI_AREA]; + struct xencomm_desc *x_desc; + int rc; + + void *desc = xencomm_create_inline(op, sizeof(evtchn_op_t)); + + if (desc == NULL) { + rc = xencomm_create_mini(xc_area, XENCOMM_MINI_AREA, + op, sizeof(evtchn_op_t), x_desc); + if (rc) + return rc; + + rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_event_channel_op), + cmd, __pa(x_desc)); + return rc; + } return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_event_channel_op), cmd, desc); You don't need both desc and x_desc. Just remove x_desc. Sounds good. diff -r bbf2db4ddf54 drivers/xen/core/xencomm.c --- a/drivers/xen/core/xencomm.cTue Dec 19 09:22:37 2006 -0500 +++ b/drivers/xen/core/xencomm.cWed Jan 10 01:15:50 2007 -0600 @@ -119,13 +119,59 @@ int xencomm_create(void *buffer, unsigne return 0; } -void *xencomm_create_inline(void *ptr) +void *xencomm_create_inline(void *ptr, unsigned long bytes) { unsigned long paddr; - - BUG_ON(!is_kernel_addr((unsigned long)ptr)); - + unsigned long first; + unsigned long last; + + first = xencomm_vtop(ptr) PAGE_MASK; + last = xencomm_vtop(ptr+bytes) PAGE_MASK; Rename first and last to something like first_phys_page and last_phys_page. Does this code actually work? It seemed like the problem with your other patch was that xencomm_vtop() doesn't work early. A simpler but overly-broad test could work here: first_page = ptr PAGE_MASK; last_page = (ptr + bytes) PAGE_MASK; + if (first != last) + return NULL; paddr = (unsigned long)xencomm_pa(ptr); BUG_ON(paddr XENCOMM_INLINE_FLAG); return (void *)(paddr | XENCOMM_INLINE_FLAG); } How has this patch been tested? Yes this one has been tested. It boots. Not thoroughly as of yet. So I just wanted comments of what I had for now. Thanks for the comments :-) -- Hollis Blanchard IBM Linux Technology Center ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel