Re: [Xen-devel] [PATCH v5 02/12] arm/mem_access: Move PAGE_SHIFT_* macros to lib.h

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Jan,


On 07/03/2017 10:40 AM, Sergej Proskurin wrote:
> Hi Jan,
>
>
> On 06/27/2017 02:04 PM, Jan Beulich wrote:
>>>>> Sergej Proskurin <proskurin@xxxxxxxxxxxxx> 06/27/17 1:52 PM >>>
>>> The following commits introduce a software guest page table walk
>>> software implementation that supports varying guest page size
>>> granularities. This commit moves already existing PAGE_SHIFT_(4K|64K)
>>> and the new PAGE_SHIFT_16K defines to a common place in xen/lib.h as
>>> to allow the following commits to use the consolidated defines.
>> I don't think the PAGE_SHIFT_* should live far away from the other PAGE_*_*
>> macros derived from them. I'm also not convinced lib.h is a good place.
> I agree. I can move related PAGE_*_* from xen/iommu.h together with
> PAGE_SIZE_* macros into a common place. As we already define PAGE_*
> macros in asm/config.h, I believe it would make sense to extend these by
> the upper PAGE_*_* macros. What do you think?
>
> If you believe asm/config.h is a good place for the upper macros, I
> would move them and simultaneously (as a separate patch) remove the
> IOMMU_PAGE_* defines from xen/iommu.h. AFAICT they act only as helpers
> and are not used elsewhere. Instead, I could add similar helpers in
> asm/config.h. S.th. like the following:
>
> ---
>
> #define PAGE_SIZE_GRAN(sz)          (1UL << PAGE_SHIFT_##sz)
> #define PAGE_MASK_GRAN(sz)          (~(PAGE_SIZE_##sz - 1))

To prevent potential type width issues with ARMv7, I would reuse the
macro from xen/iommu.h at this point:

PAGE_MASK_GRAN(sz)                     (~(u64)0 << PAGE_SHIFT_##sz)

> #define PAGE_ALIGN_GRAN(sz, addr)   (((addr) + ~PAGE_MASK_##sz) &
> PAGE_MASK_##sz)
>
> #define PAGE_SHIFT_4K               (12)
> #define PAGE_SIZE_4K                PAGE_SIZE_GRAN(4K)
> #define PAGE_MASK_4K                PAGE_MASK_GRAN(4K)
> #define PAGE_ALIGN_4K(addr)         PAGE_ALIGN_GRAN(4K, addr)
>
> #define PAGE_SHIFT_16K              (14)
> #define PAGE_SIZE_16K               PAGE_SIZE_GRAN(16K)
> #define PAGE_MASK_16K               PAGE_MASK_GRAN(16K)
> #define PAGE_ALIGN_16K(addr)        PAGE_ALIGN_GRAN(16K, addr)
>
> #define PAGE_SHIFT_64K              (16)
> #define PAGE_SIZE_64K               PAGE_SIZE_GRAN(64K)
> #define PAGE_MASK_64K               PAGE_MASK_GRAN(64K)
> #define PAGE_ALIGN_64K(addr)        PAGE_ALIGN_GRAN(64K, addr)
>
> ---
>
> Thanks,
> ~Sergej
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel

Thanks,
~Sergej


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