| Message ID | 4D74D94F.7040702@matrix-vision.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers |
Return-path: <mchehab@pedra> Envelope-to: mchehab@pedra Delivery-date: Mon, 07 Mar 2011 10:12:20 -0300 Received: from mchehab by pedra with local (Exim 4.72) (envelope-from <mchehab@pedra>) id 1PwaEW-0005zC-2K for mchehab@pedra; Mon, 07 Mar 2011 10:12:20 -0300 Received: from casper.infradead.org [85.118.1.10] by pedra with IMAP (fetchmail-6.3.17) for <mchehab@localhost> (single-drop); Mon, 07 Mar 2011 10:12:19 -0300 (BRT) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1PwaDJ-0005fj-8T; Mon, 07 Mar 2011 13:11:05 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754502Ab1CGNKm (ORCPT <rfc822; kmpark@infradead.org> + 1 other); Mon, 7 Mar 2011 08:10:42 -0500 Received: from mail1.matrix-vision.com ([78.47.19.71]:33106 "EHLO mail1.matrix-vision.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754415Ab1CGNKl (ORCPT <rfc822; linux-media@vger.kernel.org>); Mon, 7 Mar 2011 08:10:41 -0500 Received: from mail1.matrix-vision.com (localhost [127.0.0.1]) by mail1.matrix-vision.com (Postfix) with ESMTP id 52D6172399; Mon, 7 Mar 2011 14:10:40 +0100 (CET) Received: from erinome (g2.matrix-vision.com [80.152.136.245]) by mail1.matrix-vision.com (Postfix) with ESMTPA id 17BC77221C; Mon, 7 Mar 2011 14:10:40 +0100 (CET) Received: from erinome (localhost [127.0.0.1]) by erinome (Postfix) with ESMTP id 725216F8A; Mon, 7 Mar 2011 14:10:39 +0100 (CET) Received: by erinome (Postfix, from userid 108) id 639956F9C; Mon, 7 Mar 2011 14:10:39 +0100 (CET) Received: from [192.168.65.46] (host65-46.intern.matrix-vision.de [192.168.65.46]) by erinome (Postfix) with ESMTPA id 4EE0B6F8A; Mon, 7 Mar 2011 14:10:39 +0100 (CET) Message-ID: <4D74D94F.7040702@matrix-vision.de> Date: Mon, 07 Mar 2011 14:10:39 +0100 From: Michael Jones <michael.jones@matrix-vision.de> User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12) Gecko/20101029 Lightning/1.0b2 Thunderbird/3.1.6 MIME-Version: 1.0 To: David Cohen <dacohen@gmail.com> CC: Hiroshi.DOYU@nokia.com, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, fernando.lugo@ti.com, Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>, Linux Media Mailing List <linux-media@vger.kernel.org>, linux-omap@vger.kernel.org Subject: [PATCH] omap: iommu: disallow mapping NULL address References: <4D6D219D.7020605@matrix-vision.de> <201103022018.23446.laurent.pinchart@ideasonboard.com> <4D6FBC7F.1080500@matrix-vision.de> <AANLkTikAKy=CzTqEv-UGBQ1EavqmCStPNFZ5vs7vH5VK@mail.gmail.com> <4D70F985.8030902@matrix-vision.de> <AANLkTinSJpjPXWHWduLbRSmb=La3sv82ufwgsq-uR7S2@mail.gmail.com> <AANLkTi=8Sss-5xfgPmgx=J_T__=hrC1rQU-xBOdKC8Ve@mail.gmail.com> In-Reply-To: <AANLkTi=8Sss-5xfgPmgx=J_T__=hrC1rQU-xBOdKC8Ve@mail.gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-MV-Disclaimer: true (erinome) X-AV-Checked: ClamAV using ClamSMTP (erinome) X-AV-Checked: ClamAV using ClamSMTP (mail1) Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org Sender: <mchehab@pedra> |
Commit Message
Michael Jones
March 7, 2011, 1:10 p.m. UTC
From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 From: Michael Jones <michael.jones@matrix-vision.de> Date: Mon, 7 Mar 2011 13:36:15 +0100 Subject: [PATCH] omap: iommu: disallow mapping NULL address commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping the NULL address if da_start==0. Force da_start to exclude the first page. Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> --- arch/arm/plat-omap/iommu.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
Comments
On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones <michael.jones@matrix-vision.de> wrote: > From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 > From: Michael Jones <michael.jones@matrix-vision.de> > Date: Mon, 7 Mar 2011 13:36:15 +0100 > Subject: [PATCH] omap: iommu: disallow mapping NULL address > > commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping > the NULL address if da_start==0. Force da_start to exclude the > first page. what about devices that uses page 0? ipu after reset always starts from 0x00000000 how could we map that address?? Regards, Fernando. > > Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> > --- > arch/arm/plat-omap/iommu.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c > index 5990ea6..dcb5513 100644 > --- a/arch/arm/plat-omap/iommu.c > +++ b/arch/arm/plat-omap/iommu.c > @@ -850,7 +850,7 @@ int iommu_set_da_range(struct iommu *obj, u32 start, u32 end) > if (end < start || !PAGE_ALIGN(start | end)) > return -EINVAL; > > - obj->da_start = start; > + obj->da_start = max(start, (u32)PAGE_SIZE); > obj->da_end = end; > > return 0; > @@ -950,7 +950,9 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev) > obj->name = pdata->name; > obj->dev = &pdev->dev; > obj->ctx = (void *)obj + sizeof(*obj); > - obj->da_start = pdata->da_start; > + > + /* reserve the first page for NULL */ > + obj->da_start = max(pdata->da_start, (u32)PAGE_SIZE); > obj->da_end = pdata->da_end; > > mutex_init(&obj->iommu_lock); > -- > 1.7.4.1 > > > MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler > Registergericht: Amtsgericht Stuttgart, HRB 271090 > Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando <fernando.lugo@ti.com> wrote: > On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones > <michael.jones@matrix-vision.de> wrote: >> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >> From: Michael Jones <michael.jones@matrix-vision.de> >> Date: Mon, 7 Mar 2011 13:36:15 +0100 >> Subject: [PATCH] omap: iommu: disallow mapping NULL address >> >> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >> the NULL address if da_start==0. Force da_start to exclude the >> first page. > > what about devices that uses page 0? ipu after reset always starts > from 0x00000000 how could we map that address?? from 0x0? The driver sees da == 0 as error. May I ask you why do you want it? Br, David > > Regards, > Fernando. > >> >> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> >> --- >> arch/arm/plat-omap/iommu.c | 6 ++++-- >> 1 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c >> index 5990ea6..dcb5513 100644 >> --- a/arch/arm/plat-omap/iommu.c >> +++ b/arch/arm/plat-omap/iommu.c >> @@ -850,7 +850,7 @@ int iommu_set_da_range(struct iommu *obj, u32 start, u32 end) >> if (end < start || !PAGE_ALIGN(start | end)) >> return -EINVAL; >> >> - obj->da_start = start; >> + obj->da_start = max(start, (u32)PAGE_SIZE); >> obj->da_end = end; >> >> return 0; >> @@ -950,7 +950,9 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev) >> obj->name = pdata->name; >> obj->dev = &pdev->dev; >> obj->ctx = (void *)obj + sizeof(*obj); >> - obj->da_start = pdata->da_start; >> + >> + /* reserve the first page for NULL */ >> + obj->da_start = max(pdata->da_start, (u32)PAGE_SIZE); >> obj->da_end = pdata->da_end; >> >> mutex_init(&obj->iommu_lock); >> -- >> 1.7.4.1 >> >> >> MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler >> Registergericht: Amtsgericht Stuttgart, HRB 271090 >> Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner >> > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 7, 2011 at 1:19 PM, David Cohen <dacohen@gmail.com> wrote: > On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando > <fernando.lugo@ti.com> wrote: >> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones >> <michael.jones@matrix-vision.de> wrote: >>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >>> From: Michael Jones <michael.jones@matrix-vision.de> >>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>> >>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>> the NULL address if da_start==0. Force da_start to exclude the >>> first page. >> >> what about devices that uses page 0? ipu after reset always starts >> from 0x00000000 how could we map that address?? > > from 0x0? The driver sees da == 0 as error. May I ask you why do you want it? unlike DSP that you can load a register with the addres the DSP will boot, IPU core always starts from address 0x00000000, so if you take IPU out of reset it will try to access address 0x0 if not map it, there will be a mmu fault. Regards, Fernando. > > Br, > > David > >> >> Regards, >> Fernando. >> >>> >>> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> >>> --- >>> arch/arm/plat-omap/iommu.c | 6 ++++-- >>> 1 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c >>> index 5990ea6..dcb5513 100644 >>> --- a/arch/arm/plat-omap/iommu.c >>> +++ b/arch/arm/plat-omap/iommu.c >>> @@ -850,7 +850,7 @@ int iommu_set_da_range(struct iommu *obj, u32 start, u32 end) >>> if (end < start || !PAGE_ALIGN(start | end)) >>> return -EINVAL; >>> >>> - obj->da_start = start; >>> + obj->da_start = max(start, (u32)PAGE_SIZE); >>> obj->da_end = end; >>> >>> return 0; >>> @@ -950,7 +950,9 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev) >>> obj->name = pdata->name; >>> obj->dev = &pdev->dev; >>> obj->ctx = (void *)obj + sizeof(*obj); >>> - obj->da_start = pdata->da_start; >>> + >>> + /* reserve the first page for NULL */ >>> + obj->da_start = max(pdata->da_start, (u32)PAGE_SIZE); >>> obj->da_end = pdata->da_end; >>> >>> mutex_init(&obj->iommu_lock); >>> -- >>> 1.7.4.1 >>> >>> >>> MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler >>> Registergericht: Amtsgericht Stuttgart, HRB 271090 >>> Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando <fernando.lugo@ti.com> wrote: > On Mon, Mar 7, 2011 at 1:19 PM, David Cohen <dacohen@gmail.com> wrote: >> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando >> <fernando.lugo@ti.com> wrote: >>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones >>> <michael.jones@matrix-vision.de> wrote: >>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >>>> From: Michael Jones <michael.jones@matrix-vision.de> >>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>>> >>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>>> the NULL address if da_start==0. Force da_start to exclude the >>>> first page. >>> >>> what about devices that uses page 0? ipu after reset always starts >>> from 0x00000000 how could we map that address?? >> >> from 0x0? The driver sees da == 0 as error. May I ask you why do you want it? > > unlike DSP that you can load a register with the addres the DSP will > boot, IPU core always starts from address 0x00000000, so if you take > IPU out of reset it will try to access address 0x0 if not map it, > there will be a mmu fault. Hm. Looks like the iommu should not restrict any da. The valid da range should rely only on pdata. Michael, what about just update ISP's da_start on omap-iommu.c file? Set it to 0x1000. Hiroshi, any opinion? Br, David > > Regards, > Fernando. > >> >> Br, >> >> David >> >>> >>> Regards, >>> Fernando. >>> >>>> >>>> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> >>>> --- >>>> arch/arm/plat-omap/iommu.c | 6 ++++-- >>>> 1 files changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c >>>> index 5990ea6..dcb5513 100644 >>>> --- a/arch/arm/plat-omap/iommu.c >>>> +++ b/arch/arm/plat-omap/iommu.c >>>> @@ -850,7 +850,7 @@ int iommu_set_da_range(struct iommu *obj, u32 start, u32 end) >>>> if (end < start || !PAGE_ALIGN(start | end)) >>>> return -EINVAL; >>>> >>>> - obj->da_start = start; >>>> + obj->da_start = max(start, (u32)PAGE_SIZE); >>>> obj->da_end = end; >>>> >>>> return 0; >>>> @@ -950,7 +950,9 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev) >>>> obj->name = pdata->name; >>>> obj->dev = &pdev->dev; >>>> obj->ctx = (void *)obj + sizeof(*obj); >>>> - obj->da_start = pdata->da_start; >>>> + >>>> + /* reserve the first page for NULL */ >>>> + obj->da_start = max(pdata->da_start, (u32)PAGE_SIZE); >>>> obj->da_end = pdata->da_end; >>>> >>>> mutex_init(&obj->iommu_lock); >>>> -- >>>> 1.7.4.1 >>>> >>>> >>>> MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler >>>> Registergericht: Amtsgericht Stuttgart, HRB 271090 >>>> Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner >>>> >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David, On Monday 07 March 2011 20:41:21 David Cohen wrote: > On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando wrote: > > On Mon, Mar 7, 2011 at 1:19 PM, David Cohen wrote: > >> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando wrote: > >>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones wrote: > >>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 > >>>> From: Michael Jones <michael.jones@matrix-vision.de> > >>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 > >>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address > >>>> > >>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping > >>>> the NULL address if da_start==0. Force da_start to exclude the > >>>> first page. > >>> > >>> what about devices that uses page 0? ipu after reset always starts > >>> from 0x00000000 how could we map that address?? > >> > >> from 0x0? The driver sees da == 0 as error. May I ask you why do you > >> want it? > > > > unlike DSP that you can load a register with the addres the DSP will > > boot, IPU core always starts from address 0x00000000, so if you take > > IPU out of reset it will try to access address 0x0 if not map it, > > there will be a mmu fault. > > Hm. Looks like the iommu should not restrict any da. The valid da > range should rely only on pdata. > Michael, what about just update ISP's da_start on omap-iommu.c file? > Set it to 0x1000. What about patching the OMAP3 ISP driver to use a non-zero value (maybe -1) as an invalid/freed pointer ?
On Mon, Mar 7, 2011 at 11:19 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi David, Hi Laurent, > > On Monday 07 March 2011 20:41:21 David Cohen wrote: >> On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando wrote: >> > On Mon, Mar 7, 2011 at 1:19 PM, David Cohen wrote: >> >> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando wrote: >> >>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones wrote: >> >>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >> >>>> From: Michael Jones <michael.jones@matrix-vision.de> >> >>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >> >>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >> >>>> >> >>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >> >>>> the NULL address if da_start==0. Force da_start to exclude the >> >>>> first page. >> >>> >> >>> what about devices that uses page 0? ipu after reset always starts >> >>> from 0x00000000 how could we map that address?? >> >> >> >> from 0x0? The driver sees da == 0 as error. May I ask you why do you >> >> want it? >> > >> > unlike DSP that you can load a register with the addres the DSP will >> > boot, IPU core always starts from address 0x00000000, so if you take >> > IPU out of reset it will try to access address 0x0 if not map it, >> > there will be a mmu fault. >> >> Hm. Looks like the iommu should not restrict any da. The valid da >> range should rely only on pdata. >> Michael, what about just update ISP's da_start on omap-iommu.c file? >> Set it to 0x1000. > > What about patching the OMAP3 ISP driver to use a non-zero value (maybe -1) as > an invalid/freed pointer ? I wouldn't be comfortable to use 0 (or NULL) value as valid address on ISP driver. The 'da' range (da_start and da_end) is defined per VM and specified as platform data. IMO, to set da_start = 0x1000 seems to be a correct approach for ISP as it's the only client for its IOMMU instance. Regards, David > > -- > Regards, > > Laurent Pinchart > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: ext David Cohen <dacohen@gmail.com> Subject: Re: [PATCH] omap: iommu: disallow mapping NULL address Date: Mon, 7 Mar 2011 21:41:21 +0200 > On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando > <fernando.lugo@ti.com> wrote: >> On Mon, Mar 7, 2011 at 1:19 PM, David Cohen <dacohen@gmail.com> wrote: >>> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando >>> <fernando.lugo@ti.com> wrote: >>>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones >>>> <michael.jones@matrix-vision.de> wrote: >>>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >>>>> From: Michael Jones <michael.jones@matrix-vision.de> >>>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>>>> >>>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>>>> the NULL address if da_start==0. Force da_start to exclude the >>>>> first page. >>>> >>>> what about devices that uses page 0? ipu after reset always starts >>>> from 0x00000000 how could we map that address?? >>> >>> from 0x0? The driver sees da == 0 as error. May I ask you why do you want it? >> >> unlike DSP that you can load a register with the addres the DSP will >> boot, IPU core always starts from address 0x00000000, so if you take >> IPU out of reset it will try to access address 0x0 if not map it, >> there will be a mmu fault. > > Hm. Looks like the iommu should not restrict any da. The valid da > range should rely only on pdata. > Michael, what about just update ISP's da_start on omap-iommu.c file? > Set it to 0x1000. > > Hiroshi, any opinion? We have assumed that 'da == 0' is NULL so far. According to Fernando's explanation, 'da == 0' should be allowed in iovmm layer by default. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: ext David Cohen <dacohen@gmail.com> Subject: Re: [PATCH] omap: iommu: disallow mapping NULL address Date: Mon, 7 Mar 2011 23:35:31 +0200 > On Mon, Mar 7, 2011 at 11:19 PM, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: >> Hi David, > > Hi Laurent, > >> >> On Monday 07 March 2011 20:41:21 David Cohen wrote: >>> On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando wrote: >>> > On Mon, Mar 7, 2011 at 1:19 PM, David Cohen wrote: >>> >> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando wrote: >>> >>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones wrote: >>> >>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >>> >>>> From: Michael Jones <michael.jones@matrix-vision.de> >>> >>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>> >>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>> >>>> >>> >>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>> >>>> the NULL address if da_start==0. Force da_start to exclude the >>> >>>> first page. >>> >>> >>> >>> what about devices that uses page 0? ipu after reset always starts >>> >>> from 0x00000000 how could we map that address?? >>> >> >>> >> from 0x0? The driver sees da == 0 as error. May I ask you why do you >>> >> want it? >>> > >>> > unlike DSP that you can load a register with the addres the DSP will >>> > boot, IPU core always starts from address 0x00000000, so if you take >>> > IPU out of reset it will try to access address 0x0 if not map it, >>> > there will be a mmu fault. >>> >>> Hm. Looks like the iommu should not restrict any da. The valid da >>> range should rely only on pdata. >>> Michael, what about just update ISP's da_start on omap-iommu.c file? >>> Set it to 0x1000. >> >> What about patching the OMAP3 ISP driver to use a non-zero value (maybe -1) as >> an invalid/freed pointer ? > > I wouldn't be comfortable to use 0 (or NULL) value as valid address on > ISP driver. The 'da' range (da_start and da_end) is defined per VM and > specified as platform data. IMO, to set da_start = 0x1000 seems to be > a correct approach for ISP as it's the only client for its IOMMU > instance. Sounds reasonable to me too. Considering 'da == 0' as invalid can be reasonably acceptable intuitively in most cases, and just let it allowed theoretically. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Guzman Lugo, Fernando wrote: > On Mon, Mar 7, 2011 at 1:19 PM, David Cohen <dacohen@gmail.com> wrote: >> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando >> <fernando.lugo@ti.com> wrote: >>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones >>> <michael.jones@matrix-vision.de> wrote: >>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >>>> From: Michael Jones <michael.jones@matrix-vision.de> >>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>>> >>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>>> the NULL address if da_start==0. Force da_start to exclude the >>>> first page. >>> >>> what about devices that uses page 0? ipu after reset always starts >>> from 0x00000000 how could we map that address?? >> >> from 0x0? The driver sees da == 0 as error. May I ask you why do you want it? > > unlike DSP that you can load a register with the addres the DSP will > boot, IPU core always starts from address 0x00000000, so if you take > IPU out of reset it will try to access address 0x0 if not map it, > there will be a mmu fault. I think the driver for IPU (what is it, btw.?) must map the NULL address explicitly. It cannot rely on automatic allocation of the NULL address by the iommu even if it was the first allocation.
Hi Fernando, On Tue, Mar 8, 2011 at 11:13 AM, Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> wrote: > Guzman Lugo, Fernando wrote: >> On Mon, Mar 7, 2011 at 1:19 PM, David Cohen <dacohen@gmail.com> wrote: >>> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando >>> <fernando.lugo@ti.com> wrote: >>>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones >>>> <michael.jones@matrix-vision.de> wrote: >>>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >>>>> From: Michael Jones <michael.jones@matrix-vision.de> >>>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>>>> >>>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>>>> the NULL address if da_start==0. Force da_start to exclude the >>>>> first page. >>>> >>>> what about devices that uses page 0? ipu after reset always starts >>>> from 0x00000000 how could we map that address?? >>> >>> from 0x0? The driver sees da == 0 as error. May I ask you why do you want it? >> >> unlike DSP that you can load a register with the addres the DSP will >> boot, IPU core always starts from address 0x00000000, so if you take >> IPU out of reset it will try to access address 0x0 if not map it, >> there will be a mmu fault. > > I think the driver for IPU (what is it, btw.?) must map the NULL address > explicitly. It cannot rely on automatic allocation of the NULL address > by the iommu even if it was the first allocation. That's an interesting question. My first thought was "it's not automatic allocation", because it seems you know the specific 'da' IPU needs. But then, looking into the driver's API, the automatic allocation is defined whether the argument da == 0 (automatic allocation) or da != 0 (fixed da). So, by default, the IOMMU driver does not see da == 0 as valid address for fixed da. Then, why only automatic allocation should use such address? My second point is: if you're using automatic allocation, you *cannot* rely on specific da to be used, as it would be up to IOMMU driver to choose. So, doesn't matter the option, your driver seems to be wrong, unless I'm missing something. If you were using fixed da passing da = 0, you were just being lucky that it was the first request and automatic allocation returned da == 0. IMO either first page is not allowed at all or OMAP's IOMMU API should change the way it checks if it's fixed da or not. Kind regards, David > > -- > Sakari Ailus > sakari.ailus@maxwell.research.nokia.com > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 8, 2011 at 3:13 AM, Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> wrote: > Guzman Lugo, Fernando wrote: >> On Mon, Mar 7, 2011 at 1:19 PM, David Cohen <dacohen@gmail.com> wrote: >>> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando >>> <fernando.lugo@ti.com> wrote: >>>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones >>>> <michael.jones@matrix-vision.de> wrote: >>>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >>>>> From: Michael Jones <michael.jones@matrix-vision.de> >>>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>>>> >>>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>>>> the NULL address if da_start==0. Force da_start to exclude the >>>>> first page. >>>> >>>> what about devices that uses page 0? ipu after reset always starts >>>> from 0x00000000 how could we map that address?? >>> >>> from 0x0? The driver sees da == 0 as error. May I ask you why do you want it? >> >> unlike DSP that you can load a register with the addres the DSP will >> boot, IPU core always starts from address 0x00000000, so if you take >> IPU out of reset it will try to access address 0x0 if not map it, >> there will be a mmu fault. > > I think the driver for IPU (what is it, btw.?) must map the NULL address > explicitly. It cannot rely on automatic allocation of the NULL address > by the iommu even if it was the first allocation. IPU = imaging processor unit (Cortex-M3 on omap4). yeah, we should rely on that, so we will need to pass IOVMF_DA_FIXED flag, what ideally always be success because it is the first map after getting iommu handle. In this moment it is mapped direcctly using iommu.c and other layer upon that, but it would be nice be able to use iovmm in the future. Regards, Fernando. > > -- > Sakari Ailus > sakari.ailus@maxwell.research.nokia.com > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 8, 2011 at 3:55 AM, David Cohen <dacohen@gmail.com> wrote: > Hi Fernando, > > On Tue, Mar 8, 2011 at 11:13 AM, Sakari Ailus > <sakari.ailus@maxwell.research.nokia.com> wrote: >> Guzman Lugo, Fernando wrote: >>> On Mon, Mar 7, 2011 at 1:19 PM, David Cohen <dacohen@gmail.com> wrote: >>>> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando >>>> <fernando.lugo@ti.com> wrote: >>>>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones >>>>> <michael.jones@matrix-vision.de> wrote: >>>>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 >>>>>> From: Michael Jones <michael.jones@matrix-vision.de> >>>>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>>>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>>>>> >>>>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>>>>> the NULL address if da_start==0. Force da_start to exclude the >>>>>> first page. >>>>> >>>>> what about devices that uses page 0? ipu after reset always starts >>>>> from 0x00000000 how could we map that address?? >>>> >>>> from 0x0? The driver sees da == 0 as error. May I ask you why do you want it? >>> >>> unlike DSP that you can load a register with the addres the DSP will >>> boot, IPU core always starts from address 0x00000000, so if you take >>> IPU out of reset it will try to access address 0x0 if not map it, >>> there will be a mmu fault. >> >> I think the driver for IPU (what is it, btw.?) must map the NULL address >> explicitly. It cannot rely on automatic allocation of the NULL address >> by the iommu even if it was the first allocation. > > That's an interesting question. My first thought was "it's not > automatic allocation", because it seems you know the specific 'da' IPU > needs. But then, looking into the driver's API, the automatic > allocation is defined whether the argument da == 0 (automatic > allocation) or da != 0 (fixed da). So, by default, the IOMMU driver > does not see da == 0 as valid address for fixed da. Then, why only > automatic allocation should use such address? My second point is: if > you're using automatic allocation, you *cannot* rely on specific da to > be used, as it would be up to IOMMU driver to choose. So, doesn't > matter the option, your driver seems to be wrong, unless I'm missing > something. If you were using fixed da passing da = 0, you were just > being lucky that it was the first request and automatic allocation > returned da == 0. yes, the driver is wrong, it should use only flag IOVMF_DA_ANON to get an automatic address. it has to be changed too. Regards, Fernando. > IMO either first page is not allowed at all or OMAP's IOMMU API should > change the way it checks if it's fixed da or not. > > Kind regards, > > David > >> >> -- >> Sakari Ailus >> sakari.ailus@maxwell.research.nokia.com >> > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David, On Monday 07 March 2011 22:35:31 David Cohen wrote: > On Mon, Mar 7, 2011 at 11:19 PM, Laurent Pinchart wrote: > > On Monday 07 March 2011 20:41:21 David Cohen wrote: > >> On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando wrote: > >> > On Mon, Mar 7, 2011 at 1:19 PM, David Cohen wrote: > >> >> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando wrote: > >> >>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones wrote: > >> >>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 > >> >>>> 2001 From: Michael Jones <michael.jones@matrix-vision.de> > >> >>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 > >> >>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address > >> >>>> > >> >>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping > >> >>>> the NULL address if da_start==0. Force da_start to exclude the > >> >>>> first page. > >> >>> > >> >>> what about devices that uses page 0? ipu after reset always starts > >> >>> from 0x00000000 how could we map that address?? > >> >> > >> >> from 0x0? The driver sees da == 0 as error. May I ask you why do you > >> >> want it? > >> > > >> > unlike DSP that you can load a register with the addres the DSP will > >> > boot, IPU core always starts from address 0x00000000, so if you take > >> > IPU out of reset it will try to access address 0x0 if not map it, > >> > there will be a mmu fault. > >> > >> Hm. Looks like the iommu should not restrict any da. The valid da > >> range should rely only on pdata. > >> Michael, what about just update ISP's da_start on omap-iommu.c file? > >> Set it to 0x1000. > > > > What about patching the OMAP3 ISP driver to use a non-zero value (maybe > > -1) as an invalid/freed pointer ? > > I wouldn't be comfortable to use 0 (or NULL) value as valid address on > ISP driver. Why not ? The IOMMUs can use 0x00000000 as a valid address. Whether we allow it or not is a software architecture decision, not influenced by the IOMMU hardware. As some peripherals (namely IPU) require mapping memory to 0x00000000, the IOMMU layer must support it and not treat 0x00000000 specially. All da == 0 checks to aim at catching invalid address values must be removed, both from the IOMMU API and the IOMMU internals. > The 'da' range (da_start and da_end) is defined per VM and specified as > platform data. IMO, to set da_start = 0x1000 seems to be> a correct approach > for ISP as it's the only client for its IOMMU instance. We can do that, and then use 0 as an invalid pointer in the ISP driver. As the IOMMU API will use another value (what about 0xffffffff, as for the userspace mmap() call ?) to mean "invalid pointer", it might be better to use the same value in the ISP driver.
On Tue, Mar 8, 2011 at 2:31 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi David, > > On Monday 07 March 2011 22:35:31 David Cohen wrote: >> On Mon, Mar 7, 2011 at 11:19 PM, Laurent Pinchart wrote: >> > On Monday 07 March 2011 20:41:21 David Cohen wrote: >> >> On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando wrote: >> >> > On Mon, Mar 7, 2011 at 1:19 PM, David Cohen wrote: >> >> >> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando wrote: >> >> >>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones wrote: >> >> >>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 >> >> >>>> 2001 From: Michael Jones <michael.jones@matrix-vision.de> >> >> >>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >> >> >>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >> >> >>>> >> >> >>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >> >> >>>> the NULL address if da_start==0. Force da_start to exclude the >> >> >>>> first page. >> >> >>> >> >> >>> what about devices that uses page 0? ipu after reset always starts >> >> >>> from 0x00000000 how could we map that address?? >> >> >> >> >> >> from 0x0? The driver sees da == 0 as error. May I ask you why do you >> >> >> want it? >> >> > >> >> > unlike DSP that you can load a register with the addres the DSP will >> >> > boot, IPU core always starts from address 0x00000000, so if you take >> >> > IPU out of reset it will try to access address 0x0 if not map it, >> >> > there will be a mmu fault. >> >> >> >> Hm. Looks like the iommu should not restrict any da. The valid da >> >> range should rely only on pdata. >> >> Michael, what about just update ISP's da_start on omap-iommu.c file? >> >> Set it to 0x1000. >> > >> > What about patching the OMAP3 ISP driver to use a non-zero value (maybe >> > -1) as an invalid/freed pointer ? >> >> I wouldn't be comfortable to use 0 (or NULL) value as valid address on >> ISP driver. > > Why not ? The IOMMUs can use 0x00000000 as a valid address. Whether we allow > it or not is a software architecture decision, not influenced by the IOMMU > hardware. As some peripherals (namely IPU) require mapping memory to > 0x00000000, the IOMMU layer must support it and not treat 0x00000000 > specially. All da == 0 checks to aim at catching invalid address values must > be removed, both from the IOMMU API and the IOMMU internals. Yes, I completely agree with this approach. Regards, Fernando. > >> The 'da' range (da_start and da_end) is defined per VM and specified as >> platform data. IMO, to set da_start = 0x1000 seems to be> a correct approach >> for ISP as it's the only client for its IOMMU instance. > > We can do that, and then use 0 as an invalid pointer in the ISP driver. As the > IOMMU API will use another value (what about 0xffffffff, as for the userspace > mmap() call ?) to mean "invalid pointer", it might be better to use the same > value in the ISP driver. > > -- > Regards, > > Laurent Pinchart > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 8, 2011 at 10:31 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi David, > > On Monday 07 March 2011 22:35:31 David Cohen wrote: >> On Mon, Mar 7, 2011 at 11:19 PM, Laurent Pinchart wrote: >> > On Monday 07 March 2011 20:41:21 David Cohen wrote: >> >> On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando wrote: >> >> > On Mon, Mar 7, 2011 at 1:19 PM, David Cohen wrote: >> >> >> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando wrote: >> >> >>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones wrote: >> >> >>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 >> >> >>>> 2001 From: Michael Jones <michael.jones@matrix-vision.de> >> >> >>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >> >> >>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >> >> >>>> >> >> >>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >> >> >>>> the NULL address if da_start==0. Force da_start to exclude the >> >> >>>> first page. >> >> >>> >> >> >>> what about devices that uses page 0? ipu after reset always starts >> >> >>> from 0x00000000 how could we map that address?? >> >> >> >> >> >> from 0x0? The driver sees da == 0 as error. May I ask you why do you >> >> >> want it? >> >> > >> >> > unlike DSP that you can load a register with the addres the DSP will >> >> > boot, IPU core always starts from address 0x00000000, so if you take >> >> > IPU out of reset it will try to access address 0x0 if not map it, >> >> > there will be a mmu fault. >> >> >> >> Hm. Looks like the iommu should not restrict any da. The valid da >> >> range should rely only on pdata. >> >> Michael, what about just update ISP's da_start on omap-iommu.c file? >> >> Set it to 0x1000. >> > >> > What about patching the OMAP3 ISP driver to use a non-zero value (maybe >> > -1) as an invalid/freed pointer ? >> >> I wouldn't be comfortable to use 0 (or NULL) value as valid address on >> ISP driver. > > Why not ? The IOMMUs can use 0x00000000 as a valid address. Whether we allow > it or not is a software architecture decision, not influenced by the IOMMU > hardware. As some peripherals (namely IPU) require mapping memory to > 0x00000000, the IOMMU layer must support it and not treat 0x00000000 > specially. All da == 0 checks to aim at catching invalid address values must > be removed, both from the IOMMU API and the IOMMU internals. Yes, it can use and IOMMU should not treat is specially. That's the aim of my patch: [PATCH v2 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag I'm not advocating to not allow 0x0, but to not use it when user is not requesting fixed da. In many sw architecture decisions 0x0 address is a special case. To avoid any misuse, IOMMU will not use it unless it's requested. If user is not requesting fixed 'da', it's not a problem to not give 0x0 anyway. IMO that's the safer option for all cases. > >> The 'da' range (da_start and da_end) is defined per VM and specified as >> platform data. IMO, to set da_start = 0x1000 seems to be> a correct approach >> for ISP as it's the only client for its IOMMU instance. > > We can do that, and then use 0 as an invalid pointer in the ISP driver. As the > IOMMU API will use another value (what about 0xffffffff, as for the userspace > mmap() call ?) to mean "invalid pointer", it might be better to use the same > value in the ISP driver. That can be done, of course. But the main point is in OMAP3 ISP all initial register values to read/write from/to memory are 0x0. It means sometimes we can catch bugs more easily by not mapping that address. So, IMO, OMAP3 ISP should not allow to map first page. But that's a special case for this driver only. Br, David > > -- > Regards, > > Laurent Pinchart > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Cohen wrote: > On Tue, Mar 8, 2011 at 10:31 PM, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: >> Hi David, >> >> On Monday 07 March 2011 22:35:31 David Cohen wrote: >>> On Mon, Mar 7, 2011 at 11:19 PM, Laurent Pinchart wrote: >>>> On Monday 07 March 2011 20:41:21 David Cohen wrote: >>>>> On Mon, Mar 7, 2011 at 9:25 PM, Guzman Lugo, Fernando wrote: >>>>>> On Mon, Mar 7, 2011 at 1:19 PM, David Cohen wrote: >>>>>>> On Mon, Mar 7, 2011 at 9:17 PM, Guzman Lugo, Fernando wrote: >>>>>>>> On Mon, Mar 7, 2011 at 7:10 AM, Michael Jones wrote: >>>>>>>>> From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 >>>>>>>>> 2001 From: Michael Jones <michael.jones@matrix-vision.de> >>>>>>>>> Date: Mon, 7 Mar 2011 13:36:15 +0100 >>>>>>>>> Subject: [PATCH] omap: iommu: disallow mapping NULL address >>>>>>>>> >>>>>>>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping >>>>>>>>> the NULL address if da_start==0. Force da_start to exclude the >>>>>>>>> first page. >>>>>>>> >>>>>>>> what about devices that uses page 0? ipu after reset always starts >>>>>>>> from 0x00000000 how could we map that address?? >>>>>>> >>>>>>> from 0x0? The driver sees da == 0 as error. May I ask you why do you >>>>>>> want it? >>>>>> >>>>>> unlike DSP that you can load a register with the addres the DSP will >>>>>> boot, IPU core always starts from address 0x00000000, so if you take >>>>>> IPU out of reset it will try to access address 0x0 if not map it, >>>>>> there will be a mmu fault. >>>>> >>>>> Hm. Looks like the iommu should not restrict any da. The valid da >>>>> range should rely only on pdata. >>>>> Michael, what about just update ISP's da_start on omap-iommu.c file? >>>>> Set it to 0x1000. >>>> >>>> What about patching the OMAP3 ISP driver to use a non-zero value (maybe >>>> -1) as an invalid/freed pointer ? >>> >>> I wouldn't be comfortable to use 0 (or NULL) value as valid address on >>> ISP driver. >> >> Why not ? The IOMMUs can use 0x00000000 as a valid address. Whether we allow >> it or not is a software architecture decision, not influenced by the IOMMU >> hardware. As some peripherals (namely IPU) require mapping memory to >> 0x00000000, the IOMMU layer must support it and not treat 0x00000000 >> specially. All da == 0 checks to aim at catching invalid address values must >> be removed, both from the IOMMU API and the IOMMU internals. > > Yes, it can use and IOMMU should not treat is specially. That's the > aim of my patch: > [PATCH v2 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag > I'm not advocating to not allow 0x0, but to not use it when user is > not requesting fixed da. In many sw architecture decisions 0x0 address > is a special case. To avoid any misuse, IOMMU will not use it unless > it's requested. If user is not requesting fixed 'da', it's not a > problem to not give 0x0 anyway. IMO that's the safer option for all > cases. I agree. >>> The 'da' range (da_start and da_end) is defined per VM and specified as >>> platform data. IMO, to set da_start = 0x1000 seems to be> a correct approach >>> for ISP as it's the only client for its IOMMU instance. >> >> We can do that, and then use 0 as an invalid pointer in the ISP driver. As the >> IOMMU API will use another value (what about 0xffffffff, as for the userspace >> mmap() call ?) to mean "invalid pointer", it might be better to use the same >> value in the ISP driver. > > That can be done, of course. But the main point is in OMAP3 ISP all > initial register values to read/write from/to memory are 0x0. It means > sometimes we can catch bugs more easily by not mapping that address. > So, IMO, OMAP3 ISP should not allow to map first page. But that's a > special case for this driver only. I beg to disagree. The ISP isn't so special. The hardware registers (including DMA destination registers) typically are NULL after reset and NULL is used by drivers to mark a nonexistent object, for example a video buffer. There's a reason why the first page isn't mapped in the system MMU either. Regards,
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c index 5990ea6..dcb5513 100644 --- a/arch/arm/plat-omap/iommu.c +++ b/arch/arm/plat-omap/iommu.c @@ -850,7 +850,7 @@ int iommu_set_da_range(struct iommu *obj, u32 start, u32 end) if (end < start || !PAGE_ALIGN(start | end)) return -EINVAL; - obj->da_start = start; + obj->da_start = max(start, (u32)PAGE_SIZE); obj->da_end = end; return 0; @@ -950,7 +950,9 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev) obj->name = pdata->name; obj->dev = &pdev->dev; obj->ctx = (void *)obj + sizeof(*obj); - obj->da_start = pdata->da_start; + + /* reserve the first page for NULL */ + obj->da_start = max(pdata->da_start, (u32)PAGE_SIZE); obj->da_end = pdata->da_end; mutex_init(&obj->iommu_lock);