accel/amdxdna: Fix incorrect PSP firmware size

[PATCH V1] accel/amdxdna: Fix incorrect PSP firmware size

Posted by Lizhi Hou 6 months, 2 weeks ago

The incorrect PSP firmware size is used for initializing. It may
cause error for newer version firmware.

Fixes: 8c9ff1b181ba ("accel/amdxdna: Add a new driver for AMD AI Engine")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/aie2_psp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/amdxdna/aie2_psp.c b/drivers/accel/amdxdna/aie2_psp.c
index dc3a072ce3b6..f28a060a8810 100644
--- a/drivers/accel/amdxdna/aie2_psp.c
+++ b/drivers/accel/amdxdna/aie2_psp.c
@@ -126,8 +126,8 @@ struct psp_device *aie2m_psp_create(struct drm_device *ddev, struct psp_config *
 	psp->ddev = ddev;
 	memcpy(psp->psp_regs, conf->psp_regs, sizeof(psp->psp_regs));
 
-	psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN) + PSP_FW_ALIGN;
-	psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz, GFP_KERNEL);
+	psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN);
+	psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz + PSP_FW_ALIGN, GFP_KERNEL);
 	if (!psp->fw_buffer) {
 		drm_err(ddev, "no memory for fw buffer");
 		return NULL;
-- 
2.34.1

Re: [PATCH V1] accel/amdxdna: Fix incorrect PSP firmware size

Posted by Alex Deucher 6 months, 2 weeks ago

On Wed, Jun 4, 2025 at 10:42 AM Lizhi Hou <lizhi.hou@amd.com> wrote:
>
> The incorrect PSP firmware size is used for initializing. It may
> cause error for newer version firmware.
>
> Fixes: 8c9ff1b181ba ("accel/amdxdna: Add a new driver for AMD AI Engine")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> ---
>  drivers/accel/amdxdna/aie2_psp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_psp.c b/drivers/accel/amdxdna/aie2_psp.c
> index dc3a072ce3b6..f28a060a8810 100644
> --- a/drivers/accel/amdxdna/aie2_psp.c
> +++ b/drivers/accel/amdxdna/aie2_psp.c
> @@ -126,8 +126,8 @@ struct psp_device *aie2m_psp_create(struct drm_device *ddev, struct psp_config *
>         psp->ddev = ddev;
>         memcpy(psp->psp_regs, conf->psp_regs, sizeof(psp->psp_regs));
>
> -       psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN) + PSP_FW_ALIGN;
> -       psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz, GFP_KERNEL);
> +       psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN);
> +       psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz + PSP_FW_ALIGN, GFP_KERNEL);

Why do you need the extra PSP_FW_ALIGN in the allocation?

Alex

>         if (!psp->fw_buffer) {
>                 drm_err(ddev, "no memory for fw buffer");
>                 return NULL;
> --
> 2.34.1
>

Re: [PATCH V1] accel/amdxdna: Fix incorrect PSP firmware size

Posted by Lizhi Hou 6 months, 2 weeks ago

On 6/4/25 07:51, Alex Deucher wrote:
> On Wed, Jun 4, 2025 at 10:42 AM Lizhi Hou <lizhi.hou@amd.com> wrote:
>> The incorrect PSP firmware size is used for initializing. It may
>> cause error for newer version firmware.
>>
>> Fixes: 8c9ff1b181ba ("accel/amdxdna: Add a new driver for AMD AI Engine")
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>> ---
>>   drivers/accel/amdxdna/aie2_psp.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_psp.c b/drivers/accel/amdxdna/aie2_psp.c
>> index dc3a072ce3b6..f28a060a8810 100644
>> --- a/drivers/accel/amdxdna/aie2_psp.c
>> +++ b/drivers/accel/amdxdna/aie2_psp.c
>> @@ -126,8 +126,8 @@ struct psp_device *aie2m_psp_create(struct drm_device *ddev, struct psp_config *
>>          psp->ddev = ddev;
>>          memcpy(psp->psp_regs, conf->psp_regs, sizeof(psp->psp_regs));
>>
>> -       psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN) + PSP_FW_ALIGN;
>> -       psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz, GFP_KERNEL);
>> +       psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN);
>> +       psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz + PSP_FW_ALIGN, GFP_KERNEL);
> Why do you need the extra PSP_FW_ALIGN in the allocation?

The start address of the firmware is also required to be PSP_FW_ALIGN 
aligned.


Thanks,

Lizhi

