thr3ads.net - Xen devel - [Xen-devel] [PATCH 1/2] [xencomm, linux] linux xencomm consolidation [Aug 2007]

If this information is useful, please help other people find it:
Share via:

Isaku Yamahata

2007-Aug-07 08:48 UTC

[Xen-devel] [PATCH 1/2] [xencomm, linux] linux xencomm consolidation

NOTE:
This patch touches powerpc code.
I tested it with cross compilation, but didn''t run it on a powerpc
machine.
I don''t have any access to it.

# HG changeset patch
# User yamahata@valinux.co.jp
# Date 1186063791 -32400
# Node ID 5aebf2a66a51b953bfc2c5e502d214ce2f35a0a7
# Parent  8d5ae51a09a66ff450b46ebb09ff99475604ed91
various fixes xencomm.c for ia64 xencomm consolidation
  - move xen_guest_handle() macro into include/xen/xencomm.h
    It is also ia64 code.
  - is_kern_addr() is powerpc specific.
    It will be defined in linux/include/asm-ia64/xen/xencomm.h
  - fix error recovery path of xencomm_create()
    xencomm_free() requires pseudo physical address, not virtual address.
  - add one BUG_ON() to xencomm_create_mini() for alignment requirement
  - use xencomm_pa() instead of __pa() in xencomm_map() and
    xencomm_map_no_alloc().
    They should work for statically allocated area. On ia64 it isn''t in
    straight mapping area so that xencomm_pa() is necessary.
  - add gcc bug work around. gcc 4.1.2 on ia64 doesn''t handle
    properly auto variables with align attribute.

PATCHNAME: fix_xencomm_create_in_common_code

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>)

diff -r 8d5ae51a09a6 -r 5aebf2a66a51 arch/powerpc/platforms/xen/setup.h
--- a/arch/powerpc/platforms/xen/setup.h	Wed Aug 01 15:16:46 2007 +0100
+++ b/arch/powerpc/platforms/xen/setup.h	Thu Aug 02 23:09:51 2007 +0900
@@ -40,8 +40,6 @@ static inline u64 jiffies_to_ns(unsigned
 	return j * (1000000000UL / HZ);
 }
 
-#define xen_guest_handle(hnd)  ((hnd).p)
-
 extern struct page *alloc_foreign_page(void);
 extern void free_foreign_page(struct page *page);
 
diff -r 8d5ae51a09a6 -r 5aebf2a66a51 drivers/xen/core/xencomm.c
--- a/drivers/xen/core/xencomm.c	Wed Aug 01 15:16:46 2007 +0100
+++ b/drivers/xen/core/xencomm.c	Thu Aug 02 23:09:51 2007 +0900
@@ -23,6 +23,9 @@
 #include <asm/page.h>
 #include <xen/xencomm.h>
 #include <xen/interface/xen.h>
+#ifdef __ia64__
+#include <asm/xen/xencomm.h>	/* for is_kern_addr() */
+#endif
 
 static int xencomm_init(struct xencomm_desc *desc,
 			void *buffer, unsigned long bytes)
