Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

["Mario Limonciello (AMD) (kernel.org)" <superm1@kernel.org>]

On 1/7/2026 4:20 PM, Lizhi Hou wrote:
> 
> On 1/7/26 14:07, Mario Limonciello wrote:
>> On 1/7/26 4:05 PM, Lizhi Hou wrote:
>>>
>>> On 1/7/26 13:19, Mario Limonciello wrote:
>>>> On 12/18/25 7:43 PM, Lizhi Hou wrote:
>>>>> The latest firmware requires the message DMA buffer to
>>>>>    - have a minimum size of 8K
>>>>>    - use a power-of-two size
>>>>>    - be aligned to the buffer size
>>>>>    - not cross 64M boundary
>>>>>
>>>>> Update the buffer allocation logic to meet these requirements and 
>>>>> support
>>>>> the latest firmware.
>>>>
>>>> We can't guarantee that kernel and firmware are moving at the same 
>>>> time.
>>>> What happens if you run old firmware with these changes?
>>>
>>> The old firmware runs fine this these changes. The new firmware adds 
>>> more alignment and size requirements which the old one does not need.
>>
>> Ah OK - so patch 2 won't reject current firmware right?
> 
> Correct. It will not reject current firmware.
> 
Thank for confirming, no other concerns with this patch.

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>

