Re: [Xen-devel] [PATCH v6 02/14] arm/mem_access: Move PAGE_*_* macros to xen/page-defs.h

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.07.17 at 17:34, <proskurin@xxxxxxxxxxxxx> wrote:
> On 07/06/2017 05:24 PM, Jan Beulich wrote:
>>>>> On 06.07.17 at 16:53, <proskurin@xxxxxxxxxxxxx> wrote:
>>> On 07/06/2017 02:10 PM, Jan Beulich wrote:
>>>>>>> On 06.07.17 at 13:50, <proskurin@xxxxxxxxxxxxx> wrote:
>>>>> --- /dev/null
>>>>> +++ b/xen/include/xen/page-defs.h
>>>>> @@ -0,0 +1,24 @@
>>>>> +#ifndef __XEN_PAGE_DEFS_H__
>>>>> +#define __XEN_PAGE_DEFS_H__
>>>>> +
>>>>> +/* Helpers for different page granularities. */
>>>>> +#define PAGE_SIZE_GRAN(gran)        (1UL << PAGE_SHIFT_##gran)
>>>>> +#define PAGE_MASK_GRAN(gran)        (~(0ULL) << PAGE_SHIFT_##gran)
>>>> Stray parentheses. I'm also unhappy about the type difference
>>>> between size and mask. I guess both would best be paddr_t.
>>>> That'll then also allow mask to be defined as -size. Another
>>>> alternative would be to use 1L for size, thus guaranteeing
>>>> suitable sign extension when used in contexts requiring a width
>>>> wider than long.
>>>>
>>> Sounds reasonable. How about using 1L for PAGE_SIZE_GRAN to ensure a
>>> suitable sign extension for types wider than long and ~((paddr_t)0) for
>>> PAGE_MASK_GRAN?
>> Once again - I really think the two should be of identical type.
> 
> Alright, then I will use paddr_t in both cases. Thank you.

Well, considering what I've said earlier, "in both cases" is a little
suspicious - when you define mask in terms of size, there's not
going to be any explicit 2nd use of the intended type.

Jan


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