Re: [XenPPC] [PATCH] [UPDATE] Xencomm patch

[Jimi Xenidis <jimix@xxxxxxxxxxxxxx>]

On Jan 24, 2007, at 1:08 AM, Jerone Young wrote:

With all the recommendations here is another udpate to the Xencomm

patch. 

Actually, you missed some.. :)

I was unable though to successfully remove xencomm_create & replace it

with xencomm_map. It was causing issues when loading of Dom0 that would

cause the kernel to blowup.

"blowup"? as in "hang", or "trap into xmon" if the later, do you have a traceback?

Testing:

I have tested it loading xennet driver as a module, though I could not

get Xen kernel to load the block device driver (blkfront) as a module in

an initrd. Not sure why? No crash, just wouldn't load it. Otherwise

kernel loads fine as dom0 & domU.

We need to debug this.

Signed-off-by: Jerone Young <jyoung5@xxxxxxxxxx>

# HG changeset patch

# User Jerone Young <jyoung5@xxxxxxxxxx>

# Date 1169498835 21600

# Node ID 82e1886955c349ee9c473560ead37f61b787fcd7

# Parent  ab3b5849331da89e578ae0813021376d66b7f333

Initial Xencomm patch. Still have to remove xencomm_create usage.

diff -r ab3b5849331d -r 82e1886955c3 arch/powerpc/platforms/xen/gnttab.c

--- a/arch/powerpc/platforms/xen/gnttab.c Sun Jan 21 08:36:53 2007 -0500

+++ b/arch/powerpc/platforms/xen/gnttab.c Mon Jan 22 14:47:15 2007 -0600

@@ -263,8 +263,9 @@ int HYPERVISOR_grant_table_op(unsigned i

I'm pretty sure this can happen in interrupt so you'll need to use _early.

memcpy(&setup, op, sizeof(setup));

argsize = sizeof(setup);

- frame_list = xencomm_create_inline(

- xen_guest_handle(setup.frame_list));

+ frame_list = xencomm_map(

+ xen_guest_handle(setup.frame_list),

+ (sizeof(ulong) * setup.nr_frames));

s/sizeof(ulong)/sizeof(*xen_guest_handle(setup.frame_list)/

set_xen_guest_handle(setup.frame_list, frame_list);

memcpy(op, &setup, sizeof(setup));

@@ -286,7 +287,7 @@ int HYPERVISOR_grant_table_op(unsigned i

return -ENOSYS;

}

- desc = xencomm_create_inline(op);

+ desc = xencomm_map(op, argsize);

ret = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_grant_table_op), cmd,

desc, count);

diff -r ab3b5849331d -r 82e1886955c3 arch/powerpc/platforms/xen/hcall.c

--- a/arch/powerpc/platforms/xen/hcall.c Sun Jan 21 08:36:53 2007 -0500

+++ b/arch/powerpc/platforms/xen/hcall.c Mon Jan 22 14:47:15 2007 -0600

@@ -54,7 +54,7 @@

int HYPERVISOR_console_io(int cmd, int count, char *str)

{

- void *desc = xencomm_create_inline(str);

+ void *desc =  xencomm_map_early(str, count);

return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_console_io),

  cmd, count, desc);

@@ -63,19 +63,24 @@ EXPORT_SYMBOL(HYPERVISOR_console_io);

int HYPERVISOR_event_channel_op(int cmd, void *op)

{

- void *desc = xencomm_create_inline(op);

+ void *desc = xencomm_map_early(op, sizeof(evtchn_op_t));

return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_event_channel_op),

cmd, desc);

}

EXPORT_SYMBOL(HYPERVISOR_event_channel_op);

-int HYPERVISOR_xen_version_userspace(int cmd, void *arg)

+int HYPERVISOR_xen_version(int cmd, void *arg)

