WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ppc-devel

Re: [XenPPC] [PATCH] [RFC] Fix xenminicom optimizations to work for modu

To: Jerone Young <jyoung5@xxxxxxxxxx>
Subject: Re: [XenPPC] [PATCH] [RFC] Fix xenminicom optimizations to work for module
From: Hollis Blanchard <hollisb@xxxxxxxxxx>
Date: Wed, 10 Jan 2007 12:45:38 -0600
Cc: Xen-ppc <xen-ppc-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 10 Jan 2007 10:45:35 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <1168451469.5784.28.camel@thinkpad>
List-help: <mailto:xen-ppc-devel-request@lists.xensource.com?subject=help>
List-id: Xen PPC development <xen-ppc-devel.lists.xensource.com>
List-post: <mailto:xen-ppc-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ppc-devel>, <mailto:xen-ppc-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ppc-devel>, <mailto:xen-ppc-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: IBM Linux Technology Center
References: <1168451469.5784.28.camel@thinkpad>
Reply-to: Hollis Blanchard <hollisb@xxxxxxxxxx>
Sender: xen-ppc-devel-bounces@xxxxxxxxxxxxxxxxxxx
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