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.c Tue Dec 19 09:22:37 2006 -0500
> +++ b/arch/powerpc/platforms/xen/hcall.c Wed 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.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;
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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
|