Re: [Xen-devel] [PATCH v2 5/5] xen: replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when appropriate

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 14 Aug 2012, Jan Beulich wrote:
> >>> On 14.08.12 at 12:38, Stefano Stabellini 
> >>> <stefano.stabellini@xxxxxxxxxxxxx>
> wrote:
> > On Mon, 13 Aug 2012, Jan Beulich wrote:
> >> >>> On 13.08.12 at 13:24, Stefano Stabellini 
> >> >>> <stefano.stabellini@xxxxxxxxxxxxx> 
> > wrote:
> >> > On the other hand if you mean casting a XEN_GUEST_HANDLE_PARAM into a
> >> > XEN_GUEST_HANDLE to pass it to other internal functions, I don't think
> >> > there is any point in that because the other internal functions should
> >> > also have XEN_GUEST_HANDLE_PARAMs as parameters.
> >> 
> >> So you obviously need a cast from "normal" to _PARAM (so that
> >> you can pass embedded fields to functions).
> > 
> > In practice embedded fields are in guest memory, so the first thing Xen
> > does is calling copy_from_guest and work with the struct pointer
> > directly from that point on.
> 
> Perhaps we have a different understanding of embedded fields:
> I'm thinking of structure field having XEN_GUEST_HANDLE() type.
> An example would be struct mmuext_op's vcpumask field, which
> is being passed to vcpumask_to_pcpumask(). This must remain to
> be possible (and not just in x86-specific code, where it's mere luck
> that both are really identical).

Thanks for the concrete example; glancing through the common code I
didn't find any examples like this.
As I wrote in the follow up email, guest_handle_cast is just what we
need:

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 4d72700..70ffa58 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3198,7 +3198,9 @@ int do_mmuext_op(
         {
             cpumask_t pmask;
 
-            if ( unlikely(vcpumask_to_pcpumask(d, op.arg2.vcpumask, &pmask)) )
+            if ( unlikely(vcpumask_to_pcpumask(d,
+                            guest_handle_cast(op.arg2.vcpumask, const_void),
+                            &pmask)) )
             {
                 okay = 0;
                 break;

Unfortuantely that means that I have missed some substitutions from my
original "replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when
appropriate" patch.
I'll try to catch them in the next version.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel