RFC: vaddr_t and vsize_t

RFC: vaddr_t and vsize_t

[Christoph Egger]

Hi!

*BSD uses two types for virtual addresses and the virtual address space:
vaddr_t and vsize_t.

vaddr_t is used when a virtual address is meant. It is an unsigned integer and 
its size always matches sizeof(void *).
vsize_t is used when the virtual address space is meant. It is an unsigned 
integer and its size always matches the whole size of the virtual address 
space.

The difference to the pointer is that the pointer is only used to point to
something specific, e.g. a structure in the kernel.

That way mixing up virtual addresses, virtual address spaces and pointers
is hardly possible. What do you think about adding those types to Xen?


The type definition for x86_32 and x86_64 would be this:

diff -r fd2667419c53 xen/include/asm-x86/types.h
--- a/xen/include/asm-x86/types.h       Tue Jan 16 14:04:12 2007 -0500
+++ b/xen/include/asm-x86/types.h       Thu Nov 09 10:23:14 2006 +0100
@@ -36,6 +36,9 @@ typedef unsigned int u32;
 #if defined(__i386__)
 typedef signed long long s64;
 typedef unsigned long long u64;
+/* datatype to store virtual addresses. */
+typedef unsigned long vaddr_t;
+typedef unsigned long vsize_t;
 #if defined(CONFIG_X86_PAE)
 typedef u64 paddr_t;
 #define PRIpaddr "016llx"
@@ -47,6 +50,8 @@ typedef signed long s64;
 typedef signed long s64;
 typedef unsigned long u64;
 typedef unsigned long paddr_t;
+typedef unsigned long vaddr_t;
+typedef unsigned long vsize_t;
 #define PRIpaddr "016lx"
 #endif

Re: RFC: vaddr_t and vsize_t

[Keir Fraser]

On 17/1/07 15:17, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> vaddr_t is used when a virtual address is meant. It is an unsigned integer and
> its size always matches sizeof(void *).
> vsize_t is used when the virtual address space is meant. It is an unsigned
> integer and its size always matches the whole size of the virtual address
> space.

Is this distinction ever useful?

The assumption that a pointer fits in a long is rather ingrained into Xen's
code base. Any attempt to remove that assumption is going to need some big
patches, and it's really not clear that we ought to care as I'm sure it's
true for any architecture we can conceive of caring one jot about.

 -- Keir

Re: RFC: vaddr_t and vsize_t

[Christoph Egger]

On Wednesday 17 January 2007 16:49, Keir Fraser wrote:
> On 17/1/07 15:17, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > vaddr_t is used when a virtual address is meant. It is an unsigned
> > integer and its size always matches sizeof(void *).
> > vsize_t is used when the virtual address space is meant. It is an
> > unsigned integer and its size always matches the whole size of the
> > virtual address space.
>
> Is this distinction ever useful?

As long as no architecture is added to Xen which uses ILP rather LP64,
vsize_t is not really needed. For vaddr_t see below.

> The assumption that a pointer fits in a long is rather ingrained into Xen's
> code base. Any attempt to remove that assumption is going to need some big
> patches, and it's really not clear that we ought to care as I'm sure it's
> true for any architecture we can conceive of caring one jot about.

Once an ILP arch comes up, the assumption is no longer true. Until this 
happens, noone need to care.

This stuff is about removing a place where programmers can make mistakes.
vaddr_t also avoids void * arithmetics. It should be possible then to use 
-Wpointer-arith.

I don't mind if you dislike vsize_t for the above reason. What do you think
about vaddr_t?

Christoph

Re: RFC: vaddr_t and vsize_t

[Keir Fraser]

On 17/1/07 16:21, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> This stuff is about removing a place where programmers can make mistakes.
> vaddr_t also avoids void * arithmetics. It should be possible then to use
> -Wpointer-arith.

A bunch of our void* arithmetic comes from files we keep in sync with Linux.
So we'd really need Linux to clean this up before we use -Wpointer-arith, or
we have to do extra porting effort to keep files in sync.

I suppose I don't mind vaddr_t being defined and people using it if they
want to. At least it is probably clearer that translation to/from char* or
long to do pointer arithmetic. However, I'm less sure about a wholesale
conversion of Xen away from use of 'unsigned long'.

 -- Keir

Re: RFC: vaddr_t and vsize_t

[Christoph Egger]

On Wednesday 17 January 2007 17:51, Keir Fraser wrote:
> On 17/1/07 16:21, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > This stuff is about removing a place where programmers can make mistakes.
> > vaddr_t also avoids void * arithmetics. It should be possible then to use
> > -Wpointer-arith.
>
> A bunch of our void* arithmetic comes from files we keep in sync with
> Linux. So we'd really need Linux to clean this up before we use
> -Wpointer-arith, or we have to do extra porting effort to keep files in
> sync.

Well, actually I'm talking about the Xen-kernel and not about the 
Linux-kernel.

> I suppose I don't mind vaddr_t being defined and people using it if they
> want to. At least it is probably clearer that translation to/from char* or
> long to do pointer arithmetic. However, I'm less sure about a wholesale
> conversion of Xen away from use of 'unsigned long'.

Ok. I will prepare a patch. Someone has to make the start. :-)

Re: RFC: vaddr_t and vsize_t

[Keir Fraser]

On 18/1/07 7:13 am, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> I suppose I don't mind vaddr_t being defined and people using it if they
>> want to. At least it is probably clearer that translation to/from char* or
>> long to do pointer arithmetic. However, I'm less sure about a wholesale
>> conversion of Xen away from use of 'unsigned long'.
> 
> Ok. I will prepare a patch. Someone has to make the start. :-)

Start with a small patch and let's see how it goes.

 -- Keir