[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <33765a47-b37f-4435-b12c-c1d712ebc194@amd.com>
Date: Wed, 7 Jan 2026 16:07:51 -0600
From: Mario Limonciello <mario.limonciello@....com>
To: Lizhi Hou <lizhi.hou@....com>, ogabbay@...nel.org,
quic_jhugo@...cinc.com, dri-devel@...ts.freedesktop.org,
maciej.falkowski@...ux.intel.com
Cc: linux-kernel@...r.kernel.org, max.zhen@....com, sonal.santan@....com
Subject: Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer
allocation
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@....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);
>>
Powered by blists - more mailing lists