> 
> Lizhi
> 
>>
>>>
>>>
>>> Lizhi
>>>
>>>>
>>>> If the old firmware can't run with these changes then it would be 
>>>> better to instead add a fallback system.
>>>>
>>>> IE:
>>>> 1) kernel tries to load new firmware name and use new behavior
>>>> 2) if firmware is missing, kernel tries to load old firmware name 
>>>> and use old behavior.
>>>> 3) if firmware is missing in old name then fail probe
>>>>
>>>>
>>>>>
>>>>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>>>>> ---
>>>>>   drivers/accel/amdxdna/aie2_error.c   | 10 ++++-----
>>>>>   drivers/accel/amdxdna/aie2_message.c | 33 +++++++++++++++++++ 
>>>>> +--------
>>>>>   drivers/accel/amdxdna/aie2_pci.h     |  5 +++++
>>>>>   3 files changed, 33 insertions(+), 15 deletions(-)
>>>>>
>>>>> diff --git a/drivers/accel/amdxdna/aie2_error.c b/drivers/accel/ 
>>>>> amdxdna/aie2_error.c
>>>>> index d452008ec4f4..5e82df2b7cf6 100644
>>>>> --- a/drivers/accel/amdxdna/aie2_error.c
>>>>> +++ b/drivers/accel/amdxdna/aie2_error.c
>>>>> @@ -338,8 +338,7 @@ void aie2_error_async_events_free(struct 
>>>>> amdxdna_dev_hdl *ndev)
>>>>>       destroy_workqueue(events->wq);
>>>>>       mutex_lock(&xdna->dev_lock);
>>>>>   -    dma_free_noncoherent(xdna->ddev.dev, events->size, events->buf,
>>>>> -                 events->addr, DMA_FROM_DEVICE);
>>>>> +    aie2_free_msg_buffer(ndev, events->size, events->buf, events- 
>>>>> >addr);
>>>>>       kfree(events);
>>>>>   }
>>>>>   @@ -355,8 +354,8 @@ int aie2_error_async_events_alloc(struct 
>>>>> amdxdna_dev_hdl *ndev)
>>>>>       if (!events)
>>>>>           return -ENOMEM;
>>>>>   -    events->buf = dma_alloc_noncoherent(xdna->ddev.dev, 
>>>>> total_size, &events->addr,
>>>>> -                        DMA_FROM_DEVICE, GFP_KERNEL);
>>>>> +    events->buf = aie2_alloc_msg_buffer(ndev, &total_size, 
>>>>> &events- >addr);
>>>>> +
>>>>>       if (!events->buf) {
>>>>>           ret = -ENOMEM;
>>>>>           goto free_events;
>>>>> @@ -396,8 +395,7 @@ int aie2_error_async_events_alloc(struct 
>>>>> amdxdna_dev_hdl *ndev)
>>>>>   free_wq:
>>>>>       destroy_workqueue(events->wq);
>>>>>   free_buf:
>>>>> -    dma_free_noncoherent(xdna->ddev.dev, events->size, events->buf,
>>>>> -                 events->addr, DMA_FROM_DEVICE);
>>>>> +    aie2_free_msg_buffer(ndev, events->size, events->buf, events- 
>>>>> >addr);
>>>>>   free_events:
>>>>>       kfree(events);
>>>>>       return ret;
>>>>> diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/ 
>>>>> amdxdna/aie2_message.c
>>>>> index 051f4ceaabae..99215328505e 100644
>>>>> --- a/drivers/accel/amdxdna/aie2_message.c
>>>>> +++ b/drivers/accel/amdxdna/aie2_message.c
>>>>> @@ -55,6 +55,22 @@ static int aie2_send_mgmt_msg_wait(struct 
>>>>> amdxdna_dev_hdl *ndev,
>>>>>       return ret;
>>>>>   }
>>>>>   +void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl *ndev, u32 
>>>>> *size,
>>>>> +                dma_addr_t *dma_addr)
>>>>> +{
>>>>> +    struct amdxdna_dev *xdna = ndev->xdna;
>>>>> +    int order;
>>>>> +
>>>>> +    *size = max(*size, SZ_8K);
>>>>> +    order = get_order(*size);
>>>>> +    if (order > MAX_PAGE_ORDER)
>>>>> +        return NULL;
>>>>> +    *size = PAGE_SIZE << order;
>>>>> +
>>>>> +    return dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
>>>>> +                     DMA_FROM_DEVICE, GFP_KERNEL);
>>>>> +}
>>>>> +
>>>>>   int aie2_suspend_fw(struct amdxdna_dev_hdl *ndev)
>>>>>   {
>>>>>       DECLARE_AIE2_MSG(suspend, MSG_OP_SUSPEND);
>>>>> @@ -346,14 +362,13 @@ int aie2_query_status(struct amdxdna_dev_hdl 
>>>>> *ndev, char __user *buf,
>>>>>   {
>>>>>       DECLARE_AIE2_MSG(aie_column_info, MSG_OP_QUERY_COL_STATUS);
>>>>>       struct amdxdna_dev *xdna = ndev->xdna;
>>>>> +    u32 buf_sz = size, aie_bitmap = 0;
>>>>>       struct amdxdna_client *client;
>>>>>       dma_addr_t dma_addr;
>>>>> -    u32 aie_bitmap = 0;
>>>>>       u8 *buff_addr;
>>>>>       int ret;
>>>>>   -    buff_addr = dma_alloc_noncoherent(xdna->ddev.dev, size, 
>>>>> &dma_addr,
>>>>> -                      DMA_FROM_DEVICE, GFP_KERNEL);
>>>>> +    buff_addr = aie2_alloc_msg_buffer(ndev, &buf_sz, &dma_addr);
>>>>>       if (!buff_addr)
>>>>>           return -ENOMEM;
>>>>>   @@ -363,7 +378,7 @@ int aie2_query_status(struct amdxdna_dev_hdl 
>>>>> *ndev, char __user *buf,
>>>>>         *cols_filled = 0;
>>>>>       req.dump_buff_addr = dma_addr;
>>>>> -    req.dump_buff_size = size;
>>>>> +    req.dump_buff_size = buf_sz;
>>>>>       req.num_cols = hweight32(aie_bitmap);
>>>>>       req.aie_bitmap = aie_bitmap;
>>>>>   @@ -391,7 +406,7 @@ int aie2_query_status(struct amdxdna_dev_hdl 
>>>>> *ndev, char __user *buf,
>>>>>       *cols_filled = aie_bitmap;
>>>>>     fail:
>>>>> -    dma_free_noncoherent(xdna->ddev.dev, size, buff_addr, 
>>>>> dma_addr, DMA_FROM_DEVICE);
>>>>> +    aie2_free_msg_buffer(ndev, buf_sz, buff_addr, dma_addr);
>>>>>       return ret;
>>>>>   }
>>>>>   @@ -402,19 +417,19 @@ int aie2_query_telemetry(struct 
>>>>> amdxdna_dev_hdl *ndev,
>>>>>       DECLARE_AIE2_MSG(get_telemetry, MSG_OP_GET_TELEMETRY);
>>>>>       struct amdxdna_dev *xdna = ndev->xdna;
>>>>>       dma_addr_t dma_addr;
>>>>> +    u32 buf_sz = size;
>>>>>       u8 *addr;
>>>>>       int ret;
>>>>>         if (header->type >= MAX_TELEMETRY_TYPE)
>>>>>           return -EINVAL;
>>>>>   -    addr = dma_alloc_noncoherent(xdna->ddev.dev, size, &dma_addr,
>>>>> -                     DMA_FROM_DEVICE, GFP_KERNEL);
>>>>> +    addr = aie2_alloc_msg_buffer(ndev, &buf_sz, &dma_addr);
>>>>>       if (!addr)
>>>>>           return -ENOMEM;
>>>>>         req.buf_addr = dma_addr;
>>>>> -    req.buf_size = size;
>>>>> +    req.buf_size = buf_sz;
>>>>>       req.type = header->type;
>>>>>         drm_clflush_virt_range(addr, size); /* device can access */
>>>>> @@ -440,7 +455,7 @@ int aie2_query_telemetry(struct amdxdna_dev_hdl 
>>>>> *ndev,
>>>>>       header->minor = resp.minor;
>>>>>     free_buf:
>>>>> -    dma_free_noncoherent(xdna->ddev.dev, size, addr, dma_addr, 
>>>>> DMA_FROM_DEVICE);
>>>>> +    aie2_free_msg_buffer(ndev, buf_sz, addr, dma_addr);
>>>>>       return ret;
>>>>>   }
>>>>>   diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/ 
>>>>> amdxdna/aie2_pci.h
>>>>> index a929fa98a121..e1745f07b268 100644
>>>>> --- a/drivers/accel/amdxdna/aie2_pci.h
>>>>> +++ b/drivers/accel/amdxdna/aie2_pci.h
>>>>> @@ -336,6 +336,11 @@ int aie2_sync_bo(struct amdxdna_hwctx *hwctx, 
>>>>> struct amdxdna_sched_job *job,
>>>>>            int (*notify_cb)(void *, void __iomem *, size_t));
>>>>>   int aie2_config_debug_bo(struct amdxdna_hwctx *hwctx, struct 
>>>>> amdxdna_sched_job *job,
>>>>>                int (*notify_cb)(void *, void __iomem *, size_t));
>>>>> +void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl *ndev, u32 *size,
>>>>> +                dma_addr_t *dma_addr);
>>>>> +#define aie2_free_msg_buffer(ndev, size, buff_addr, 
>>>>> dma_addr)        \
>>>>> +    dma_free_noncoherent((ndev)->xdna->ddev.dev, size, 
>>>>> buff_addr,    \
>>>>> +                 dma_addr, DMA_FROM_DEVICE)
>>>>>     /* aie2_hwctx.c */
>>>>>   int aie2_hwctx_init(struct amdxdna_hwctx *hwctx);
>>>>
>>