Re: [Xen-devel] [PATCH v2 3/3] xen/arm: fix mask calculation in pdx_init_mask

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.06.19 at 00:02, <julien.grall@xxxxxxx> wrote:
> On 6/3/19 10:56 PM, Stefano Stabellini wrote:
>> On Thu, 9 May 2019, Jan Beulich wrote:
>>>>>> On 09.05.19 at 00:47, <sstabellini@xxxxxxxxxx> wrote:
>>>> --- a/xen/common/pdx.c
>>>> +++ b/xen/common/pdx.c
>>>> @@ -50,9 +50,13 @@ static u64 __init fill_mask(u64 mask)
>>>>       return mask;
>>>>   }
>>>>   
>>>> +/*
>>>> + * We always map the first 1<<MAX_ORDER pages of RAM, hence, they
>>>> + * are left uncompressed.
>>>> + */
>>>>   u64 __init pdx_init_mask(u64 base_addr)
>>>>   {
>>>> -    return fill_mask(base_addr - 1);
>>>> +    return fill_mask(max(base_addr, (u64)1 << (MAX_ORDER + PAGE_SHIFT)) - 
>>>> 1);
>>>
>>> Personally I think that despite the surrounding u64 this should be
>>> uint64_t. You could avoid this altogether by using 1ULL.
>> 
>> I cannot use 1ULL because it would break the build: the reason is that
>> u64 is defined as unsigned long on some architectures and unsigned long
>> long on others. The pointers comparison in the max macro will fail to
>> compile.

Hmm, ugly - we indeed have UINT64_C() only in crypto code right now.

>> I could use uint64_t, that works, but I think is not a good idea to use
>> potentially different types between the arguments passed to max. If you
>> still would like me to change (u64)1 to (uint64_t)1 please explain why
>> in more details.
> 
> We are phasing out uXX in favor of uintXX_t so I agree with Jan that we 
> want to avoid introducing more here.

Is this sufficient detail for you? (Honestly I wouldn't what else to add.)

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel