Re: [PATCH 2/2] drm/amdgpu/dma_buf: fix page_link check

[Matthew Auld <matthew.auld@intel.com>]

On 07/04/2025 15:46, Christian König wrote:
> Am 07.04.25 um 16:44 schrieb Matthew Auld:
>> On 07/04/2025 15:32, Christian König wrote:
>>> Am 07.04.25 um 16:18 schrieb Matthew Auld:
>>>> The page_link lower bits of the first sg could contain something like
>>>> SG_END, if we are mapping a single VRAM page or contiguous blob which
>>>> fits into one sg entry. Rather pull out the struct page, and use that in
>>>> our check to know if we mapped struct pages vs VRAM.
>>>>
>>>> Fixes: f44ffd677fb3 ("drm/amdgpu: add support for exporting VRAM using DMA-buf v3")
>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>>>> Cc: Christian König <christian.koenig@amd.com>
>>>> Cc: amd-gfx@lists.freedesktop.org
>>>> Cc: <stable@vger.kernel.org> # v5.8+
>>>
>>> Good point, haven't thought about that at all since we only abuse the sg table as DMA addr container.
>>>
>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>>
>>> Were is patch #1 from this series?
>>
>> That one is xe specific:
>> https://lore.kernel.org/intel-xe/20250407141823.44504-3-matthew.auld@intel.com/T/#m4ef16e478cfc8853d4518448dd345a66d5a7f6d9
>>
>> I copied your approach with using page_link here, but with added sg_page().
> 
> Feel free to add my Acked-by to that one as well.

Thanks.

> 
> I just wanted to double check if we need to push the patches upstream together, but that looks like we can take each through individual branches.

Sounds good.

> 
> Thanks,
> Christian.
> 
>>
>>>
>>> Thanks,
>>> Christian.
>>>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>>> index 9f627caedc3f..c9842a0e2a1c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>>> @@ -184,7 +184,7 @@ static void amdgpu_dma_buf_unmap(struct dma_buf_attachment *attach,
>>>>                     struct sg_table *sgt,
>>>>                     enum dma_data_direction dir)
>>>>    {
>>>> -    if (sgt->sgl->page_link) {
>>>> +    if (sg_page(sgt->sgl)) {
>>>>            dma_unmap_sgtable(attach->dev, sgt, dir, 0);
>>>>            sg_free_table(sgt);
>>>>            kfree(sgt);
>>>
>>
>