@@ -111,7 +114,7 @@ static int xencomm_create(void *buffer, 
 	rc = xencomm_init(desc, buffer, bytes);
 	if (rc) {
 		printk("%s failure: %d\n", "xencomm_init", rc);
-		xencomm_free(desc);
+		xencomm_free((void *)__pa(desc));
 		return rc;
 	}
 
@@ -146,6 +149,7 @@ static int xencomm_create_mini(void *buf
 {
 	int rc = 0;
 	struct xencomm_desc *desc;
+	BUG_ON(((unsigned long)xc_desc) % sizeof(*xc_desc) != 0);
 
 	desc = (void *)xc_desc; 
 
@@ -170,7 +174,7 @@ void *xencomm_map(void *ptr, unsigned lo
 	if (rc || desc == NULL)
 		return NULL;
 
-	return (void *)__pa(desc);
+	return xencomm_pa(desc);
 }
 
 void *__xencomm_map_no_alloc(void *ptr, unsigned long bytes, 
@@ -188,5 +192,5 @@ void *__xencomm_map_no_alloc(void *ptr, 
 	if (rc)
 		return NULL;
  
-	return (void *)__pa(desc);
+	return xencomm_pa(desc);
 }
diff -r 8d5ae51a09a6 -r 5aebf2a66a51 include/xen/xencomm.h
--- a/include/xen/xencomm.h	Wed Aug 01 15:16:46 2007 +0100
+++ b/include/xen/xencomm.h	Thu Aug 02 23:09:51 2007 +0900
@@ -35,10 +35,29 @@ extern void *__xencomm_map_no_alloc(void
 extern void *__xencomm_map_no_alloc(void *ptr, unsigned long bytes, 
 				struct xencomm_mini *xc_area);
 
-#define xencomm_map_no_alloc(ptr, bytes) \
-	({struct xencomm_mini xc_desc\
-		__attribute__((__aligned__(sizeof(struct xencomm_mini))));\
-		__xencomm_map_no_alloc(ptr, bytes, &xc_desc);})
+#ifdef __ia64__
+/*
+ * workaround:
+ * gcc 4.1.2 on ia64 doesn''t handle properly stack variable with
+ * __attribute__((__align__(sizeof(struct xencomm_mini))))
+ */
+#define XENCOMM_MINI_ALIGNED(xc_desc, n)				\
+	unsigned char xc_desc ## _base[((n) + 1 ) *			\
+				       sizeof(struct xencomm_mini)];	\
+	struct xencomm_mini *xc_desc = (struct xencomm_mini*)		\
+		((unsigned long)xc_desc ## _base +			\
+		 (sizeof(struct xencomm_mini) -				\
+		  ((unsigned long)xc_desc ## _base) %			\
+		  sizeof(struct xencomm_mini)));
+#else
+#define XENCOMM_MINI_ALIGNED(xc_desc, n)				\
+	struct xencomm_mini xc_desc ## _base[(n)]			\
+	__attribute__((__aligned__(sizeof(struct xencomm_mini))));	\
+	struct xencomm_mini *xc_desc = &xc_desc ## _base[0];
+#endif
+#define xencomm_map_no_alloc(ptr, bytes)			\
+	({XENCOMM_MINI_ALIGNED(xc_desc, 1);			\
+		__xencomm_map_no_alloc(ptr, bytes, xc_desc);})
 
 /* provided by architecture code: */
 extern unsigned long xencomm_vtop(unsigned long vaddr);
@@ -48,4 +67,6 @@ static inline void *xencomm_pa(void *ptr
 	return (void *)xencomm_vtop((unsigned long)ptr);
 }
 
+#define xen_guest_handle(hnd)  ((hnd).p)
+
 #endif /* _LINUX_XENCOMM_H_ */


-- 
yamahata

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

Isaku Yamahata

2007-Aug-07 08:57 UTC

head link

[Xen-devel] [PATCH 2/2] [xencomm, linux] linux xencomm consolidation: struct xencomm_handle

IA64 xencomm introduced opaque struct xencomm_handle for xencommized value.
But common code didn''t. This patch itroduce it to common xencomm code.
NOTE: I tested this patch with cross compile for powerpc.

Hollis, Do you like this patch?
I created this patch because Tristan already introduced
struct xencomm_handle in ia64 xencomm code.
However I''m not sure you like it or not and I don''t insist on
it.
If you don''t, I will remove struct xencomm_handle from ia64 code.


# HG changeset patch
# User yamahata@valinux.co.jp
# Date 1186473244 -32400
# Node ID 6a2f4915295018a054678d67b8b482dca9e9fcf6
# Parent  5aebf2a66a51b953bfc2c5e502d214ce2f35a0a7
xencomm: introduce opaque type struct xencomm_handle* for xencommized value
PATCHNAME: introduce_xencomm_handle

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff -r 5aebf2a66a51 -r 6a2f49152950 arch/powerpc/platforms/xen/hcall.c
--- a/arch/powerpc/platforms/xen/hcall.c	Thu Aug 02 23:09:51 2007 +0900
+++ b/arch/powerpc/platforms/xen/hcall.c	Tue Aug 07 16:54:04 2007 +0900
@@ -56,7 +56,7 @@
 
 int HYPERVISOR_console_io(int cmd, int count, char *str)
 {
-	void *desc;
+	struct xencomm_handle *desc;
 	int rc;
 
 	desc = xencomm_map_no_alloc(str, count);
@@ -76,7 +76,8 @@ int HYPERVISOR_event_channel_op(int cmd,
 {
 	int rc;
 
-	void *desc = xencomm_map_no_alloc(op, sizeof(evtchn_op_t));
+	struct xencomm_handle *desc +		xencomm_map_no_alloc(op, sizeof(evtchn_op_t));
 	if (desc == NULL)
 		return -EINVAL;
 
@@ -92,7 +93,7 @@ EXPORT_SYMBOL(HYPERVISOR_event_channel_o
 
 int HYPERVISOR_xen_version(int cmd, void *arg)
 {
-	void *desc;
+	struct xencomm_handle *desc;
 	const unsigned long hcall = __HYPERVISOR_xen_version;
 	int argsize;
 	int rc;
@@ -144,7 +145,8 @@ EXPORT_SYMBOL(HYPERVISOR_xen_version);
 
 int HYPERVISOR_physdev_op(int cmd, void *op)
 {
-	void *desc = xencomm_map_no_alloc(op, sizeof(physdev_op_t));
+	struct xencomm_handle *desc +		xencomm_map_no_alloc(op, sizeof(physdev_op_t));
 	int rc;
 
 	if (desc == NULL)
@@ -163,8 +165,8 @@ int HYPERVISOR_sched_op(int cmd, void *a
 {
 	int argsize = 0;
 	int rc = -EINVAL;
-	void *desc;
-	evtchn_port_t *ports = NULL;
+	struct xencomm_handle *desc;
+	struct xencomm_handle *ports = NULL;
 
 	switch (cmd) {
 	case SCHEDOP_yield:
@@ -187,7 +189,7 @@ int HYPERVISOR_sched_op(int cmd, void *a
 		if (ports == NULL)
 			return -ENOMEM;
 
-		set_xen_guest_handle(sched_poll.ports, ports);
+		set_xen_guest_handle(sched_poll.ports, (evtchn_port_t *)ports);
 		memcpy(arg, &sched_poll, sizeof(sched_poll));
 
 		}
@@ -222,7 +224,7 @@ int HYPERVISOR_suspend(unsigned long sre
 	struct sched_shutdown sched_shutdown = {
 		.reason = SHUTDOWN_suspend,
 	};
-	void *desc;
+	struct xencomm_handle *desc;
 
 	desc = xencomm_map_no_alloc(&sched_shutdown, sizeof(struct
sched_shutdown));
 
@@ -234,7 +236,7 @@ int HYPERVISOR_kexec_op(unsigned long op
 int HYPERVISOR_kexec_op(unsigned long op, void *args)
 {
 	unsigned long argsize;
-	void *desc;
+	struct xencomm_handle *desc;
 
 	switch (op) {
 		case KEXEC_CMD_kexec_get_range:
@@ -316,8 +318,8 @@ static int xenppc_privcmd_domctl(privcmd
 {
 	xen_domctl_t kern_op;
 	xen_domctl_t __user *user_op = (xen_domctl_t __user *)hypercall->arg[0];
-	void *op_desc;
-	void *desc = NULL;
+	struct xencomm_handle *op_desc;
+	struct xencomm_handle *desc = NULL;
 	int ret = 0;
 
 	if (copy_from_user(&kern_op, user_op, sizeof(xen_domctl_t)))
@@ -349,7 +351,7 @@ static int xenppc_privcmd_domctl(privcmd
 			ret = -ENOMEM;
 
 		set_xen_guest_handle(kern_op.u.getmemlist.buffer,
-				     desc);
+				     (void *)desc);
 		break;
 	case XEN_DOMCTL_getpageframeinfo:
 		break;
@@ -362,7 +364,7 @@ static int xenppc_privcmd_domctl(privcmd
 			ret = -ENOMEM;
 
 		set_xen_guest_handle(kern_op.u.getpageframeinfo2.array,
-				     desc);
+				     (void *)desc);
 		break;
 	case XEN_DOMCTL_shadow_op:
 
@@ -376,7 +378,7 @@ static int xenppc_privcmd_domctl(privcmd
 				ret = -ENOMEM;
 
 			set_xen_guest_handle(kern_op.u.shadow_op.dirty_bitmap,
-				    	 desc);
+				    	 (void *)desc);
 		}
 		break;
 	case XEN_DOMCTL_max_mem:
@@ -391,7 +393,7 @@ static int xenppc_privcmd_domctl(privcmd
 			ret = -ENOMEM;
 
 		set_xen_guest_handle(kern_op.u.vcpucontext.ctxt,
-				     desc);
+				     (void *)desc);
 		break;
 	case XEN_DOMCTL_getvcpuinfo:
 		break;
@@ -405,7 +407,7 @@ static int xenppc_privcmd_domctl(privcmd
 			ret = -ENOMEM;
 
 		set_xen_guest_handle(kern_op.u.vcpuaffinity.cpumap.bitmap,
-				     desc);
+				     (void *)desc);
 		break;
 	case XEN_DOMCTL_max_vcpus:
 	case XEN_DOMCTL_scheduler_op:
@@ -442,8 +444,8 @@ static int xenppc_privcmd_sysctl(privcmd
 {
 	xen_sysctl_t kern_op;
 	xen_sysctl_t __user *user_op = (xen_sysctl_t __user *)hypercall->arg[0];
-	struct xencomm_desc *op_desc;
-	void *desc = NULL;
+	struct xencomm_handle *op_desc;
+	struct xencomm_handle *desc = NULL;
 	int ret = 0;
 
 	if (copy_from_user(&kern_op, user_op, sizeof(xen_sysctl_t)))
@@ -470,7 +472,7 @@ static int xenppc_privcmd_sysctl(privcmd
 			ret = -ENOMEM;
 
 		set_xen_guest_handle(kern_op.u.readconsole.buffer,
-				     desc);
+				     (void *)desc);
 		break;
 	case XEN_SYSCTL_tbuf_op:
 	case XEN_SYSCTL_physinfo:
@@ -491,7 +493,7 @@ static int xenppc_privcmd_sysctl(privcmd
 			ret = -ENOMEM;
 
 		set_xen_guest_handle(kern_op.u.getdomaininfolist.buffer,
-				     desc);
+				     (void *)desc);
 		break;
 	default:
 		printk(KERN_ERR "%s: unknown sysctl cmd %d\n", __func__,
kern_op.cmd);
@@ -517,8 +519,8 @@ static int xenppc_privcmd_platform_op(pr
 	xen_platform_op_t kern_op;
 	xen_platform_op_t __user *user_op  			(xen_platform_op_t __user
*)hypercall->arg[0];
-	void *op_desc;
-	void *desc = NULL;
+	struct xencomm_handle *op_desc;
+	struct xencomm_handle *desc = NULL;
 	int ret = 0;
 
 	if (copy_from_user(&kern_op, user_op, sizeof(xen_platform_op_t)))
@@ -566,7 +568,7 @@ int HYPERVISOR_memory_op(unsigned int cm
 int HYPERVISOR_memory_op(unsigned int cmd, void *arg)
 {
 	int ret;
-	void *op_desc;
+	struct xencomm_handle *op_desc;
 	xen_memory_reservation_t *mop;
 
 
@@ -581,7 +583,7 @@ int HYPERVISOR_memory_op(unsigned int cm
 	case XENMEM_increase_reservation:
 	case XENMEM_decrease_reservation:
 	case XENMEM_populate_physmap: {
-		void *desc = NULL;
+		struct xencomm_handle *desc = NULL;
 
 		if (xen_guest_handle(mop->extent_start)) {
 			desc = xencomm_map(
@@ -595,7 +597,7 @@ int HYPERVISOR_memory_op(unsigned int cm
 			}
 
 			set_xen_guest_handle(mop->extent_start,
-					     desc);
+					     (void *)desc);
 		}
 
 		ret = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_memory_op),
@@ -650,7 +652,7 @@ static int xenppc_privcmd_version(privcm
 
 static int xenppc_privcmd_event_channel_op(privcmd_hypercall_t *hypercall)
 {
-	struct xencomm_desc *desc;
+	struct xencomm_handle *desc;
 	unsigned int argsize;
 	int ret;
 
@@ -856,7 +858,7 @@ int HYPERVISOR_vcpu_op(int cmd, int vcpu
 {
 	int argsize;
 	const unsigned long hcall = __HYPERVISOR_vcpu_op;
-	void *desc;
+	struct xencomm_handle *desc;
 	int rc;
 
 	switch (cmd) {
diff -r 5aebf2a66a51 -r 6a2f49152950 drivers/xen/core/xencomm.c
--- a/drivers/xen/core/xencomm.c	Thu Aug 02 23:09:51 2007 +0900
+++ b/drivers/xen/core/xencomm.c	Tue Aug 07 16:54:04 2007 +0900
@@ -83,7 +83,7 @@ static struct xencomm_desc *xencomm_allo
 	return desc;
 }
 
-void xencomm_free(void *desc)
+void xencomm_free(struct xencomm_handle *desc)
 {
 	if (desc && !((ulong)desc & XENCOMM_INLINE_FLAG))
 		free_page((unsigned long)__va(desc));
@@ -114,7 +114,7 @@ static int xencomm_create(void *buffer, 
 	rc = xencomm_init(desc, buffer, bytes);
 	if (rc) {
 		printk("%s failure: %d\n", "xencomm_init", rc);
-		xencomm_free((void *)__pa(desc));
+		xencomm_free((struct xencomm_handle *)__pa(desc));
 		return rc;
 	}
 
@@ -131,7 +131,7 @@ static int is_phys_contiguous(unsigned l
 	return (addr < VMALLOC_START) || (addr >= VMALLOC_END);
 }
 
-static void *xencomm_create_inline(void *ptr)
+static struct xencomm_handle *xencomm_create_inline(void *ptr)
 {
 	unsigned long paddr;
 
@@ -139,7 +139,7 @@ static void *xencomm_create_inline(void 
 
 	paddr = (unsigned long)xencomm_pa(ptr);
 	BUG_ON(paddr & XENCOMM_INLINE_FLAG);
-	return (void *)(paddr | XENCOMM_INLINE_FLAG);
+	return (struct xencomm_handle *)(paddr | XENCOMM_INLINE_FLAG);
 }
 
 /* "mini" routine, for stack-based communications: */
@@ -161,7 +161,7 @@ static int xencomm_create_mini(void *buf
 	return rc;
 }
 
-void *xencomm_map(void *ptr, unsigned long bytes)
+struct xencomm_handle *xencomm_map(void *ptr, unsigned long bytes)
 {
 	int rc;
 	struct xencomm_desc *desc;
@@ -177,7 +177,7 @@ void *xencomm_map(void *ptr, unsigned lo
 	return xencomm_pa(desc);
 }
 
-void *__xencomm_map_no_alloc(void *ptr, unsigned long bytes, 
+struct xencomm_handle *__xencomm_map_no_alloc(void *ptr, unsigned long bytes, 
 			struct xencomm_mini *xc_desc)
 {
 	int rc;
diff -r 5aebf2a66a51 -r 6a2f49152950 include/xen/xencomm.h
--- a/include/xen/xencomm.h	Thu Aug 02 23:09:51 2007 +0900
+++ b/include/xen/xencomm.h	Tue Aug 07 16:54:04 2007 +0900
@@ -30,10 +30,14 @@ struct xencomm_mini {
 	uint64_t address[XENCOMM_MINI_ADDRS];
 };
 
-extern void xencomm_free(void *desc);
-extern void *xencomm_map(void *ptr, unsigned long bytes);
-extern void *__xencomm_map_no_alloc(void *ptr, unsigned long bytes, 
-				struct xencomm_mini *xc_area);
+/* To avoid additionnal virt to phys conversion, an opaque structure is
+   presented.  */
+struct xencomm_handle;
+
+extern void xencomm_free(struct xencomm_handle *desc);
+extern struct xencomm_handle *xencomm_map(void *ptr, unsigned long bytes);
+extern struct xencomm_handle *__xencomm_map_no_alloc(void *ptr,
+			unsigned long bytes,  struct xencomm_mini *xc_area);
 
 #ifdef __ia64__
 /*


-- 
yamahata


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

Isaku Yamahata

2007-Aug-10 06:59 UTC

head link

Re: [Xen-devel] [PATCH 1/2] [xencomm, linux] linux xencomm consolidation

On Tue, Aug 07, 2007 at 05:48:54PM +0900, Isaku Yamahata
wrote:>   - add gcc bug work around. gcc 4.1.2 on ia64 doesn''t handle
>     properly auto variables with align attribute.
I thought this gcc issue was ia64 specific at first.
But it seems to be generic (non-arch specific) issue.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16660
(The patch exists, but it doesn''t seem to be included in the main line
yet.)

Does the current xencomm_map_no_alloc() work on powerpc properly?
At least my powerpc64-linux-gcc (gcc 4.1.1) doesn''t seem to support
__attribute__((aligned(32))) on stack variables.

thanks,
-- 
yamahata

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

Hollis Blanchard

2007-Aug-10 15:59 UTC

head link

Re: [XenPPC] Re: [Xen-devel] [PATCH 1/2] [xencomm, linux] linux xencomm consolidation

On Fri, 2007-08-10 at 15:59 +0900, Isaku Yamahata wrote:> On Tue, Aug 07, 2007 at 05:48:54PM +0900, Isaku Yamahata wrote:
> >   - add gcc bug work around. gcc 4.1.2 on ia64 doesn''t handle
> >     properly auto variables with align attribute.
> 
> I thought this gcc issue was ia64 specific at first.
> But it seems to be generic (non-arch specific) issue.
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16660
> (The patch exists, but it doesn''t seem to be included in the main
line yet.)
> 
> Does the current xencomm_map_no_alloc() work on powerpc properly?
> At least my powerpc64-linux-gcc (gcc 4.1.1) doesn''t seem to
support
> __attribute__((aligned(32))) on stack variables.
Hmm, the bug includes a comment that says "If the code is not very
complex, the alignment appears to work," so I guess we''ve just
been
getting lucky...

-- 
Hollis Blanchard
IBM Linux Technology Center


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

Xen devel - Aug 2007 - [PATCH 1/2] [xencomm, linux] linux xencomm consolidation

[Xen-devel] [PATCH 1/2] [xencomm, linux] linux xencomm consolidation

[Xen-devel] [PATCH 2/2] [xencomm, linux] linux xencomm consolidation: struct xencomm_handle

Re: [Xen-devel] [PATCH 1/2] [xencomm, linux] linux xencomm consolidation

Re: [XenPPC] Re: [Xen-devel] [PATCH 1/2] [xencomm, linux] linux xencomm consolidation