From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= <roger.pau@citrix.com>
Subject: Re: [PATCH v2] x86/HVM: change the flags cpu context
 field to uint64_t
Date: Mon, 18 Jan 2016 17:33:38 +0100
Message-ID: <569D13E2.7030202@citrix.com>
References: <569CFAEE.3050805@citrix.com>
	<1453129771-78737-1-git-send-email-roger.pau@citrix.com>
	<569D10F002000078000C8169@prv-mh.provo.novell.com>
	<569D0396.2060207@citrix.com> <569D070E.9070209@citrix.com>
	<569D1FD202000078000C8202@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=8183bab3b=roger.pau@citrix.com>)
	id 1aLCkR-0003c9-Ti
	for xen-devel@lists.xenproject.org; Mon, 18 Jan 2016 16:33:44 +0000
In-Reply-To: <569D1FD202000078000C8202@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Ian Campbell <ian.campbell@citrix.com>, xen-devel@lists.xenproject.org
List-Id: xen-devel@lists.xenproject.org

El 18/01/16 a les 17.24, Jan Beulich ha escrit:
>>>> On 18.01.16 at 16:38, <roger.pau@citrix.com> 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@citrix.com> 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.