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] Fix xenminicom optimazation to work for modules

To: Hollis Blanchard <hollisb@xxxxxxxxxx>
Subject: Re: [XenPPC] [PATCH] Fix xenminicom optimazation to work for modules
From: Jerone Young <jyoung5@xxxxxxxxxx>
Date: Thu, 11 Jan 2007 14:56:18 -0600
Cc: Xen-ppc <xen-ppc-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 11 Jan 2007 12:56:00 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <1168542396.25395.18.camel@basalt>
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>
References: <1168539130.26923.2.camel@thinkpad> <1168542396.25395.18.camel@basalt>
Sender: xen-ppc-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Thu, 2007-01-11 at 13:06 -0600, Hollis Blanchard wrote:
> On Thu, 2007-01-11 at 12:12 -0600, Jerone Young wrote:
> >
> > @@ -121,7 +156,7 @@ int HYPERVISOR_xen_version(int cmd, void
> >  int HYPERVISOR_xen_version(int cmd, void *arg)
> >  {
> >         if (is_kernel_addr((unsigned long)arg)) {
> > -               void *desc = xencomm_create_inline(arg);
> > +               void *desc = xencomm_create_inline(arg, sizeof(__u64));
> >                 return 
> > plpar_hcall_norets(XEN_MARK(__HYPERVISOR_xen_version), cmd, desc);
> >         }
> >         return HYPERVISOR_xen_version_userspace(cmd, arg);
> 
> Any use of is_kernel_addr() is a red flag.

So the issue here is what else to use to use the userspace function. It
looked liked like this was actually a correct way of going about it in
this case. I'll figure something better.

> 
> > -void *xencomm_create_inline(void *ptr)
> > +void *xencomm_create_inline(void *ptr, unsigned long bytes)
> >  {
> >         unsigned long paddr;
> > +       unsigned long first_page;
> > +       unsigned long last_page;
> >
> > -       BUG_ON(!is_kernel_addr((unsigned long)ptr));
> > +       first_page = xencomm_vtop(ptr) & PAGE_MASK;
> > +       last_page = xencomm_vtop(ptr + bytes) & PAGE_MASK;
> > +
> > +       if (first_page != last_page)
> > +               return NULL;
> 
> I still think you should drop xencomm_vtop(). If ptr and ptr+bytes are
> on different virtual pages, they will be on different physical pages
> too, so we don't need to do the more expensive virtual-to-physical
> translation you're doing here.

Will drop it.

> 
> 
> 
> 
> But anyways, let's think about abstracting this a little bit.
> Pseudo-code below.
> 
> First, the test we really want is "is this area of memory physically
> contiguous?" If so, we can do inline.
> 
> void *xencomm_map(void *ptr, ulong bytes)
> {
>       if (is_phys_contiguous(ptr))
>               return xencomm_create_inline(ptr);
>       return xencomm_create(ptr, bytes);
> }
> 
> In particular we know that vmalloc space, from which modules are
> allocated, is not physically contiguous. Other code makes reference to
> VMALLOC_START/END, so we can too:
> 
> int is_physically_contiguous(ulong addr)
> {
>       return (ptr < VMALLOC_START) || (ptr >= VMALLOC_END);
> }
> 
> We can have a separate "early" function:
> 
> #define xencomm_map_early(ptr, bytes) \
>       char xc_area[bytes*2]; \
>       __xencomm_map_early(ptr, bytes, xc_area)
> 
> void *__xencomm_map_early(void *ptr, ulong bytes, char *xc_area)
> {
>       if (is_phys_contiguous(ptr))
>               return xencomm_create_inline(ptr);
>       return xencomm_create_mini(ptr, bytes, xc_area);
> }
> 
> (We need that macro because the *caller* needs to allocate stack space.)
> 
> With these interfaces, all a caller needs to do is use xencomm_map() or
> xencomm_map_early(), and all the details of inline or mini or regular
> are hidden.
> 
> Does this make sense (to anybody)?

Got the basics. I'll get out another patch and that will show if I got
what you are saying.

> 
> --
> Hollis Blanchard
> IBM Linux Technology Center
> 


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

<Prev in Thread] Current Thread [Next in Thread>