RE: tmem 32-on-64 needs (Re: [Xen-devel] [PATCH] tmem cleanup)

[Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>]

Hi Jan --

Thanks much for looking at this.

32-on-64 support does work already, but I think as we discussed
before, it is due to careful structure organizaton which is
fragile (e.g. if someone adds another field in the future).

I wasn't aware of the restriction on public unions.  The
changes are strictly syntactic, correct?

> and the compatibility header generation script depends on
> all compound types' fields to be named.

Ah, so that's what was causing those problems for me!

> first wanted to see if there's anything that I'm overlooking.

I can't think of anything you might be overlooking.
Do you want me to submit the patch or do you already
have one underway?

>--- 2009-06-10.orig/xen/common/tmem_xen.c      2009-05-27 13:54:07.000000000 
>+0200
>+++ 2009-06-10/xen/common/tmem_xen.c   2009-06-15 15:00:48.000000000 +0200
>@@ -87,10 +87,7 @@ static inline void *cli_mfn_to_va(tmem_c
>     unsigned long cli_mfn;
>     p2m_type_t t;
> 
>-
>-    if (is_pv_32on64_vcpu(current))
>-        cmfn.p = (void *)((unsigned long)cmfn.p & 0xffffffffUL);
>-    cli_mfn = mfn_x(gfn_to_mfn(current->domain,(unsigned long)cmfn.p,&t));
>+    cli_mfn = mfn_x(gfn_to_mfn(current->domain, cmfn, &t));

Are you sure this works?  I seem to recall I tried that first
and it failed on 32-on-64.  However it was a long time ago
so may have been due to a different problem.

Thanks again,
Dan

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxxxx]
> Sent: Monday, June 15, 2009 8:09 AM
> To: Dan Magenheimer
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: tmem 32-on-64 needs (Re: [Xen-devel] [PATCH] tmem cleanup)
> 
> 
> >>> "Jan Beulich" <JBeulich@xxxxxxxxxx> 15.06.09 15:47 >>>
> >- don't mis-use guest handle for passing an MFN value
> >- eliminate unnecessary (and misplaced) use of XEN_GUEST_HANDLE_64
> >- use copy_from_guest() instead of __copy_from_guest() for 
> loading the
> >  argument structure
> 
> I ran across these when looking at what changes 32-on-64 
> support for tmem
> would require. However, there's another issue that I didn't 
> feel like immediately
> sending a patch out for: xen/include/public/tmem.h uses 
> anonymous unions
> and structures - all other public headers (with the exception 
> of the declaration
> of xenpf_set_processor_pminfo and xen_sysctl_pm_op, which probably are
> mistakes/oversights) avoid this (or use it only when it is 
> known to be only
> compiled with gcc)) as being a non-standard feature, and the 
> compatibility
> header generation script depends on all compound types' 
> fields to be named.
> 
> I realize that this will require changes on the client side 
> too, which is why I
> first wanted to see if there's anything that I'm overlooking.
> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
>

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