Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
Hi Loic, > >> -Original Message- >> From: Suman Anna >> Sent: mercredi 24 octobre 2018 05:47 >> To: Loic PALLARDY ; bjorn.anders...@linaro.org; >> o...@wizery.com >> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; >> Arnaud POULIQUEN ; >> benjamin.gaign...@linaro.org >> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device >> address requested >> >> On 10/23/18 2:40 PM, Loic PALLARDY wrote: >>> Hi Suman, >>> >>>> -Original Message- >>>> From: Suman Anna >>>> Sent: mardi 23 octobre 2018 19:26 >>>> To: Loic PALLARDY ; bjorn.anders...@linaro.org; >>>> o...@wizery.com >>>> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; >>>> Arnaud POULIQUEN ; >>>> benjamin.gaign...@linaro.org >>>> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if >> device >>>> address requested >>>> >>>> Hi Loic, >>>> >>>> On 7/27/18 8:14 AM, Loic Pallardy wrote: >>>>> If there is no IOMMU associate to remote processor device, >>>>> remoteproc_core won't be able to satisfy device address requested >>>>> in firmware resource table. >>>>> Return an error as configuration won't be coherent. >>>>> >>>>> Signed-off-by: Loic Pallardy >>>> >>>> This patch is breaking my Davinci platforms. It is not really required >>>> that you _should_ have IOMMUs when a valid DA is mentioned. Please >> see >>>> the existing description (paras 4 and 5) on the fw_rsc_carveout >>>> kerneldoc in remoteproc.h file. >>> >>> Thanks for pointing this comment. Indeed sMMU is not mandatory, and at >> first sight I agree we should remove the restriction introduced by the patch. >>> Driver porting on the series should be done before adding this. >>>> >>>> We do have platforms where we have some internal sub-modules within >> the >>>> remote processor sub-system that provides some linear >>>> address-translation (most common case with 32-bit processors supporting >>>> 64-bit addresses). Also, we have some upcoming SoCs where we have an >>>> MMU >>>> but is not programmable by Linux. >>>> >>>> There is one comment there, but I don't think this is actually handled >>>> in the current remoteproc core. >>>> "If @da is set to >>>> * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and >> then >>>> * overwrite @da with the dynamically allocated address." >>>> >>> I don't remember it was implemented like described. >> >> Yes, it was missing, and one of your patches seem to add this behavior >> now. That said, I really don't think the remoteproc core can dictate the >> da. Even if the individual remoteproc driver were to furnish this, how >> would you get such data without forcing a fixed behavior for all >> possible firmwares (not desirable). We should get rid of this comment, >> and any code that seems to do this. > > Agree that if some rules are fixed by firmware, rproc core can't overwrite > them. > It was the goal of the patch as without sMMU if you don't have a match > between rproc core carveout allocation and firmware rsc request, rproc core > can't answer the request. Question is what are we going to use to check that - da or pa? With fixed memory support, I would think that this would have to be based on pa if specified. Given that the meaning of da can change from firmware to firmware (in the case of some translators), but you can only reserve a fixed reserved memory at kernel bootup time. > In the rest of the series, da in rsc table is updated only in some specific > cases (i.e. da is equal to FW_RSC_ADDR_ANY). If there is a force update I > agree we should not allow it. >> >>> >>> I have remarks about the comment: >>> "* We will always use @da to negotiate the device addresses, even if it >>> * isn't using an iommu. In that case, though, it will obviously contain >>> * physical addresses." >>> >>> When there is no sMMU, we can't consider that da contains a physical >> address because coprocessor can have its own memory map just because it >> is a 32bit processor accessing only a part of the memory and the main is >> 64bit >> one. The 2 processors won't see the internal memory at the same base >> address for example. >> >>
Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
Hi Loic, > >> -Original Message- >> From: Suman Anna >> Sent: mercredi 24 octobre 2018 05:47 >> To: Loic PALLARDY ; bjorn.anders...@linaro.org; >> o...@wizery.com >> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; >> Arnaud POULIQUEN ; >> benjamin.gaign...@linaro.org >> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device >> address requested >> >> On 10/23/18 2:40 PM, Loic PALLARDY wrote: >>> Hi Suman, >>> >>>> -Original Message- >>>> From: Suman Anna >>>> Sent: mardi 23 octobre 2018 19:26 >>>> To: Loic PALLARDY ; bjorn.anders...@linaro.org; >>>> o...@wizery.com >>>> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; >>>> Arnaud POULIQUEN ; >>>> benjamin.gaign...@linaro.org >>>> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if >> device >>>> address requested >>>> >>>> Hi Loic, >>>> >>>> On 7/27/18 8:14 AM, Loic Pallardy wrote: >>>>> If there is no IOMMU associate to remote processor device, >>>>> remoteproc_core won't be able to satisfy device address requested >>>>> in firmware resource table. >>>>> Return an error as configuration won't be coherent. >>>>> >>>>> Signed-off-by: Loic Pallardy >>>> >>>> This patch is breaking my Davinci platforms. It is not really required >>>> that you _should_ have IOMMUs when a valid DA is mentioned. Please >> see >>>> the existing description (paras 4 and 5) on the fw_rsc_carveout >>>> kerneldoc in remoteproc.h file. >>> >>> Thanks for pointing this comment. Indeed sMMU is not mandatory, and at >> first sight I agree we should remove the restriction introduced by the patch. >>> Driver porting on the series should be done before adding this. >>>> >>>> We do have platforms where we have some internal sub-modules within >> the >>>> remote processor sub-system that provides some linear >>>> address-translation (most common case with 32-bit processors supporting >>>> 64-bit addresses). Also, we have some upcoming SoCs where we have an >>>> MMU >>>> but is not programmable by Linux. >>>> >>>> There is one comment there, but I don't think this is actually handled >>>> in the current remoteproc core. >>>> "If @da is set to >>>> * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and >> then >>>> * overwrite @da with the dynamically allocated address." >>>> >>> I don't remember it was implemented like described. >> >> Yes, it was missing, and one of your patches seem to add this behavior >> now. That said, I really don't think the remoteproc core can dictate the >> da. Even if the individual remoteproc driver were to furnish this, how >> would you get such data without forcing a fixed behavior for all >> possible firmwares (not desirable). We should get rid of this comment, >> and any code that seems to do this. > > Agree that if some rules are fixed by firmware, rproc core can't overwrite > them. > It was the goal of the patch as without sMMU if you don't have a match > between rproc core carveout allocation and firmware rsc request, rproc core > can't answer the request. Question is what are we going to use to check that - da or pa? With fixed memory support, I would think that this would have to be based on pa if specified. Given that the meaning of da can change from firmware to firmware (in the case of some translators), but you can only reserve a fixed reserved memory at kernel bootup time. > In the rest of the series, da in rsc table is updated only in some specific > cases (i.e. da is equal to FW_RSC_ADDR_ANY). If there is a force update I > agree we should not allow it. >> >>> >>> I have remarks about the comment: >>> "* We will always use @da to negotiate the device addresses, even if it >>> * isn't using an iommu. In that case, though, it will obviously contain >>> * physical addresses." >>> >>> When there is no sMMU, we can't consider that da contains a physical >> address because coprocessor can have its own memory map just because it >> is a 32bit processor accessing only a part of the memory and the main is >> 64bit >> one. The 2 processors won't see the internal memory at the same base >> address for example. >> >>
RE: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
> -Original Message- > From: Suman Anna > Sent: mercredi 24 octobre 2018 05:47 > To: Loic PALLARDY ; bjorn.anders...@linaro.org; > o...@wizery.com > Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; > Arnaud POULIQUEN ; > benjamin.gaign...@linaro.org > Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device > address requested > > On 10/23/18 2:40 PM, Loic PALLARDY wrote: > > Hi Suman, > > > >> -Original Message- > >> From: Suman Anna > >> Sent: mardi 23 octobre 2018 19:26 > >> To: Loic PALLARDY ; bjorn.anders...@linaro.org; > >> o...@wizery.com > >> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; > >> Arnaud POULIQUEN ; > >> benjamin.gaign...@linaro.org > >> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if > device > >> address requested > >> > >> Hi Loic, > >> > >> On 7/27/18 8:14 AM, Loic Pallardy wrote: > >>> If there is no IOMMU associate to remote processor device, > >>> remoteproc_core won't be able to satisfy device address requested > >>> in firmware resource table. > >>> Return an error as configuration won't be coherent. > >>> > >>> Signed-off-by: Loic Pallardy > >> > >> This patch is breaking my Davinci platforms. It is not really required > >> that you _should_ have IOMMUs when a valid DA is mentioned. Please > see > >> the existing description (paras 4 and 5) on the fw_rsc_carveout > >> kerneldoc in remoteproc.h file. > > > > Thanks for pointing this comment. Indeed sMMU is not mandatory, and at > first sight I agree we should remove the restriction introduced by the patch. > > Driver porting on the series should be done before adding this. > >> > >> We do have platforms where we have some internal sub-modules within > the > >> remote processor sub-system that provides some linear > >> address-translation (most common case with 32-bit processors supporting > >> 64-bit addresses). Also, we have some upcoming SoCs where we have an > >> MMU > >> but is not programmable by Linux. > >> > >> There is one comment there, but I don't think this is actually handled > >> in the current remoteproc core. > >> "If @da is set to > >> * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and > then > >> * overwrite @da with the dynamically allocated address." > >> > > I don't remember it was implemented like described. > > Yes, it was missing, and one of your patches seem to add this behavior > now. That said, I really don't think the remoteproc core can dictate the > da. Even if the individual remoteproc driver were to furnish this, how > would you get such data without forcing a fixed behavior for all > possible firmwares (not desirable). We should get rid of this comment, > and any code that seems to do this. Agree that if some rules are fixed by firmware, rproc core can't overwrite them. It was the goal of the patch as without sMMU if you don't have a match between rproc core carveout allocation and firmware rsc request, rproc core can't answer the request. In the rest of the series, da in rsc table is updated only in some specific cases (i.e. da is equal to FW_RSC_ADDR_ANY). If there is a force update I agree we should not allow it. > > > > > I have remarks about the comment: > > "* We will always use @da to negotiate the device addresses, even if it > > * isn't using an iommu. In that case, though, it will obviously contain > > * physical addresses." > > > > When there is no sMMU, we can't consider that da contains a physical > address because coprocessor can have its own memory map just because it > is a 32bit processor accessing only a part of the memory and the main is 64bit > one. The 2 processors won't see the internal memory at the same base > address for example. > > Agreed, believe it was valid when it was written (32-bit platforms > supporting 32-bit addresses). I think this is akin to an IPA > (Intermediate Physical Address). > > > So what should we do when carveout allocated by host is not fitting with > resource table request? > > - put a warning and overwrite da address in the resource table? > > Hmm, why da? This goes to my earlier comment about how you are able to > decide the da. Atleast your current ST driver seems to be assigning the > same value as the physical bus address for da, which would prompt why > you would still need a carveout entry in the resource table if it is
RE: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
> -Original Message- > From: Suman Anna > Sent: mercredi 24 octobre 2018 05:47 > To: Loic PALLARDY ; bjorn.anders...@linaro.org; > o...@wizery.com > Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; > Arnaud POULIQUEN ; > benjamin.gaign...@linaro.org > Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device > address requested > > On 10/23/18 2:40 PM, Loic PALLARDY wrote: > > Hi Suman, > > > >> -Original Message- > >> From: Suman Anna > >> Sent: mardi 23 octobre 2018 19:26 > >> To: Loic PALLARDY ; bjorn.anders...@linaro.org; > >> o...@wizery.com > >> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; > >> Arnaud POULIQUEN ; > >> benjamin.gaign...@linaro.org > >> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if > device > >> address requested > >> > >> Hi Loic, > >> > >> On 7/27/18 8:14 AM, Loic Pallardy wrote: > >>> If there is no IOMMU associate to remote processor device, > >>> remoteproc_core won't be able to satisfy device address requested > >>> in firmware resource table. > >>> Return an error as configuration won't be coherent. > >>> > >>> Signed-off-by: Loic Pallardy > >> > >> This patch is breaking my Davinci platforms. It is not really required > >> that you _should_ have IOMMUs when a valid DA is mentioned. Please > see > >> the existing description (paras 4 and 5) on the fw_rsc_carveout > >> kerneldoc in remoteproc.h file. > > > > Thanks for pointing this comment. Indeed sMMU is not mandatory, and at > first sight I agree we should remove the restriction introduced by the patch. > > Driver porting on the series should be done before adding this. > >> > >> We do have platforms where we have some internal sub-modules within > the > >> remote processor sub-system that provides some linear > >> address-translation (most common case with 32-bit processors supporting > >> 64-bit addresses). Also, we have some upcoming SoCs where we have an > >> MMU > >> but is not programmable by Linux. > >> > >> There is one comment there, but I don't think this is actually handled > >> in the current remoteproc core. > >> "If @da is set to > >> * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and > then > >> * overwrite @da with the dynamically allocated address." > >> > > I don't remember it was implemented like described. > > Yes, it was missing, and one of your patches seem to add this behavior > now. That said, I really don't think the remoteproc core can dictate the > da. Even if the individual remoteproc driver were to furnish this, how > would you get such data without forcing a fixed behavior for all > possible firmwares (not desirable). We should get rid of this comment, > and any code that seems to do this. Agree that if some rules are fixed by firmware, rproc core can't overwrite them. It was the goal of the patch as without sMMU if you don't have a match between rproc core carveout allocation and firmware rsc request, rproc core can't answer the request. In the rest of the series, da in rsc table is updated only in some specific cases (i.e. da is equal to FW_RSC_ADDR_ANY). If there is a force update I agree we should not allow it. > > > > > I have remarks about the comment: > > "* We will always use @da to negotiate the device addresses, even if it > > * isn't using an iommu. In that case, though, it will obviously contain > > * physical addresses." > > > > When there is no sMMU, we can't consider that da contains a physical > address because coprocessor can have its own memory map just because it > is a 32bit processor accessing only a part of the memory and the main is 64bit > one. The 2 processors won't see the internal memory at the same base > address for example. > > Agreed, believe it was valid when it was written (32-bit platforms > supporting 32-bit addresses). I think this is akin to an IPA > (Intermediate Physical Address). > > > So what should we do when carveout allocated by host is not fitting with > resource table request? > > - put a warning and overwrite da address in the resource table? > > Hmm, why da? This goes to my earlier comment about how you are able to > decide the da. Atleast your current ST driver seems to be assigning the > same value as the physical bus address for da, which would prompt why > you would still need a carveout entry in the resource table if it is
Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
On 10/23/18 2:40 PM, Loic PALLARDY wrote: > Hi Suman, > >> -Original Message- >> From: Suman Anna >> Sent: mardi 23 octobre 2018 19:26 >> To: Loic PALLARDY ; bjorn.anders...@linaro.org; >> o...@wizery.com >> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; >> Arnaud POULIQUEN ; >> benjamin.gaign...@linaro.org >> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device >> address requested >> >> Hi Loic, >> >> On 7/27/18 8:14 AM, Loic Pallardy wrote: >>> If there is no IOMMU associate to remote processor device, >>> remoteproc_core won't be able to satisfy device address requested >>> in firmware resource table. >>> Return an error as configuration won't be coherent. >>> >>> Signed-off-by: Loic Pallardy >> >> This patch is breaking my Davinci platforms. It is not really required >> that you _should_ have IOMMUs when a valid DA is mentioned. Please see >> the existing description (paras 4 and 5) on the fw_rsc_carveout >> kerneldoc in remoteproc.h file. > > Thanks for pointing this comment. Indeed sMMU is not mandatory, and at first > sight I agree we should remove the restriction introduced by the patch. > Driver porting on the series should be done before adding this. >> >> We do have platforms where we have some internal sub-modules within the >> remote processor sub-system that provides some linear >> address-translation (most common case with 32-bit processors supporting >> 64-bit addresses). Also, we have some upcoming SoCs where we have an >> MMU >> but is not programmable by Linux. >> >> There is one comment there, but I don't think this is actually handled >> in the current remoteproc core. >> "If @da is set to >> * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and then >> * overwrite @da with the dynamically allocated address." >> > I don't remember it was implemented like described. Yes, it was missing, and one of your patches seem to add this behavior now. That said, I really don't think the remoteproc core can dictate the da. Even if the individual remoteproc driver were to furnish this, how would you get such data without forcing a fixed behavior for all possible firmwares (not desirable). We should get rid of this comment, and any code that seems to do this. > > I have remarks about the comment: > "* We will always use @da to negotiate the device addresses, even if it > * isn't using an iommu. In that case, though, it will obviously contain > * physical addresses." > > When there is no sMMU, we can't consider that da contains a physical address > because coprocessor can have its own memory map just because it is a 32bit > processor accessing only a part of the memory and the main is 64bit one. The > 2 processors won't see the internal memory at the same base address for > example. Agreed, believe it was valid when it was written (32-bit platforms supporting 32-bit addresses). I think this is akin to an IPA (Intermediate Physical Address). > So what should we do when carveout allocated by host is not fitting with > resource table request? > - put a warning and overwrite da address in the resource table? Hmm, why da? This goes to my earlier comment about how you are able to decide the da. Atleast your current ST driver seems to be assigning the same value as the physical bus address for da, which would prompt why you would still need a carveout entry in the resource table if it is truly one-to-one. Eg, I have an upcoming usecase with R5Fs on newer TI SoCs where we actually have a sub-module called Region Address Translator (RAT) which can only be programmed by the R5F for translating the 32-bit CPU addresses to larger physical address space, and yet I need the da and pa to be able to do loading. I cannot dictate the da since that is what the firmware images are linked against. So, have to rely on the firmware providing this data for me. > - stop rproc probe as no match detected? I think that is the safest approach. > > Later in the series, carveout allocation is changed. Resource table carveout > are either linked with an existing carveout registered by driver or added to > carveout list for allocations. > In the case you described, TI driver should first register the specific > carveout regions thank to the helper. The current series should still continue to work without having to enforce new name assignments (unless needed and being defined to use the new features being added). > > Regards, > Loic > >> regards >> Suman >> >>> --- >>> drivers/remoteproc/remoteproc_core.c | 10 +- >>> 1
Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
On 10/23/18 2:40 PM, Loic PALLARDY wrote: > Hi Suman, > >> -Original Message- >> From: Suman Anna >> Sent: mardi 23 octobre 2018 19:26 >> To: Loic PALLARDY ; bjorn.anders...@linaro.org; >> o...@wizery.com >> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; >> Arnaud POULIQUEN ; >> benjamin.gaign...@linaro.org >> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device >> address requested >> >> Hi Loic, >> >> On 7/27/18 8:14 AM, Loic Pallardy wrote: >>> If there is no IOMMU associate to remote processor device, >>> remoteproc_core won't be able to satisfy device address requested >>> in firmware resource table. >>> Return an error as configuration won't be coherent. >>> >>> Signed-off-by: Loic Pallardy >> >> This patch is breaking my Davinci platforms. It is not really required >> that you _should_ have IOMMUs when a valid DA is mentioned. Please see >> the existing description (paras 4 and 5) on the fw_rsc_carveout >> kerneldoc in remoteproc.h file. > > Thanks for pointing this comment. Indeed sMMU is not mandatory, and at first > sight I agree we should remove the restriction introduced by the patch. > Driver porting on the series should be done before adding this. >> >> We do have platforms where we have some internal sub-modules within the >> remote processor sub-system that provides some linear >> address-translation (most common case with 32-bit processors supporting >> 64-bit addresses). Also, we have some upcoming SoCs where we have an >> MMU >> but is not programmable by Linux. >> >> There is one comment there, but I don't think this is actually handled >> in the current remoteproc core. >> "If @da is set to >> * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and then >> * overwrite @da with the dynamically allocated address." >> > I don't remember it was implemented like described. Yes, it was missing, and one of your patches seem to add this behavior now. That said, I really don't think the remoteproc core can dictate the da. Even if the individual remoteproc driver were to furnish this, how would you get such data without forcing a fixed behavior for all possible firmwares (not desirable). We should get rid of this comment, and any code that seems to do this. > > I have remarks about the comment: > "* We will always use @da to negotiate the device addresses, even if it > * isn't using an iommu. In that case, though, it will obviously contain > * physical addresses." > > When there is no sMMU, we can't consider that da contains a physical address > because coprocessor can have its own memory map just because it is a 32bit > processor accessing only a part of the memory and the main is 64bit one. The > 2 processors won't see the internal memory at the same base address for > example. Agreed, believe it was valid when it was written (32-bit platforms supporting 32-bit addresses). I think this is akin to an IPA (Intermediate Physical Address). > So what should we do when carveout allocated by host is not fitting with > resource table request? > - put a warning and overwrite da address in the resource table? Hmm, why da? This goes to my earlier comment about how you are able to decide the da. Atleast your current ST driver seems to be assigning the same value as the physical bus address for da, which would prompt why you would still need a carveout entry in the resource table if it is truly one-to-one. Eg, I have an upcoming usecase with R5Fs on newer TI SoCs where we actually have a sub-module called Region Address Translator (RAT) which can only be programmed by the R5F for translating the 32-bit CPU addresses to larger physical address space, and yet I need the da and pa to be able to do loading. I cannot dictate the da since that is what the firmware images are linked against. So, have to rely on the firmware providing this data for me. > - stop rproc probe as no match detected? I think that is the safest approach. > > Later in the series, carveout allocation is changed. Resource table carveout > are either linked with an existing carveout registered by driver or added to > carveout list for allocations. > In the case you described, TI driver should first register the specific > carveout regions thank to the helper. The current series should still continue to work without having to enforce new name assignments (unless needed and being defined to use the new features being added). > > Regards, > Loic > >> regards >> Suman >> >>> --- >>> drivers/remoteproc/remoteproc_core.c | 10 +- >>> 1
RE: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
Hi Suman,
> -Original Message-
> From: Suman Anna
> Sent: mardi 23 octobre 2018 19:26
> To: Loic PALLARDY ; bjorn.anders...@linaro.org;
> o...@wizery.com
> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org;
> Arnaud POULIQUEN ;
> benjamin.gaign...@linaro.org
> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device
> address requested
>
> Hi Loic,
>
> On 7/27/18 8:14 AM, Loic Pallardy wrote:
> > If there is no IOMMU associate to remote processor device,
> > remoteproc_core won't be able to satisfy device address requested
> > in firmware resource table.
> > Return an error as configuration won't be coherent.
> >
> > Signed-off-by: Loic Pallardy
>
> This patch is breaking my Davinci platforms. It is not really required
> that you _should_ have IOMMUs when a valid DA is mentioned. Please see
> the existing description (paras 4 and 5) on the fw_rsc_carveout
> kerneldoc in remoteproc.h file.
Thanks for pointing this comment. Indeed sMMU is not mandatory, and at first
sight I agree we should remove the restriction introduced by the patch.
Driver porting on the series should be done before adding this.
>
> We do have platforms where we have some internal sub-modules within the
> remote processor sub-system that provides some linear
> address-translation (most common case with 32-bit processors supporting
> 64-bit addresses). Also, we have some upcoming SoCs where we have an
> MMU
> but is not programmable by Linux.
>
> There is one comment there, but I don't think this is actually handled
> in the current remoteproc core.
> "If @da is set to
> * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and then
> * overwrite @da with the dynamically allocated address."
>
I don't remember it was implemented like described.
I have remarks about the comment:
"* We will always use @da to negotiate the device addresses, even if it
* isn't using an iommu. In that case, though, it will obviously contain
* physical addresses."
When there is no sMMU, we can't consider that da contains a physical address
because coprocessor can have its own memory map just because it is a 32bit
processor accessing only a part of the memory and the main is 64bit one. The 2
processors won't see the internal memory at the same base address for example.
So what should we do when carveout allocated by host is not fitting with
resource table request?
- put a warning and overwrite da address in the resource table?
- stop rproc probe as no match detected?
Later in the series, carveout allocation is changed. Resource table carveout
are either linked with an existing carveout registered by driver or added to
carveout list for allocations.
In the case you described, TI driver should first register the specific
carveout regions thank to the helper.
Regards,
Loic
> regards
> Suman
>
> > ---
> > drivers/remoteproc/remoteproc_core.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> > index 4cd1a8e..437fabf 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -657,7 +657,15 @@ static int rproc_handle_carveout(struct rproc
> *rproc,
> > * to use the iommu-based DMA API: we expect 'dma' to contain the
> > * physical address in this case.
> > */
> > - if (rproc->domain) {
> > +
> > + if (rsc->da != FW_RSC_ADDR_ANY && !rproc->domain) {
> > + dev_err(dev->parent,
> > + "Bad carveout rsc configuration\n");
> > + ret = -ENOMEM;
> > + goto dma_free;
> > + }
> > +
> > + if (rsc->da != FW_RSC_ADDR_ANY && rproc->domain) {
> > mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> > if (!mapping) {
> > ret = -ENOMEM;
> >
RE: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
Hi Suman,
> -Original Message-
> From: Suman Anna
> Sent: mardi 23 octobre 2018 19:26
> To: Loic PALLARDY ; bjorn.anders...@linaro.org;
> o...@wizery.com
> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org;
> Arnaud POULIQUEN ;
> benjamin.gaign...@linaro.org
> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device
> address requested
>
> Hi Loic,
>
> On 7/27/18 8:14 AM, Loic Pallardy wrote:
> > If there is no IOMMU associate to remote processor device,
> > remoteproc_core won't be able to satisfy device address requested
> > in firmware resource table.
> > Return an error as configuration won't be coherent.
> >
> > Signed-off-by: Loic Pallardy
>
> This patch is breaking my Davinci platforms. It is not really required
> that you _should_ have IOMMUs when a valid DA is mentioned. Please see
> the existing description (paras 4 and 5) on the fw_rsc_carveout
> kerneldoc in remoteproc.h file.
Thanks for pointing this comment. Indeed sMMU is not mandatory, and at first
sight I agree we should remove the restriction introduced by the patch.
Driver porting on the series should be done before adding this.
>
> We do have platforms where we have some internal sub-modules within the
> remote processor sub-system that provides some linear
> address-translation (most common case with 32-bit processors supporting
> 64-bit addresses). Also, we have some upcoming SoCs where we have an
> MMU
> but is not programmable by Linux.
>
> There is one comment there, but I don't think this is actually handled
> in the current remoteproc core.
> "If @da is set to
> * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and then
> * overwrite @da with the dynamically allocated address."
>
I don't remember it was implemented like described.
I have remarks about the comment:
"* We will always use @da to negotiate the device addresses, even if it
* isn't using an iommu. In that case, though, it will obviously contain
* physical addresses."
When there is no sMMU, we can't consider that da contains a physical address
because coprocessor can have its own memory map just because it is a 32bit
processor accessing only a part of the memory and the main is 64bit one. The 2
processors won't see the internal memory at the same base address for example.
So what should we do when carveout allocated by host is not fitting with
resource table request?
- put a warning and overwrite da address in the resource table?
- stop rproc probe as no match detected?
Later in the series, carveout allocation is changed. Resource table carveout
are either linked with an existing carveout registered by driver or added to
carveout list for allocations.
In the case you described, TI driver should first register the specific
carveout regions thank to the helper.
Regards,
Loic
> regards
> Suman
>
> > ---
> > drivers/remoteproc/remoteproc_core.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> > index 4cd1a8e..437fabf 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -657,7 +657,15 @@ static int rproc_handle_carveout(struct rproc
> *rproc,
> > * to use the iommu-based DMA API: we expect 'dma' to contain the
> > * physical address in this case.
> > */
> > - if (rproc->domain) {
> > +
> > + if (rsc->da != FW_RSC_ADDR_ANY && !rproc->domain) {
> > + dev_err(dev->parent,
> > + "Bad carveout rsc configuration\n");
> > + ret = -ENOMEM;
> > + goto dma_free;
> > + }
> > +
> > + if (rsc->da != FW_RSC_ADDR_ANY && rproc->domain) {
> > mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> > if (!mapping) {
> > ret = -ENOMEM;
> >
Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
Hi Loic,
On 7/27/18 8:14 AM, Loic Pallardy wrote:
> If there is no IOMMU associate to remote processor device,
> remoteproc_core won't be able to satisfy device address requested
> in firmware resource table.
> Return an error as configuration won't be coherent.
>
> Signed-off-by: Loic Pallardy
This patch is breaking my Davinci platforms. It is not really required
that you _should_ have IOMMUs when a valid DA is mentioned. Please see
the existing description (paras 4 and 5) on the fw_rsc_carveout
kerneldoc in remoteproc.h file.
We do have platforms where we have some internal sub-modules within the
remote processor sub-system that provides some linear
address-translation (most common case with 32-bit processors supporting
64-bit addresses). Also, we have some upcoming SoCs where we have an MMU
but is not programmable by Linux.
There is one comment there, but I don't think this is actually handled
in the current remoteproc core.
"If @da is set to
* FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and then
* overwrite @da with the dynamically allocated address."
regards
Suman
> ---
> drivers/remoteproc/remoteproc_core.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index 4cd1a8e..437fabf 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -657,7 +657,15 @@ static int rproc_handle_carveout(struct rproc *rproc,
>* to use the iommu-based DMA API: we expect 'dma' to contain the
>* physical address in this case.
>*/
> - if (rproc->domain) {
> +
> + if (rsc->da != FW_RSC_ADDR_ANY && !rproc->domain) {
> + dev_err(dev->parent,
> + "Bad carveout rsc configuration\n");
> + ret = -ENOMEM;
> + goto dma_free;
> + }
> +
> + if (rsc->da != FW_RSC_ADDR_ANY && rproc->domain) {
> mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> if (!mapping) {
> ret = -ENOMEM;
>
Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
Hi Loic,
On 7/27/18 8:14 AM, Loic Pallardy wrote:
> If there is no IOMMU associate to remote processor device,
> remoteproc_core won't be able to satisfy device address requested
> in firmware resource table.
> Return an error as configuration won't be coherent.
>
> Signed-off-by: Loic Pallardy
This patch is breaking my Davinci platforms. It is not really required
that you _should_ have IOMMUs when a valid DA is mentioned. Please see
the existing description (paras 4 and 5) on the fw_rsc_carveout
kerneldoc in remoteproc.h file.
We do have platforms where we have some internal sub-modules within the
remote processor sub-system that provides some linear
address-translation (most common case with 32-bit processors supporting
64-bit addresses). Also, we have some upcoming SoCs where we have an MMU
but is not programmable by Linux.
There is one comment there, but I don't think this is actually handled
in the current remoteproc core.
"If @da is set to
* FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and then
* overwrite @da with the dynamically allocated address."
regards
Suman
> ---
> drivers/remoteproc/remoteproc_core.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index 4cd1a8e..437fabf 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -657,7 +657,15 @@ static int rproc_handle_carveout(struct rproc *rproc,
>* to use the iommu-based DMA API: we expect 'dma' to contain the
>* physical address in this case.
>*/
> - if (rproc->domain) {
> +
> + if (rsc->da != FW_RSC_ADDR_ANY && !rproc->domain) {
> + dev_err(dev->parent,
> + "Bad carveout rsc configuration\n");
> + ret = -ENOMEM;
> + goto dma_free;
> + }
> +
> + if (rsc->da != FW_RSC_ADDR_ANY && rproc->domain) {
> mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> if (!mapping) {
> ret = -ENOMEM;
>
[PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
If there is no IOMMU associate to remote processor device,
remoteproc_core won't be able to satisfy device address requested
in firmware resource table.
Return an error as configuration won't be coherent.
Signed-off-by: Loic Pallardy
---
drivers/remoteproc/remoteproc_core.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index 4cd1a8e..437fabf 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -657,7 +657,15 @@ static int rproc_handle_carveout(struct rproc *rproc,
* to use the iommu-based DMA API: we expect 'dma' to contain the
* physical address in this case.
*/
- if (rproc->domain) {
+
+ if (rsc->da != FW_RSC_ADDR_ANY && !rproc->domain) {
+ dev_err(dev->parent,
+ "Bad carveout rsc configuration\n");
+ ret = -ENOMEM;
+ goto dma_free;
+ }
+
+ if (rsc->da != FW_RSC_ADDR_ANY && rproc->domain) {
mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
if (!mapping) {
ret = -ENOMEM;
--
1.9.1
[PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
If there is no IOMMU associate to remote processor device,
remoteproc_core won't be able to satisfy device address requested
in firmware resource table.
Return an error as configuration won't be coherent.
Signed-off-by: Loic Pallardy
---
drivers/remoteproc/remoteproc_core.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index 4cd1a8e..437fabf 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -657,7 +657,15 @@ static int rproc_handle_carveout(struct rproc *rproc,
* to use the iommu-based DMA API: we expect 'dma' to contain the
* physical address in this case.
*/
- if (rproc->domain) {
+
+ if (rsc->da != FW_RSC_ADDR_ANY && !rproc->domain) {
+ dev_err(dev->parent,
+ "Bad carveout rsc configuration\n");
+ ret = -ENOMEM;
+ goto dma_free;
+ }
+
+ if (rsc->da != FW_RSC_ADDR_ANY && rproc->domain) {
mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
if (!mapping) {
ret = -ENOMEM;
--
1.9.1
