From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v5 14/14] drivers/passthrough: arm: Add support for SMMU drivers Date: Wed, 14 May 2014 13:47:42 +0100 Message-ID: <537365EE.60500@linaro.org> References: <1399996230-18201-1-git-send-email-julien.grall@linaro.org> <1399996230-18201-15-git-send-email-julien.grall@linaro.org> <5373377B020000780001201A@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WkYb4-0001Ni-H5 for xen-devel@lists.xenproject.org; Wed, 14 May 2014 12:47:46 +0000 Received: by mail-ee0-f42.google.com with SMTP id d49so1341089eek.1 for ; Wed, 14 May 2014 05:47:45 -0700 (PDT) In-Reply-To: <5373377B020000780001201A@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org, stefano.stabellini@citrix.com, ian.campbell@citrix.com, Xiantao Zhang , tim@xen.org List-Id: xen-devel@lists.xenproject.org On 05/14/2014 08:29 AM, Jan Beulich wrote: >>>> On 13.05.14 at 17:50, 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. I will use this solution. > Anyway, > Acked-by: Jan Beulich > for this small piece of non-ARM code, no matter whether you follow > the advice. Thanks! -- Julien Grall