From mboxrd@z Thu Jan 1 00:00:00 1970 From: KyongHo Cho Subject: Re: [RFC 7/7] iommu/core: split mapping to page sizes as supported by the hardware Date: Wed, 7 Sep 2011 17:05:16 +0900 Message-ID: References: <1314984756-4400-1-git-send-email-ohad@wizery.com> <1314984756-4400-8-git-send-email-ohad@wizery.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Ohad Ben-Cohen Cc: Arnd Bergmann , Joerg Roedel , Hiroshi DOYU , linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Laurent Pinchart , David Brown , linux-omap@vger.kernel.org, David Woodhouse , linux-arm-kernel@lists.infradead.org List-Id: linux-omap@vger.kernel.org On Wed, Sep 7, 2011 at 3:01 PM, Ohad Ben-Cohen wrote: > Hi Cho, > > On Wed, Sep 7, 2011 at 4:30 AM, KyongHo Cho wrote: >> Please find the following link that I submitted for our IOMMU. >> https://lkml.org/lkml/2011/7/3/152 >> >> s5p_iommu_map/unmap accepts any order of physical address and iova >> without support of your suggestion if the order is not less than PAGE_SHIFT > > That's exactly what I'm trying to prevent; there's no reason to > duplicate the exact same logic in every iommu driver. > > As a result, your map function is quite big (it even duplicates the > same logic for every page size it tries). > I agree that my map function is big :) However, it is faster because it does not calculate the given order and not call any function. > Try to rebase your patch on the "iommu/core: split mapping to page > sizes as supported by the hardware" patch I've just sent, and see how > significantly simpler your code becomes. > > Relying on a single implementation, provided by the IOMMU core, means > less code to maintain (and debug) and a consistent behavior (across > all supported hardware) exposed to users of the IOMMU API. > >> But I think IOMMU API must not expect that the caller of iommu_map() knows >> about the restriction of IOMMU API implementation. > > Right. But I don't think there's any disagreement here ? > Yes, sorry:) > If any, then I think that s5p_iommu_map() is more limited than what I > propose: if it is given a 64KB region aligned only to a 4KB address, > it will BUG_ON() it. While not ideal, I don't think there's any reason > not to map it using 4KB pages (which is exactly what the new > iommu_map() I propose will do). > It is a logical error because the caller of iommu_map() gives 4KB aligned physical address while saying that it is 64KB aligned. I think it must not be happened . > Thanks, > Ohad. Thank you. KyongHo. From mboxrd@z Thu Jan 1 00:00:00 1970 From: pullip.cho@samsung.com (KyongHo Cho) Date: Wed, 7 Sep 2011 17:05:16 +0900 Subject: [RFC 7/7] iommu/core: split mapping to page sizes as supported by the hardware In-Reply-To: References: <1314984756-4400-1-git-send-email-ohad@wizery.com> <1314984756-4400-8-git-send-email-ohad@wizery.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Sep 7, 2011 at 3:01 PM, Ohad Ben-Cohen wrote: > Hi Cho, > > On Wed, Sep 7, 2011 at 4:30 AM, KyongHo Cho wrote: >> Please find the following link that I submitted for our IOMMU. >> https://lkml.org/lkml/2011/7/3/152 >> >> s5p_iommu_map/unmap accepts any order of physical address and iova >> without support of your suggestion if the order is not less than PAGE_SHIFT > > That's exactly what I'm trying to prevent; there's no reason to > duplicate the exact same logic in every iommu driver. > > As a result, your map function is quite big (it even duplicates the > same logic for every page size it tries). > I agree that my map function is big :) However, it is faster because it does not calculate the given order and not call any function. > Try to rebase your patch on the "iommu/core: split mapping to page > sizes as supported by the hardware" patch I've just sent, and see how > significantly simpler your code becomes. > > Relying on a single implementation, provided by the IOMMU core, means > less code to maintain (and debug) and a consistent behavior (across > all supported hardware) exposed to users of the IOMMU API. > >> But I think IOMMU API must not expect that the caller of iommu_map() knows >> about the restriction of IOMMU API implementation. > > Right. But I don't think there's any disagreement here ? > Yes, sorry:) > If any, then I think that s5p_iommu_map() is more limited than what I > propose: if it is given a 64KB region aligned only to a 4KB address, > it will BUG_ON() it. While not ideal, I don't think there's any reason > not to map it using 4KB pages (which is exactly what the new > iommu_map() I propose will do). > It is a logical error because the caller of iommu_map() gives 4KB aligned physical address while saying that it is 64KB aligned. I think it must not be happened . > Thanks, > Ohad. Thank you. KyongHo.