Re: [Xen-devel] [PATCH 17/21] xen: xen_ulong_t substitution

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-10-09 at 13:51 +0100, Stefano Stabellini wrote:
> On Tue, 9 Oct 2012, Ian Campbell wrote:
> > On Tue, 2012-10-09 at 13:39 +0100, Stefano Stabellini wrote:
> > > On Fri, 5 Oct 2012, Ian Campbell wrote:
> > > > On Fri, 2012-10-05 at 11:38 +0100, Ian Campbell wrote:
> > > > > From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > > > > 
> > > > > There is still an unwanted unsigned long in the xen public interface:
> > > > > replace it with xen_ulong_t.
> > > > > 
> > > > > Also typedef xen_ulong_t to uint64_t on ARM.
> > > > 
> > > > Should this change be applied to the uses of XEN_GUEST_HANDLE(ulong)
> > > > too? My main concern is the one in struct gnttab_setup_table but there
> > > > are a few others.
> > > > 
> > > > I suspect XEN_GUEST_HANDLE(ulong) needs to be removed entirely,
> > > > everywhere it is used should be XEN_GUEST_HANDLE(xen_ulong_t) instead?
> > > 
> > > It is not necessary, because all the XEN_GUEST_HANDLE(ulong) are already
> > > 64 bit on ARM. A 32 bit guest is going to pass a 32 bit unsigned long in a
> > > 64 bit field, while a 64 bit guest is going to pass a 64 bit unsigned
> > > long in a 64 bit field. Either way it will work.
> > 
> > XEN_GUEST_HANDLE(ulong) is unsigned long on all platforms, see
> > xen/include/public/xen.h:
> >         __DEFINE_XEN_GUEST_HANDLE(ulong, unsigned long);
> > 
> > The existence of this handle is dangerous since it contains a type which
> > varies in size but it is (slightly) opaque so you might not notice.
> > 
> > The ulong handle is only really usable/desirable on x86 where retaining
> > the ABI requires us to use unsigned long for some fields, but we have
> > already defined xen_ulong_t which has the correct semantics on both ARM
> > and x86.
> > 
> > I propose the following.
> 
> It is certainly an improvement. Also I didn't notice the
> XEN_GUEST_HANDLE_PARAM(ulong): that is actually an error.
> We also need a corresponding patch for Linux.

I need to tesdt both this and the h/v side a bit more but here it is.

8<---------------------------

>From c55591bbe3b1d5164641075b95f3c95418bcdf79 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Fri, 5 Oct 2012 17:39:19 +0100
Subject: [PATCH] xen: grant: use xen_pfn_t type for frame_list.

This correctly sizes it as 64 bit on ARM but leaves it as unsigned
long on x86 (therefore no intended change on x86).

The long and ulong guest handles are now unused (and a bit dangerous)
so remove them.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 arch/arm/include/asm/xen/interface.h |    2 --
 arch/x86/include/asm/xen/interface.h |    2 --
 include/xen/interface/grant_table.h  |    2 +-
 3 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/xen/interface.h 
b/arch/arm/include/asm/xen/interface.h
index ad87917..9ac9f4e 100644
--- a/arch/arm/include/asm/xen/interface.h
+++ b/arch/arm/include/asm/xen/interface.h
@@ -35,10 +35,8 @@ typedef uint64_t xen_ulong_t;
 /* Guest handles for primitive C types. */
 __DEFINE_GUEST_HANDLE(uchar, unsigned char);
 __DEFINE_GUEST_HANDLE(uint,  unsigned int);
-__DEFINE_GUEST_HANDLE(ulong, unsigned long);
 DEFINE_GUEST_HANDLE(char);
 DEFINE_GUEST_HANDLE(int);
-DEFINE_GUEST_HANDLE(long);
 DEFINE_GUEST_HANDLE(void);
 DEFINE_GUEST_HANDLE(uint64_t);
 DEFINE_GUEST_HANDLE(uint32_t);
diff --git a/arch/x86/include/asm/xen/interface.h 
b/arch/x86/include/asm/xen/interface.h
index d67f3c6..ed602f8 100644
--- a/arch/x86/include/asm/xen/interface.h
+++ b/arch/x86/include/asm/xen/interface.h
@@ -55,10 +55,8 @@ typedef unsigned long xen_ulong_t;
 /* Guest handles for primitive C types. */
 __DEFINE_GUEST_HANDLE(uchar, unsigned char);
 __DEFINE_GUEST_HANDLE(uint,  unsigned int);
-__DEFINE_GUEST_HANDLE(ulong, unsigned long);
 DEFINE_GUEST_HANDLE(char);
 DEFINE_GUEST_HANDLE(int);
-DEFINE_GUEST_HANDLE(long);
 DEFINE_GUEST_HANDLE(void);
 DEFINE_GUEST_HANDLE(uint64_t);
 DEFINE_GUEST_HANDLE(uint32_t);
diff --git a/include/xen/interface/grant_table.h 
b/include/xen/interface/grant_table.h
index f9f8b97..e40fae9 100644
--- a/include/xen/interface/grant_table.h
+++ b/include/xen/interface/grant_table.h
@@ -310,7 +310,7 @@ struct gnttab_setup_table {
     uint32_t nr_frames;
     /* OUT parameters. */
     int16_t  status;              /* GNTST_* */
-    GUEST_HANDLE(ulong) frame_list;
+    GUEST_HANDLE(xen_pfn_t) frame_list;
 };
 DEFINE_GUEST_HANDLE_STRUCT(gnttab_setup_table);
 
-- 
1.7.2.5




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