Re: [Xen-staging] [xen-unstable] linux: User-space grant table device.

Re: [Xen-staging] [xen-unstable] linux: User-space grant table device.

[Alex Williamson]

On Sat, 2007-03-31 at 13:52 +0100, Xen staging patchbot-unstable wrote:
> # HG changeset patch
> # User Keir Fraser <keir@xensource.com>
> # Date 1175345604 -3600
> # Node ID 5e65a86c8982b9159223b542cc7b7e3af61a06e4
> # Parent  7180d2e61f926023e24750c53fd4203a71f2a3ae
> linux: User-space grant table device.
> 
> A character device for accessing (in user-space) pages that have been
> granted by other domains.
...
> +	/* This flag ensures that the page tables are not unpinned before the
> +	 * VM area is unmapped. Therefore Xen still recognises the PTE as
> +	 * belonging to an L1 pagetable, and the grant unmap operation will
> +	 * succeed, even if the process does not exit cleanly.
> +	 */
> +	vma->vm_mm->context.has_foreign_mappings = 1;

   mm_context_t is arch specific, common code shouldn't be poking at it
like this.

	Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.

Re: Re: [Xen-staging] [xen-unstable] linux: User-space grant table device.

[Keir Fraser]

On 31/3/07 16:33, "Alex Williamson" <alex.williamson@hp.com> wrote:

>> + /* This flag ensures that the page tables are not unpinned before the
>> +  * VM area is unmapped. Therefore Xen still recognises the PTE as
>> +  * belonging to an L1 pagetable, and the grant unmap operation will
>> +  * succeed, even if the process does not exit cleanly.
>> +  */
>> + vma->vm_mm->context.has_foreign_mappings = 1;
> 
>    mm_context_t is arch specific, common code shouldn't be poking at it
> like this.

Good point. That part is entirely x86-specific. I'll add an ifdef.

 -- Keir

Re: [Xen-staging] [xen-unstable] linux: User-space grant table device.

[Alex Williamson]

On Sat, 2007-03-31 at 13:52 +0100, Xen staging patchbot-unstable wrote:
> --- a/linux-2.6-xen-sparse/drivers/xen/util.c	Sat Mar 31 12:42:02 2007 +0100
> +++ b/linux-2.6-xen-sparse/drivers/xen/util.c	Sat Mar 31 13:53:24 2007 +0100
> @@ -4,6 +4,23 @@
>  #include <linux/vmalloc.h>
>  #include <asm/uaccess.h>
>  #include <xen/driver_util.h>
> +
> +struct class *get_xen_class(void)
> +{
> +	static struct class *xen_class;
> +
> +	if (xen_class)
> +		return xen_class;
> +
> +	xen_class = class_create(THIS_MODULE, "xen");
> +	if (IS_ERR(xen_class)) {
> +		printk("Failed to create xen sysfs class.\n");
> +		xen_class = NULL;
> +	}
> +
> +	return xen_class;
> +}
> +EXPORT_SYMBOL_GPL(get_xen_class);

   Maybe this could be moved to a more common spot since only x86 builds
with CONFIG_XEN_UTIL?  Thanks,

	Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.

Re: Re: [Xen-staging] [xen-unstable] linux: User-space grant table device.

[Keir Fraser]

On 31/3/07 16:46, "Alex Williamson" <alex.williamson@hp.com> wrote:

>    Maybe this could be moved to a more common spot since only x86 builds
> with CONFIG_XEN_UTIL?  Thanks,

You can't get much more generic than a file named 'util.c' at the root of
drivers/xen.

Everyone should build it and the x86-specific portions (if there really are
any -- it all looks pretty generic to me even if no other architectures
currently use the functions defined in there) should be ifdef'ed or perhaps
relocated to a new file.

 -- Keir

Re: Re: [Xen-staging] [xen-unstable] linux: User-space grant table device.

[Alex Williamson]

On Sat, 2007-03-31 at 16:56 +0100, Keir Fraser wrote:
> On 31/3/07 16:46, "Alex Williamson" <alex.williamson@hp.com> wrote:
> 
> >    Maybe this could be moved to a more common spot since only x86 builds
> > with CONFIG_XEN_UTIL?  Thanks,
> 
> You can't get much more generic than a file named 'util.c' at the root of
> drivers/xen.
> 
> Everyone should build it and the x86-specific portions (if there really are
> any -- it all looks pretty generic to me even if no other architectures
> currently use the functions defined in there) should be ifdef'ed or perhaps
> relocated to a new file.

   True, util.c ought to be a good place to dump stuff like this.
Unfortunately we define our own alloc/free_vm_area(), so the existing
functions in there are the problems.  Maybe those should be moved to
arch/i386/mach-xen/util.c, or ifdef out as below.  Thanks,

	Alex

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
---

diff -r fbe52b559161 linux-2.6-xen-sparse/arch/ia64/Kconfig
--- a/linux-2.6-xen-sparse/arch/ia64/Kconfig	Sat Mar 31 14:05:57 2007 +0100
+++ b/linux-2.6-xen-sparse/arch/ia64/Kconfig	Sat Mar 31 10:24:30 2007 -0600
@@ -577,7 +577,7 @@ source "crypto/Kconfig"
 #
 if XEN
 config XEN_UTIL
-	default n
+	default y
 
 config XEN_BALLOON
 	default y
diff -r fbe52b559161 linux-2.6-xen-sparse/drivers/xen/util.c
--- a/linux-2.6-xen-sparse/drivers/xen/util.c	Sat Mar 31 14:05:57 2007 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/util.c	Sat Mar 31 10:24:01 2007 -0600
@@ -21,6 +21,8 @@ struct class *get_xen_class(void)
 	return xen_class;
 }
 EXPORT_SYMBOL_GPL(get_xen_class);
+
+#ifndef CONFIG_IA64
 
 static int f(pte_t *pte, struct page *pmd_page, unsigned long addr, void *data)
 {
@@ -63,3 +65,5 @@ void free_vm_area(struct vm_struct *area
 	kfree(area);
 }
 EXPORT_SYMBOL_GPL(free_vm_area);
+
+#endif /* !CONFIG_IA64 */

Re: Re: [Xen-staging] [xen-unstable] linux: User-space grant table device.

[Keir Fraser]

On 31/3/07 17:34, "Alex Williamson" <alex.williamson@hp.com> wrote:

>> Everyone should build it and the x86-specific portions (if there really are
>> any -- it all looks pretty generic to me even if no other architectures
>> currently use the functions defined in there) should be ifdef'ed or perhaps
>> relocated to a new file.
> 
>    True, util.c ought to be a good place to dump stuff like this.
> Unfortunately we define our own alloc/free_vm_area(), so the existing
> functions in there are the problems.  Maybe those should be moved to
> arch/i386/mach-xen/util.c, or ifdef out as below.  Thanks,

Oh I see. Actually I think the difference is not strictly
architecture-related, but due to the fact that ia64 uses auto-translate, so
you need actual pseudophysical addresses relating to the virtual address
that is returned. The right answer here is to have a combined function that
tests for auto_translated_physmap. Possibly the interface function should
even be modified to do allocate-area-and-map, rather than leaving the
mapping to a separate function. Anyhow, for now I'll make the 'generic'
versions __attribute__((weak)).

 -- Keir