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

[Mario Limonciello <mario.limonciello@amd.com>]

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?

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