{

struct xencomm_desc *desc;

const unsigned long hcall = __HYPERVISOR_xen_version;

int argsize;

int rc;

+

+ if (is_phys_contiguous((unsigned long)arg)) {

+ desc = xencomm_map_early(arg, sizeof(__u64));

+ return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_xen_version), cmd, desc);

+ }

But what if cmd is not 8 bytes like the many choices below?!

switch (cmd) {

case XENVER_version:

@@ -118,18 +123,10 @@ int HYPERVISOR_xen_version_userspace(int

}

EXPORT_SYMBOL(HYPERVISOR_xen_version);

-int HYPERVISOR_xen_version(int cmd, void *arg)

-{

- if (is_kernel_addr((unsigned long)arg)) {

- void *desc = xencomm_create_inline(arg);

- return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_xen_version), cmd, desc);

- }

- return HYPERVISOR_xen_version_userspace(cmd, arg);

-}

int HYPERVISOR_physdev_op(int cmd, void *op)

Interrupt FOR SURE, please use _early.

{

- void *desc = xencomm_create_inline(op);

+ void *desc = xencomm_map(op, sizeof(physdev_op_t));

return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_physdev_op),

cmd, desc);

@@ -138,6 +135,7 @@ EXPORT_SYMBOL(HYPERVISOR_physdev_op);

Interrupt FOR SURE, please use _early.

int HYPERVISOR_sched_op(int cmd, void *arg)

{

+ int argsize = 0;

struct xencomm_desc *desc;

switch (cmd) {

@@ -151,24 +149,30 @@ int HYPERVISOR_sched_op(int cmd, void *a

Interrupt FOR SURE, please use _early.

evtchn_port_t *ports;

struct sched_poll sched_poll;

+ argsize = sizeof(struct sched_poll);

+

memcpy(&sched_poll, arg, sizeof(sched_poll));

- ports = xencomm_create_inline(

- xen_guest_handle(sched_poll.ports));

+ ports = xencomm_map(

+ xen_guest_handle(sched_poll.ports),

+ (sizeof(evtchn_port_t) * sched_poll.nr_ports));

set_xen_guest_handle(sched_poll.ports, ports);

memcpy(arg, &sched_poll, sizeof(sched_poll));

}

break;

case SCHEDOP_shutdown:

+ argsize = sizeof(struct sched_shutdown);

+ break;

case SCHEDOP_remote_shutdown:

+ argsize = sizeof(struct sched_remote_shutdown);

break;

default:

printk(KERN_ERR "%s: unknown sched op %d\n", __func__, cmd);

return -ENOSYS;

}

- desc = xencomm_create_inline(arg);

+ desc = xencomm_map(arg, argsize);

return plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),

cmd, desc);

diff -r ab3b5849331d -r 82e1886955c3 drivers/xen/core/xencomm.c

--- a/drivers/xen/core/xencomm.c Sun Jan 21 08:36:53 2007 -0500

+++ b/drivers/xen/core/xencomm.c Mon Jan 22 14:47:15 2007 -0600

@@ -123,9 +123,89 @@ void *xencomm_create_inline(void *ptr)

Since you removed all external callers, this is either gone, or static.

{

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" routines, for stack-based communications: */

+static void *xencomm_alloc_mini(void *area, int arealen)

I thought we agreed this is no longer needed, because of the alignment attribute in its allocation

+{

+ unsigned long base = (unsigned long)area;

+ unsigned int left_in_page;

+

+ left_in_page = PAGE_SIZE - base % PAGE_SIZE;

+

+ /* we probably fit right at the front of area */

+ if (left_in_page >= sizeof(struct xencomm_mini)) {

+ return area;

+ }

+

+ /* if not, see if area is big enough to advance to the next page */

+ if ((arealen - left_in_page) >= sizeof(struct xencomm_mini))

+ return (void *)(base + left_in_page);

+

+ /* area was too small */

+ return NULL;

+}

+

+int xencomm_create_mini(void *area, int arealen, void *buffer,

+ unsigned long bytes, struct xencomm_desc **ret)

After _alloc_mini() is gone this function reduces to

{

desc->nr_addrs = XENCOMM_MINI_ADDRS;

if (!(rc = xencomm_init(desc, buffer, bytes)))

*ret = desc;

return rc;

}

+{

+ struct xencomm_desc *desc;

+ int rc;

+

+ desc = xencomm_alloc_mini(area, arealen);

+ if (!desc)

+ return -ENOMEM;

+ desc->nr_addrs = XENCOMM_MINI_ADDRS;

+

+ rc = xencomm_init(desc, buffer, bytes);

+ if (rc)

+ return rc;

+

+ *ret = desc;

+ return 0;

+}

+

+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,

+ char *xc_area)

