[PATCH] Use fixed-width types in the memory event interface

[PATCH] Use fixed-width types in the memory event interface

[Tim Deegan]

Set the types in the public memory_event header file to use fixed-sized
and self-aligned fields rather than "unsigned long".  AIUI this feature
only works with 64-bit hypervisors but I think this change will be
necessary to use 32-on-64 dom0 tools.

This breaks compatibility with older builds of the tools, but I can't
see any way to avoid it short of __attribute__((__packed__)).

I'd like an ack/nack from Patrick on this, please. 

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r 7b00193bd033 xen/include/public/mem_event.h
--- a/xen/include/public/mem_event.h	Mon Jun 28 17:40:16 2010 +0100
+++ b/xen/include/public/mem_event.h	Tue Jun 29 14:38:06 2010 +0100
@@ -40,14 +40,14 @@
 
 
 typedef struct mem_event_shared_page {
-    int port;
+    uint32_t port;
 } mem_event_shared_page_t;
 
 typedef struct mem_event_st {
-    unsigned long gfn;
-    unsigned long offset;
-    unsigned long p2mt;
-    int vcpu_id;
+    uint64_t gfn;
+    uint64_t offset;
+    uint32_t p2mt;
+    int32_t vcpu_id;
     uint64_t flags;
 } mem_event_request_t, mem_event_response_t;

Re: [PATCH] Use fixed-width types in the memory event interface

[Patrick Colp]

On 29 June 2010 06:47, Tim Deegan <Tim.Deegan@citrix.com> wrote:
> Set the types in the public memory_event header file to use fixed-sized
> and self-aligned fields rather than "unsigned long".  AIUI this feature
> only works with 64-bit hypervisors but I think this change will be
> necessary to use 32-on-64 dom0 tools.
>
> This breaks compatibility with older builds of the tools, but I can't
> see any way to avoid it short of __attribute__((__packed__)).
>
> I'd like an ack/nack from Patrick on this, please.
>
> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>



> diff -r 7b00193bd033 xen/include/public/mem_event.h
> --- a/xen/include/public/mem_event.h    Mon Jun 28 17:40:16 2010 +0100
> +++ b/xen/include/public/mem_event.h    Tue Jun 29 14:38:06 2010 +0100
> @@ -40,14 +40,14 @@
>
>
>  typedef struct mem_event_shared_page {
> -    int port;
> +    uint32_t port;
>  } mem_event_shared_page_t;
>
>  typedef struct mem_event_st {
> -    unsigned long gfn;
> -    unsigned long offset;
> -    unsigned long p2mt;
> -    int vcpu_id;
> +    uint64_t gfn;
> +    uint64_t offset;
> +    uint32_t p2mt;
> +    int32_t vcpu_id;
>     uint64_t flags;
>  } mem_event_request_t, mem_event_response_t;
>
>
>

Re: [PATCH] Use fixed-width types in the memory event interface

[Patrick Colp]

A mostly unrelated question: I notice that vcpu_id is set to int32_t
rather than uint32_t. This matches with the vcpu_id field in struct
vcpu, but I'm just curious as to why this isn't an unsigned int in
struct vcpu. Is there ever a time when vcpu_id is negative? I realise
it's unlikely that we'll need to consider 4 billion vcpus (at least
not anytime soon), but just wondering if there's any particular reason
why this is a signed integer instead of an unsigned one.


Patrick


On 29 June 2010 07:28, Patrick Colp <pjcolp@cs.ubc.ca> wrote:
> On 29 June 2010 06:47, Tim Deegan <Tim.Deegan@citrix.com> wrote:
>> Set the types in the public memory_event header file to use fixed-sized
>> and self-aligned fields rather than "unsigned long".  AIUI this feature
>> only works with 64-bit hypervisors but I think this change will be
>> necessary to use 32-on-64 dom0 tools.
>>
>> This breaks compatibility with older builds of the tools, but I can't
>> see any way to avoid it short of __attribute__((__packed__)).
>>
>> I'd like an ack/nack from Patrick on this, please.
>>
>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
>
> Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>
>
>
>
>> diff -r 7b00193bd033 xen/include/public/mem_event.h
>> --- a/xen/include/public/mem_event.h    Mon Jun 28 17:40:16 2010 +0100
>> +++ b/xen/include/public/mem_event.h    Tue Jun 29 14:38:06 2010 +0100
>> @@ -40,14 +40,14 @@
>>
>>
>>  typedef struct mem_event_shared_page {
>> -    int port;
>> +    uint32_t port;
>>  } mem_event_shared_page_t;
>>
>>  typedef struct mem_event_st {
>> -    unsigned long gfn;
>> -    unsigned long offset;
>> -    unsigned long p2mt;
>> -    int vcpu_id;
>> +    uint64_t gfn;
>> +    uint64_t offset;
>> +    uint32_t p2mt;
>> +    int32_t vcpu_id;
>>     uint64_t flags;
>>  } mem_event_request_t, mem_event_response_t;
>>
>>
>>
>

Re: Re: [PATCH] Use fixed-width types in the memory event interface

[Keir Fraser]

On 29/06/2010 15:37, "Patrick Colp" <pjcolp@cs.ubc.ca> wrote:

> A mostly unrelated question: I notice that vcpu_id is set to int32_t
> rather than uint32_t. This matches with the vcpu_id field in struct
> vcpu, but I'm just curious as to why this isn't an unsigned int in
> struct vcpu. Is there ever a time when vcpu_id is negative? I realise
> it's unlikely that we'll need to consider 4 billion vcpus (at least
> not anytime soon), but just wondering if there's any particular reason
> why this is a signed integer instead of an unsigned one.

No.

 K.

> 
> Patrick
> 
> 
> On 29 June 2010 07:28, Patrick Colp <pjcolp@cs.ubc.ca> wrote:
>> On 29 June 2010 06:47, Tim Deegan <Tim.Deegan@citrix.com> wrote:
>>> Set the types in the public memory_event header file to use fixed-sized
>>> and self-aligned fields rather than "unsigned long".  AIUI this feature
>>> only works with 64-bit hypervisors but I think this change will be
>>> necessary to use 32-on-64 dom0 tools.
>>> 
>>> This breaks compatibility with older builds of the tools, but I can't
>>> see any way to avoid it short of __attribute__((__packed__)).
>>> 
>>> I'd like an ack/nack from Patrick on this, please.
>>> 
>>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
>> 
>> Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>
>> 
>> 
>> 
>>> diff -r 7b00193bd033 xen/include/public/mem_event.h
>>> --- a/xen/include/public/mem_event.h    Mon Jun 28 17:40:16 2010 +0100
>>> +++ b/xen/include/public/mem_event.h    Tue Jun 29 14:38:06 2010 +0100
>>> @@ -40,14 +40,14 @@
>>> 
>>> 
>>>  typedef struct mem_event_shared_page {
>>> -    int port;
>>> +    uint32_t port;
>>>  } mem_event_shared_page_t;
>>> 
>>>  typedef struct mem_event_st {
>>> -    unsigned long gfn;
>>> -    unsigned long offset;
>>> -    unsigned long p2mt;
>>> -    int vcpu_id;
>>> +    uint64_t gfn;
>>> +    uint64_t offset;
>>> +    uint32_t p2mt;
>>> +    int32_t vcpu_id;
>>>     uint64_t flags;
>>>  } mem_event_request_t, mem_event_response_t;
>>> 
>>> 
>>> 
>> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel