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.
> -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.
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)?
--
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
|
Home