On Jan 25, 2007, at 8:06 PM, Jerone Young wrote:
This patch should address all the issues Jimi has pointed out.
yes it does... thank you.
but still has a few issues..
Throughout the patch:
1. xencomm_create_inline() could never fail, xencomm_map() can so you
need to check ALL of them.
1. _inline() never allocates _map() can so you always have to call
xencomm_free()
1. Please return a -ERRNO and not -1 all the time.
1. you made the descriptor void * but not everywhere.
other specific issues:
@@ -246,9 +244,9 @@ static int xenppc_privcmd_domctl(privcmd
return -EACCES;
}
- ret = xencomm_create(&kern_op, sizeof(xen_domctl_t), &op_desc,
GFP_KERNEL);
- if (ret)
- return ret;
+ op_desc = xencomm_map(&kern_op, sizeof(xen_domctl_t));
+ if (op_desc)
+ return -1;
if (op_desc) cannot be right.
Can't see how domain creation could have possibly worked, did you
test that?
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 26 02:59:31 2028 -0600
@@ -82,11 +82,11 @@ static struct xencomm_desc *xencomm_allo
void xencomm_free(struct xencomm_desc *desc)
_map() returns a "void *" so _free() should take one.
{
- if (desc)
+ if (desc && !((ulong)desc & XENCOMM_INLINE_FLAG))
free_page((unsigned long)desc);
}
-int xencomm_create(void *buffer, unsigned long bytes, struct
xencomm_desc **ret, gfp_t gfp_mask)
+static int xencomm_create(void *buffer, unsigned long bytes,
struct xencomm_desc **ret, gfp_t gfp_mask)
{
struct xencomm_desc *desc;
int rc;
@@ -119,13 +119,68 @@ int xencomm_create(void *buffer, unsigne
return 0;
}
-void *xencomm_create_inline(void *ptr)
+static void *xencomm_create_inline(void *ptr)
{
unsigned long paddr;
- BUG_ON(!is_kernel_addr((unsigned long)ptr));
+ BUG_ON(!is_phys_contiguous((unsigned long)ptr));
paddr = (unsigned long)xencomm_pa(ptr);
BUG_ON(paddr & XENCOMM_INLINE_FLAG);
return (void *)(paddr | XENCOMM_INLINE_FLAG);
}
+
+/* "mini" routine, for stack-based communications: */
+static int xencomm_create_mini(int arealen, void *buffer,
+ unsigned long bytes, struct xencomm_desc **ret)
+{
+ struct xencomm_desc desc;
You are returning a stack/auto variable here, thats a No No!
+ int rc = 0;
+
+ desc.nr_addrs = XENCOMM_MINI_ADDRS;
+
+ if (! (rc = xencomm_init(&desc, buffer, bytes)));
+ *ret = &desc;
+
+ return rc;
+}
+
+void *xencomm_map(void *ptr, unsigned long bytes)
+{
+ int rc;
+ struct xencomm_desc *desc;
+
+ if (is_phys_contiguous((unsigned long)ptr))
+ return xencomm_create_inline(ptr);
+
+ rc = xencomm_create(ptr, bytes, &desc, GFP_KERNEL);
+
+ if (rc)
+ return NULL;
+
+ return xencomm_pa(desc);
+}
+
+void *__xencomm_map_early(void *ptr, unsigned long bytes,
+ struct xencomm_mini *xc_area)
+{
+ int rc;
+ struct xencomm_desc *desc = NULL;
+
+ if (is_phys_contiguous((unsigned long)ptr))
+ return xencomm_create_inline(ptr);
+
+ rc = xencomm_create_mini(XENCOMM_MINI_AREA,ptr, bytes,
whitespace
+ &desc);
+
+ if (rc)
+ return NULL;
+
+ return (void*)__pa(desc);
+}
+
+/* check if is physically contiguous memory */
+int is_phys_contiguous(unsigned long addr)
Can we please make this static now!
+{
+ return (addr < VMALLOC_START) || (addr >= VMALLOC_END);
+}
diff -r bbf2db4ddf54 include/xen/xencomm.h
--- a/include/xen/xencomm.h Tue Dec 19 09:22:37 2006 -0500
+++ b/include/xen/xencomm.h Wed Jan 26 02:17:31 2028 -0600
@@ -16,6 +16,7 @@
* Copyright (C) IBM Corp. 2006
*
* Authors: Hollis Blanchard <hollisb@xxxxxxxxxx>
+ * Jerone Young <jyoung5@xxxxxxxxxx>
*/
#ifndef _LINUX_XENCOMM_H_
@@ -23,10 +24,23 @@
#include <xen/interface/xencomm.h>
-extern int xencomm_create(void *buffer, unsigned long bytes,
- struct xencomm_desc **desc, gfp_t type);
+#define XENCOMM_MINI_ADDRS 3
+struct xencomm_mini {
+ struct xencomm_desc _desc;
+ uint64_t address[XENCOMM_MINI_ADDRS];
+};
+#define XENCOMM_MINI_AREA (sizeof(struct xencomm_mini) * 2)
This is no longer used, right?
+
extern void xencomm_free(struct xencomm_desc *desc);
-extern void *xencomm_create_inline(void *ptr);
+extern void *xencomm_map(void *ptr, unsigned long bytes);
+extern void *__xencomm_map_early(void *ptr, unsigned long bytes,
+ struct xencomm_mini *xc_area);
+extern int is_phys_contiguous(unsigned long addr);
begone
+
+#define xencomm_map_early(ptr, bytes) \
+ ({struct xencomm_mini xc_area\
+ __attribute__((__aligned__(sizeof(struct xencomm_mini))));\
+ __xencomm_map_early(ptr, bytes, &xc_area);})
/* provided by architecture code: */
extern unsigned long xencomm_vtop(unsigned long vaddr);
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
| 
Home