[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <77edbbff-661e-4a4e-b455-8cda6bd84b9e@kernel.org>
Date: Wed, 7 Jan 2026 21:26:26 -0600
From: "Mario Limonciello (AMD) (kernel.org)" <superm1@...nel.org>
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/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@...nel.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@....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