From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from jeth.damsy.net (jeth.damsy.net [51.159.152.102]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 96E21276038 for ; Mon, 23 Feb 2026 15:04:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=51.159.152.102 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771859050; cv=none; b=dtSGKNPvag8x0XJr6DDCm/dxJBWWRfN+xyjfxazdxw5gGYv+4J2nm8Y/tl3Xnjg1P3QcOMCYo2vCSYIWKza0nCZxpRtsCtS11JX63lqJtiUXGgs99WKj4ZRQTz8jylbZ8C8pf9uBUBJb6ejXIjImXkl8v7TD3c4ybqTuLtfvaBY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771859050; c=relaxed/simple; bh=1jhGCby/Dq8MtroTBqAXnGAXr7rq5GG2F40DU6uSj/s=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=kzZCJoh5Fgz8ZGh2biJGSwatWACgudf3358HNzQDfdLn/bGRALE2rfDTH1FyAuYKJyG+g3LxsXg1WPrpLDgUrLTwaXNiI+3Q9/N6FzgP3ThhS+hdHg2Xefvln9D0CtQUynTjNH7zCh2x6y8qROwDry7e4K2Bw8Fzg3pSAp0yWyQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=damsy.net; spf=pass smtp.mailfrom=damsy.net; dkim=pass (2048-bit key) header.d=damsy.net header.i=@damsy.net header.b=OB5Pnnwk; dkim=permerror (0-bit key) header.d=damsy.net header.i=@damsy.net header.b=mqi3tkex; arc=none smtp.client-ip=51.159.152.102 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=damsy.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=damsy.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=damsy.net header.i=@damsy.net header.b="OB5Pnnwk"; dkim=permerror (0-bit key) header.d=damsy.net header.i=@damsy.net header.b="mqi3tkex" DKIM-Signature: v=1; a=rsa-sha256; s=202408r; d=damsy.net; c=relaxed/relaxed; h=From:To:Subject:Date:Message-ID; t=1771859016; bh=//VyQSb1AAV7R2tybo1nmuq E1TMTuJ9/sXf5xbAxuLQ=; b=OB5PnnwkJToiY2UZwgtIrXom49CvJKfPBVuPXnISLiKqYkAXHc ZYJX3nyKh39WIJs8+fxvwI+B66YkGFJ4cTq8//fXkftzua5IFKZZY87YKNLU8IfthJiH6F0WvB0 2wbgTJ6G6fglY4QNLvSSVpyn7v+5dqrJGHJ12Fqt9rrQei/zeKuOJWctH4P5DPrAhu1wyUGyNg+ COhii0/pKvEg3TD+xIM0WM2q2psum9vGbaYQ77eaJBnk8QxVVXMM1HVBMhlHHNImuvPMp+eaZLc NCYYrU0lcsHazduhqx1rtieYxVhdNnfwRJyacHVpJr8mRDvb08uWk+i/3Z+ocO4Jh2w==; DKIM-Signature: v=1; a=ed25519-sha256; s=202408e; d=damsy.net; c=relaxed/relaxed; h=From:To:Subject:Date:Message-ID; t=1771859016; bh=//VyQSb1AAV7R2tybo1nmuq E1TMTuJ9/sXf5xbAxuLQ=; b=mqi3tkexMVYPE4S9pIIvoV8bkGunoJtTNnusw+RtqVTEp+9m/l GnvhBR2XnzCpuJthrHMms1hrMzyUPJiQNLBQ==; Message-ID: <97ea1340-a17a-47f9-b491-2b6bd70cc81c@damsy.net> Date: Mon, 23 Feb 2026 16:03:34 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 6/6] drm/amdgpu: dump job ibs in the devcoredump To: Alex Deucher Cc: Pierre-Eric Pelloux-Prayer , Alex Deucher , =?UTF-8?Q?Christian_K=C3=B6nig?= , David Airlie , Simona Vetter , amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20260211102657.4180-1-pierre-eric.pelloux-prayer@amd.com> <20260211102657.4180-6-pierre-eric.pelloux-prayer@amd.com> <78b10f62-60d1-4dee-94a0-fa2eaf139cb6@damsy.net> Content-Language: en-US From: Pierre-Eric Pelloux-Prayer In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Le 17/02/2026 à 17:47, Alex Deucher a écrit : > On Tue, Feb 17, 2026 at 11:38 AM Pierre-Eric Pelloux-Prayer > wrote: >> >> >> >> Le 17/02/2026 à 17:20, Alex Deucher a écrit : >>> On Wed, Feb 11, 2026 at 6:07 AM Pierre-Eric Pelloux-Prayer >>> wrote: >>>> >>>> Now that we have a worker thread, we can try to access the >>>> IBs of the job. The process is: >>>> * get the VM from the PASID >>>> * get the BO from its VA and the VM >>>> * map the BO for CPU access >>>> * copy everything, then add it to the dump >>>> Each step can fail so we have to be cautious. >>>> These operations can be slow so when amdgpu_devcoredump_format >>>> is called only to determine the size of the buffer we skip all >>>> of them and assume they will succeed. >>>> >>>> Signed-off-by: Pierre-Eric Pelloux-Prayer >>>> --- >>>> .../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c | 84 ++++++++++++++++++- >>>> 1 file changed, 83 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c >>>> index d0af8a294abf..d576518c212d 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c >>>> @@ -200,12 +200,20 @@ static void amdgpu_devcoredump_fw_info(struct amdgpu_device *adev, >>>> static ssize_t >>>> amdgpu_devcoredump_format(char *buffer, size_t count, struct amdgpu_coredump_info *coredump) >>>> { >>>> + struct amdgpu_device *adev = coredump->adev; >>>> struct drm_printer p; >>>> struct drm_print_iterator iter; >>>> struct amdgpu_vm_fault_info *fault_info; >>>> + struct amdgpu_bo_va_mapping *mapping; >>>> struct amdgpu_ip_block *ip_block; >>>> + struct amdgpu_res_cursor cursor; >>>> + struct amdgpu_bo *abo, *root; >>>> + uint64_t va_start, offset; >>>> struct amdgpu_ring *ring; >>>> - int ver, i, j; >>>> + struct amdgpu_vm *vm; >>>> + u32 *ib_content; >>>> + uint8_t *kptr; >>>> + int ver, i, j, r; >>>> u32 ring_idx, off; >>>> >>>> iter.data = buffer; >>>> @@ -323,6 +331,80 @@ amdgpu_devcoredump_format(char *buffer, size_t count, struct amdgpu_coredump_inf >>>> else if (coredump->reset_vram_lost) >>>> drm_printf(&p, "VRAM is lost due to GPU reset!\n"); >>>> >>>> + if (coredump->num_ibs) { >>>> + if (buffer) >>>> + vm = amdgpu_vm_lock_by_pasid(adev, &root, coredump->pasid); >>>> + else >>>> + vm = NULL; >>> >>> Is there any point in doing the loop if the vm is NULL? >> >> Yes : when doing the first pass to size the final buffer I skip the >> buffers mapping + read operations that might be slow and instead just >> account for the outputting of ib_size_dw dwords. > > But if you don't map the buffer, you'll just be dumping the random > content from ib_content[]. No. I'll send a v2 with more comments, but what's happening is that this function is called twice: * the first time with buffer=NULL. In this case we just need the drm_printf calls to happen. Because the output' size isn't dependent on the values read from ib_content it's fine to jump to the output_ib_content label as early as possible. * the second time buffer is not NULL. If the vm is now NULL, we won't enter the loop at all. The other advantage of skipping the vm lookup and bo mapping when sizing the buffer is that we size the buffer as if all future lookups/mapping are successful. Pierre-Eric > >> >>> >>>> + >>>> + for (int i = 0; i < coredump->num_ibs && (!buffer || vm); i++) { >>>> + ib_content = kvmalloc_array(coredump->ibs[i].ib_size_dw, 4, >>>> + GFP_KERNEL); >>> >>> Shouldn't this be GFP_NOWAIT? >> >> This is executed by a worker so GFP_KERNEL should be ok? > > Oh, right. Yeah, should be ok. > > Alex > >> >> Pierre-Eric >> >>> >>> Alex >>> >>>> + if (!ib_content) >>>> + continue; >>>> + >>>> + if (!vm) >>>> + goto output_ib_content; >>>> + >>>> + va_start = coredump->ibs[i].gpu_addr & AMDGPU_GMC_HOLE_MASK; >>>> + mapping = amdgpu_vm_bo_lookup_mapping(vm, va_start / AMDGPU_GPU_PAGE_SIZE); >>>> + if (!mapping) >>>> + goto free_ib_content; >>>> + >>>> + offset = va_start - (mapping->start * AMDGPU_GPU_PAGE_SIZE); >>>> + abo = amdgpu_bo_ref(mapping->bo_va->base.bo); >>>> + r = amdgpu_bo_reserve(abo, false); >>>> + if (r) >>>> + goto free_ib_content; >>>> + >>>> + if (abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS) { >>>> + off = 0; >>>> + >>>> + if (abo->tbo.resource->mem_type != TTM_PL_VRAM) >>>> + goto unreserve_abo; >>>> + >>>> + amdgpu_res_first(abo->tbo.resource, offset, >>>> + coredump->ibs[i].ib_size_dw * 4, >>>> + &cursor); >>>> + while (cursor.remaining) { >>>> + amdgpu_device_mm_access(adev, cursor.start / 4, >>>> + &ib_content[off], cursor.size / 4, >>>> + false); >>>> + off += cursor.size; >>>> + amdgpu_res_next(&cursor, cursor.size); >>>> + } >>>> + } else { >>>> + r = ttm_bo_kmap(&abo->tbo, 0, >>>> + PFN_UP(abo->tbo.base.size), >>>> + &abo->kmap); >>>> + if (r) >>>> + goto unreserve_abo; >>>> + >>>> + kptr = amdgpu_bo_kptr(abo); >>>> + kptr += offset; >>>> + memcpy(ib_content, kptr, >>>> + coredump->ibs[i].ib_size_dw * 4); >>>> + >>>> + amdgpu_bo_kunmap(abo); >>>> + } >>>> + >>>> +output_ib_content: >>>> + drm_printf(&p, "\nIB #%d 0x%llx %d dw\n", >>>> + i, coredump->ibs[i].gpu_addr, coredump->ibs[i].ib_size_dw); >>>> + for (int j = 0; j < coredump->ibs[i].ib_size_dw; j++) >>>> + drm_printf(&p, "0x%08x\n", ib_content[j]); >>>> +unreserve_abo: >>>> + if (vm) >>>> + amdgpu_bo_unreserve(abo); >>>> +free_ib_content: >>>> + kfree(ib_content); >>>> + } >>>> + if (vm) { >>>> + amdgpu_bo_unreserve(root); >>>> + amdgpu_bo_unref(&root); >>>> + } >>>> + } >>>> + >>>> return count - iter.remain; >>>> } >>>> >>>> -- >>>> 2.43.0 >>>>