+{

+ int rc;

+ struct xencomm_desc *desc;

+

+ if (is_phys_contiguous((unsigned long)ptr))

+ return xencomm_create_inline(ptr);

+

+ rc = xencomm_create_mini(xc_area, XENCOMM_MINI_AREA,ptr, bytes,

+ &desc);

+

+ if (rc)

+ return NULL;

+

+ return (void*)__pa(desc);

+}

+

+/* check if is physically contiguous memory */

+int is_phys_contiguous(unsigned long addr)

+{

+ return (addr < VMALLOC_START) || (addr >= VMALLOC_END);

+}

diff -r ab3b5849331d -r 82e1886955c3 include/xen/xencomm.h

--- a/include/xen/xencomm.h Sun Jan 21 08:36:53 2007 -0500

+++ b/include/xen/xencomm.h Mon Jan 22 14:47:15 2007 -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,25 @@

#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)

+

extern void xencomm_free(struct xencomm_desc *desc);

+extern int xencomm_create(void *buffer, unsigned long bytes,

+ struct xencomm_desc **ret, gfp_t gfp_mask);

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,

+ char *xc_area);

+extern int is_phys_contiguous(unsigned long addr);

+

+#define xencomm_map_early(ptr, bytes) \

+ ({char xc_area[XENCOMM_MINI_AREA];\

+        __xencomm_map_early(ptr, bytes, xc_area);})

/* provided by architecture code: */

extern unsigned long xencomm_vtop(unsigned long vaddr);

# HG changeset patch

# User Jerone Young <jyoung5@xxxxxxxxxx>

# Date 1169614163 21600

# Node ID c326865b79895e91ac57e548eda03cda3c4c0dcc

# Parent  82e1886955c349ee9c473560ead37f61b787fcd7

Use structure instead of char

It is not the char that is important, it is the alignment attribute

diff -r 82e1886955c3 -r c326865b7989 drivers/xen/core/xencomm.c

--- a/drivers/xen/core/xencomm.c Mon Jan 22 14:47:15 2007 -0600

+++ b/drivers/xen/core/xencomm.c Tue Jan 23 22:49:23 2007 -0600

@@ -187,7 +187,7 @@ void *xencomm_map(void *ptr, unsigned lo

}

void *__xencomm_map_early(void *ptr, unsigned long bytes,

- char *xc_area)

+ struct xencomm_mini *xc_area)

{

int rc;

struct xencomm_desc *desc;

diff -r 82e1886955c3 -r c326865b7989 include/xen/xencomm.h

--- a/include/xen/xencomm.h Mon Jan 22 14:47:15 2007 -0600

+++ b/include/xen/xencomm.h Tue Jan 23 22:49:23 2007 -0600

@@ -37,12 +37,13 @@ extern void *xencomm_create_inline(void

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,

- char *xc_area);

+ struct xencomm_mini *xc_area);

extern int is_phys_contiguous(unsigned long addr);

#define xencomm_map_early(ptr, bytes) \

- ({char xc_area[XENCOMM_MINI_AREA];\

-        __xencomm_map_early(ptr, bytes, xc_area);})

+ ({struct xencomm_mini xc_area[XENCOMM_MINI_AREA]\

This is no longer and array, and XENCOMM_MINI_AREA can be removed complety.

+ __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