2021-03-16 04:51:25

by Suman Anna

[permalink] [raw]
Subject: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs

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 <[email protected]>
---
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


2021-03-24 21:36:20

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs

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 <[email protected]>

Probably a good idea to CC stable as well...

Reviewed-by: Mathieu Poirier <[email protected]>

> ---
> 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
>

2021-03-25 03:19:25

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs

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 <[email protected]>
>
> Probably a good idea to CC stable as well...
>
> Reviewed-by: Mathieu Poirier <[email protected]>

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
>>

2021-03-25 17:38:07

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs

On Wed, 24 Mar 2021 at 11:09, Suman Anna <[email protected]> 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 <[email protected]>
> >
> > Probably a good idea to CC stable as well...
> >
> > Reviewed-by: Mathieu Poirier <[email protected]>
>
> 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
> >>
>

2021-03-25 20:11:29

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs

On 3/25/21 12:36 PM, Mathieu Poirier wrote:
> On Wed, 24 Mar 2021 at 11:09, Suman Anna <[email protected]> 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 <[email protected]>
>>>
>>> Probably a good idea to CC stable as well...
>>>
>>> Reviewed-by: Mathieu Poirier <[email protected]>
>>
>> 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
>>>>
>>

2021-03-25 20:21:10

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs

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 <[email protected]> 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 <[email protected]>
>>>>
>>>> Probably a good idea to CC stable as well...
>>>>
>>>> Reviewed-by: Mathieu Poirier <[email protected]>
>>>
>>> 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
>>>>>
>>>
>