[XenPPC] [PATCH] [RFC] Fix xenminicom optimizations to work for module

2007-01-10 Thread Jerone Young

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

2007-01-10 Thread Hollis Blanchard
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

2007-01-10 Thread Jerone Young
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