| Message ID | 20210315205859.19590-1-s-anna@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | remoteproc: pru: Fix firmware loading crashes on K3 SoCs | expand |
On Mon, Mar 15, 2021 at 03:58:59PM -0500, Suman Anna wrote: > The K3 PRUs are 32-bit processors and in general have some limitations > in using the standard ARMv8 memcpy function for loading firmware segments, > so the driver already uses a custom memcpy implementation. This added > logic however is limited to only IRAMs at the moment, but the loading > into Data RAMs is not completely ok either and does generate a kernel > crash for unaligned accesses. > > Fix these crashes by removing the existing IRAM logic limitation and > extending the custom memcpy usage to Data RAMs as well for all K3 SoCs. > > Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores on K3 AM65x SoCs") > Signed-off-by: Suman Anna <s-anna@ti.com> Probably a good idea to CC stable as well... Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > --- > drivers/remoteproc/pru_rproc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c > index 2667919d76b3..16979c1cd2f4 100644 > --- a/drivers/remoteproc/pru_rproc.c > +++ b/drivers/remoteproc/pru_rproc.c > @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const struct firmware *fw) > break; > } > > - if (pru->data->is_k3 && is_iram) { > + if (pru->data->is_k3) { > ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset, > filesz); > if (ret) { > -- > 2.30.1 >
On 3/23/21 6:20 PM, Mathieu Poirier wrote: > On Mon, Mar 15, 2021 at 03:58:59PM -0500, Suman Anna wrote: >> The K3 PRUs are 32-bit processors and in general have some limitations >> in using the standard ARMv8 memcpy function for loading firmware segments, >> so the driver already uses a custom memcpy implementation. This added >> logic however is limited to only IRAMs at the moment, but the loading >> into Data RAMs is not completely ok either and does generate a kernel >> crash for unaligned accesses. >> >> Fix these crashes by removing the existing IRAM logic limitation and >> extending the custom memcpy usage to Data RAMs as well for all K3 SoCs. >> >> Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores on K3 AM65x SoCs") >> Signed-off-by: Suman Anna <s-anna@ti.com> > > Probably a good idea to CC stable as well... > > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> Thanks Mathieu. This patch is already staged on Bjorn's rproc-fixes branch though and part of linux-next since next-20210319. I have posted an additional 3-patch series for some more PRU fixes. Do you want me to post a v2 for those with stable Cc'd? regards Suman > >> --- >> drivers/remoteproc/pru_rproc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c >> index 2667919d76b3..16979c1cd2f4 100644 >> --- a/drivers/remoteproc/pru_rproc.c >> +++ b/drivers/remoteproc/pru_rproc.c >> @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const struct firmware *fw) >> break; >> } >> >> - if (pru->data->is_k3 && is_iram) { >> + if (pru->data->is_k3) { >> ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset, >> filesz); >> if (ret) { >> -- >> 2.30.1 >>
On Wed, 24 Mar 2021 at 11:09, Suman Anna <s-anna@ti.com> wrote: > > On 3/23/21 6:20 PM, Mathieu Poirier wrote: > > On Mon, Mar 15, 2021 at 03:58:59PM -0500, Suman Anna wrote: > >> The K3 PRUs are 32-bit processors and in general have some limitations > >> in using the standard ARMv8 memcpy function for loading firmware segments, > >> so the driver already uses a custom memcpy implementation. This added > >> logic however is limited to only IRAMs at the moment, but the loading > >> into Data RAMs is not completely ok either and does generate a kernel > >> crash for unaligned accesses. > >> > >> Fix these crashes by removing the existing IRAM logic limitation and > >> extending the custom memcpy usage to Data RAMs as well for all K3 SoCs. > >> > >> Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores on K3 AM65x SoCs") > >> Signed-off-by: Suman Anna <s-anna@ti.com> > > > > Probably a good idea to CC stable as well... > > > > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > Thanks Mathieu. This patch is already staged on Bjorn's rproc-fixes branch > though and part of linux-next since next-20210319. I have posted an additional > 3-patch series for some more PRU fixes. Do you want me to post a v2 for those > with stable Cc'd? I didn't notice Bjorn had already picked it up. Since the object is now public there is no need to send a V2 for this one. I haven't looked at your other 3-patch series but if you think it is stable material then yes, please send a new revision that CC stable. Mathieu > > regards > Suman > > > > >> --- > >> drivers/remoteproc/pru_rproc.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c > >> index 2667919d76b3..16979c1cd2f4 100644 > >> --- a/drivers/remoteproc/pru_rproc.c > >> +++ b/drivers/remoteproc/pru_rproc.c > >> @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const struct firmware *fw) > >> break; > >> } > >> > >> - if (pru->data->is_k3 && is_iram) { > >> + if (pru->data->is_k3) { > >> ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset, > >> filesz); > >> if (ret) { > >> -- > >> 2.30.1 > >> >
On 3/25/21 12:36 PM, Mathieu Poirier wrote: > On Wed, 24 Mar 2021 at 11:09, Suman Anna <s-anna@ti.com> wrote: >> >> On 3/23/21 6:20 PM, Mathieu Poirier wrote: >>> On Mon, Mar 15, 2021 at 03:58:59PM -0500, Suman Anna wrote: >>>> The K3 PRUs are 32-bit processors and in general have some limitations >>>> in using the standard ARMv8 memcpy function for loading firmware segments, >>>> so the driver already uses a custom memcpy implementation. This added >>>> logic however is limited to only IRAMs at the moment, but the loading >>>> into Data RAMs is not completely ok either and does generate a kernel >>>> crash for unaligned accesses. >>>> >>>> Fix these crashes by removing the existing IRAM logic limitation and >>>> extending the custom memcpy usage to Data RAMs as well for all K3 SoCs. >>>> >>>> Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores on K3 AM65x SoCs") >>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>> >>> Probably a good idea to CC stable as well... >>> >>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> >> >> Thanks Mathieu. This patch is already staged on Bjorn's rproc-fixes branch >> though and part of linux-next since next-20210319. I have posted an additional >> 3-patch series for some more PRU fixes. Do you want me to post a v2 for those >> with stable Cc'd? > > I didn't notice Bjorn had already picked it up. Since the object is > now public there is no need to send a V2 for this one. I haven't > looked at your other 3-patch series but if you think it is stable > material then yes, please send a new revision that CC stable. Alright, will do. regards Suman > > Mathieu > >> >> regards >> Suman >> >>> >>>> --- >>>> drivers/remoteproc/pru_rproc.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c >>>> index 2667919d76b3..16979c1cd2f4 100644 >>>> --- a/drivers/remoteproc/pru_rproc.c >>>> +++ b/drivers/remoteproc/pru_rproc.c >>>> @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const struct firmware *fw) >>>> break; >>>> } >>>> >>>> - if (pru->data->is_k3 && is_iram) { >>>> + if (pru->data->is_k3) { >>>> ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset, >>>> filesz); >>>> if (ret) { >>>> -- >>>> 2.30.1 >>>> >>
On 3/25/21 3:09 PM, Suman Anna wrote: > On 3/25/21 12:36 PM, Mathieu Poirier wrote: >> On Wed, 24 Mar 2021 at 11:09, Suman Anna <s-anna@ti.com> wrote: >>> >>> On 3/23/21 6:20 PM, Mathieu Poirier wrote: >>>> On Mon, Mar 15, 2021 at 03:58:59PM -0500, Suman Anna wrote: >>>>> The K3 PRUs are 32-bit processors and in general have some limitations >>>>> in using the standard ARMv8 memcpy function for loading firmware segments, >>>>> so the driver already uses a custom memcpy implementation. This added >>>>> logic however is limited to only IRAMs at the moment, but the loading >>>>> into Data RAMs is not completely ok either and does generate a kernel >>>>> crash for unaligned accesses. >>>>> >>>>> Fix these crashes by removing the existing IRAM logic limitation and >>>>> extending the custom memcpy usage to Data RAMs as well for all K3 SoCs. >>>>> >>>>> Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores on K3 AM65x SoCs") >>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>> >>>> Probably a good idea to CC stable as well... >>>> >>>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> >>> >>> Thanks Mathieu. This patch is already staged on Bjorn's rproc-fixes branch >>> though and part of linux-next since next-20210319. I have posted an additional >>> 3-patch series for some more PRU fixes. Do you want me to post a v2 for those >>> with stable Cc'd? >> >> I didn't notice Bjorn had already picked it up. Since the object is >> now public there is no need to send a V2 for this one. I haven't >> looked at your other 3-patch series but if you think it is stable >> material then yes, please send a new revision that CC stable. > > Alright, will do. On a second thought, we don't have any dts nodes in-kernel yet (5.13-rc1 would be the first kernel with PRU nodes), so those are not critical for v5.11 kernel. As long as they get fixed in either the current v5.12-rc's or by v5.13-rc1, we will be fine. regards Suman > > regards > Suman > >> >> Mathieu >> >>> >>> regards >>> Suman >>> >>>> >>>>> --- >>>>> drivers/remoteproc/pru_rproc.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c >>>>> index 2667919d76b3..16979c1cd2f4 100644 >>>>> --- a/drivers/remoteproc/pru_rproc.c >>>>> +++ b/drivers/remoteproc/pru_rproc.c >>>>> @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const struct firmware *fw) >>>>> break; >>>>> } >>>>> >>>>> - if (pru->data->is_k3 && is_iram) { >>>>> + if (pru->data->is_k3) { >>>>> ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset, >>>>> filesz); >>>>> if (ret) { >>>>> -- >>>>> 2.30.1 >>>>> >>> >
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c index 2667919d76b3..16979c1cd2f4 100644 --- a/drivers/remoteproc/pru_rproc.c +++ b/drivers/remoteproc/pru_rproc.c @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const struct firmware *fw) break; } - if (pru->data->is_k3 && is_iram) { + if (pru->data->is_k3) { ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset, filesz); if (ret) {
The K3 PRUs are 32-bit processors and in general have some limitations in using the standard ARMv8 memcpy function for loading firmware segments, so the driver already uses a custom memcpy implementation. This added logic however is limited to only IRAMs at the moment, but the loading into Data RAMs is not completely ok either and does generate a kernel crash for unaligned accesses. Fix these crashes by removing the existing IRAM logic limitation and extending the custom memcpy usage to Data RAMs as well for all K3 SoCs. Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores on K3 AM65x SoCs") Signed-off-by: Suman Anna <s-anna@ti.com> --- drivers/remoteproc/pru_rproc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)