Re: [Xen-devel] [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t

[Roger Pau Monnà <roger.pau@xxxxxxxxxx>]

El 18/01/16 a les 17.24, Jan Beulich ha escrit:
>>>> On 18.01.16 at 16:38, <roger.pau@xxxxxxxxxx> wrote:
>> El 18/01/16 a les 16.24, Andrew Cooper ha escrit:
>>> On 18/01/16 15:21, Jan Beulich wrote:
>>>>>>> On 18.01.16 at 16:09, <roger.pau@xxxxxxxxxx> wrote:
>>>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>>>> @@ -161,8 +161,8 @@ struct hvm_hw_cpu {
>>>>>      uint32_t error_code;
>>>>>  
>>>>>  #define _XEN_X86_FPU_INITIALISED        0
>>>>> -#define XEN_X86_FPU_INITIALISED         (1U<<_XEN_X86_FPU_INITIALISED)
>>>>> -    uint32_t flags;
>>>>> +#define XEN_X86_FPU_INITIALISED         (1UL<<_XEN_X86_FPU_INITIALISED)
>>>>> +    uint64_t flags;
>>>>>  };
>>>> How is the UL going to make this safe for a 32-bit consumer?
>>>> Makes me think that, other than just said in reply to v1, it'll
>>>> indeed be better to have a separate field (with a separate
>>>> zero-check)... The (undesirable imo) alternative being to use
>>>> 1L instead.
>>>
>>> I am happy either way.  My R-b stands.
>>
>> What about using ULL or simply casting to uint64_t?
> 
> ULL might not be supported by pre-C99 compilers. Casting to
> uint64_t is, well, ugly. The flags field really has no business
> being wider then 32 bits.

Right, although Xen uses gnu99 this is a public header.

You should see a v3 somewhere with a proper check for the padding filed.
While doing this I've also realised that the padding fields in the other
structs in the same file don't seem to be checked at all.

Roger.


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