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

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Jan,

On 07/06/2017 06:20 PM, Jan Beulich wrote:
>>>> 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.
> 

Apparently, I have misunderstood you before, now I see your point. I
will gladly define MASK through -SIZE and thus avoid having the type
difference at this point.

Cheers,
~Sergej

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