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 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?

>> +#define PAGE_ALIGN_GRAN(gran, addr) (((addr) + ~PAGE_MASK_##gran) & 
>> PAGE_MASK_##gran)
>> +
>> +#define PAGE_SHIFT_4K               (12)
> Stray parentheses again.
>
> Also, with you adding a new header that'll fall under REST
> maintainership, you should have Cc-ed all the REST maintainers
> imo.

Thank you for adding the REST maintainers in Cc. In addition, I have
included Konrad Rzeszutek Wilk and Julien Grall.

Cheers,
~Sergej


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