>
> Alex
>
>>          if (!psp->fw_buffer) {
>>                  drm_err(ddev, "no memory for fw buffer");
>>                  return NULL;
>> --
>> 2.34.1
>>

Re: [PATCH V1] accel/amdxdna: Fix incorrect PSP firmware size

Posted by Alex Deucher 6 months, 1 week ago

On Wed, Jun 4, 2025 at 8:12 PM Lizhi Hou <lizhi.hou@amd.com> wrote:
>
>
> On 6/4/25 07:51, Alex Deucher wrote:
> > On Wed, Jun 4, 2025 at 10:42 AM Lizhi Hou <lizhi.hou@amd.com> wrote:
> >> The incorrect PSP firmware size is used for initializing. It may
> >> cause error for newer version firmware.
> >>
> >> Fixes: 8c9ff1b181ba ("accel/amdxdna: Add a new driver for AMD AI Engine")
> >> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> >> ---
> >>   drivers/accel/amdxdna/aie2_psp.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/accel/amdxdna/aie2_psp.c b/drivers/accel/amdxdna/aie2_psp.c
> >> index dc3a072ce3b6..f28a060a8810 100644
> >> --- a/drivers/accel/amdxdna/aie2_psp.c
> >> +++ b/drivers/accel/amdxdna/aie2_psp.c
> >> @@ -126,8 +126,8 @@ struct psp_device *aie2m_psp_create(struct drm_device *ddev, struct psp_config *
> >>          psp->ddev = ddev;
> >>          memcpy(psp->psp_regs, conf->psp_regs, sizeof(psp->psp_regs));
> >>
> >> -       psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN) + PSP_FW_ALIGN;
> >> -       psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz, GFP_KERNEL);
> >> +       psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN);
> >> +       psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz + PSP_FW_ALIGN, GFP_KERNEL);
> > Why do you need the extra PSP_FW_ALIGN in the allocation?
>
> The start address of the firmware is also required to be PSP_FW_ALIGN
> aligned.

Acked-by: Alex Deucher <alexander.deucher@amd.com>

>
>
> Thanks,
>
> Lizhi
>
> >
> > Alex
> >
> >>          if (!psp->fw_buffer) {
> >>                  drm_err(ddev, "no memory for fw buffer");
> >>                  return NULL;
> >> --
> >> 2.34.1
> >>

Re: [PATCH V1] accel/amdxdna: Fix incorrect PSP firmware size

Posted by Lizhi Hou 6 months, 1 week ago

Pushed to drm-misc-fixes

On 6/9/25 06:35, Alex Deucher wrote:
> On Wed, Jun 4, 2025 at 8:12 PM Lizhi Hou <lizhi.hou@amd.com> wrote:
>>
>> On 6/4/25 07:51, Alex Deucher wrote:
>>> On Wed, Jun 4, 2025 at 10:42 AM Lizhi Hou <lizhi.hou@amd.com> wrote:
>>>> The incorrect PSP firmware size is used for initializing. It may
>>>> cause error for newer version firmware.
>>>>
>>>> Fixes: 8c9ff1b181ba ("accel/amdxdna: Add a new driver for AMD AI Engine")
>>>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>>>> ---
>>>>    drivers/accel/amdxdna/aie2_psp.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/accel/amdxdna/aie2_psp.c b/drivers/accel/amdxdna/aie2_psp.c
>>>> index dc3a072ce3b6..f28a060a8810 100644
>>>> --- a/drivers/accel/amdxdna/aie2_psp.c
>>>> +++ b/drivers/accel/amdxdna/aie2_psp.c
>>>> @@ -126,8 +126,8 @@ struct psp_device *aie2m_psp_create(struct drm_device *ddev, struct psp_config *
>>>>           psp->ddev = ddev;
>>>>           memcpy(psp->psp_regs, conf->psp_regs, sizeof(psp->psp_regs));
>>>>
>>>> -       psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN) + PSP_FW_ALIGN;
>>>> -       psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz, GFP_KERNEL);
>>>> +       psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN);
>>>> +       psp->fw_buffer = drmm_kmalloc(ddev, psp->fw_buf_sz + PSP_FW_ALIGN, GFP_KERNEL);
>>> Why do you need the extra PSP_FW_ALIGN in the allocation?
>> The start address of the firmware is also required to be PSP_FW_ALIGN
>> aligned.
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>
>>
>> Thanks,
>>
>> Lizhi
>>
>>> Alex
>>>
>>>>           if (!psp->fw_buffer) {
>>>>                   drm_err(ddev, "no memory for fw buffer");
>>>>                   return NULL;
>>>> --
>>>> 2.34.1
>>>>