Re: [Xen-devel] [PATCH v5 14/14] drivers/passthrough: arm: Add support for SMMU drivers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.05.14 at 17:50, <julien.grall@xxxxxxxxxx> wrote:
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -40,6 +40,9 @@ extern bool_t amd_iommu_perdev_intremap;
>  #define PAGE_MASK_4K        (((u64)-1) << PAGE_SHIFT_4K)
>  #define PAGE_ALIGN_4K(addr) (((addr) + PAGE_SIZE_4K - 1) & PAGE_MASK_4K)
>  
> +#define PAGE_SHIFT_64K      (16)
> +#define PAGE_SIZE_64K       (1UL << PAGE_SHIFT_64K)

I think for consistency you should also define PAGE_MASK_64K and
PAGE_ALIGN_64K, perhaps via helper macros then also to be used
to implement the 4k variants. That's not the least because you can't
safely use PAGE_SIZE_64K to implement address masking on 32-bit
ARM (which iirc supports physical addresses wider than 32 bits), due
to the use of 1UL.

E.g.

#define IOMMU_PAGE_SIZE(sz)        (1UL << PAGE_SHIFT_##sz)
#define IOMMU_PAGE_MASK(sz)        (~(u64)0 << PAGE_SHIFT_##sz)
#define IOMMU_PAGE_ALIGN(sz, addr) (((addr) + ~PAGE_MASK_##sz) & PAGE_MASK_##sz)

#define PAGE_SIZE_4K        IOMMU_PAGE_SIZE(4K)
etc.

Anyway,
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
for this small piece of non-ARM code, no matter whether you follow
the advice.

Jan


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