[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3ff55fef-efd7-4de6-b81f-a5a57021e4bb@amd.com>
Date: Thu, 7 Mar 2024 18:13:10 +0530
From: "Khatri, Sunil" <sukhatri@....com>
To: Christian König <christian.koenig@....com>,
Sunil Khatri <sunil.khatri@....com>, Alex Deucher
<alexander.deucher@....com>, Shashank Sharma <shashank.sharma@....com>
Cc: amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, Mukul Joshi <mukul.joshi@....com>,
Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@....com>
Subject: Re: [PATCH] drm/amdgpu: add vm fault information to devcoredump
On 3/7/2024 6:10 PM, Christian König wrote:
> Am 07.03.24 um 09:37 schrieb Khatri, Sunil:
>>
>> On 3/7/2024 1:47 PM, Christian König wrote:
>>> Am 06.03.24 um 19:19 schrieb Sunil Khatri:
>>>> Add page fault information to the devcoredump.
>>>>
>>>> Output of devcoredump:
>>>> **** AMDGPU Device Coredump ****
>>>> version: 1
>>>> kernel: 6.7.0-amd-staging-drm-next
>>>> module: amdgpu
>>>> time: 29.725011811
>>>> process_name: soft_recovery_p PID: 1720
>>>>
>>>> Ring timed out details
>>>> IP Type: 0 Ring Name: gfx_0.0.0
>>>>
>>>> [gfxhub] Page fault observed for GPU family:143
>>>> Faulty page starting at address 0x0000000000000000
>>>> Protection fault status register:0x301031
>>>>
>>>> VRAM is lost due to GPU reset!
>>>>
>>>> Signed-off-by: Sunil Khatri <sunil.khatri@....com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 15 ++++++++++++++-
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
>>>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>>>> index 147100c27c2d..d7fea6cdf2f9 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>>>> @@ -203,8 +203,20 @@ amdgpu_devcoredump_read(char *buffer, loff_t
>>>> offset, size_t count,
>>>> coredump->ring->name);
>>>> }
>>>> + if (coredump->fault_info.status) {
>>>> + struct amdgpu_vm_fault_info *fault_info =
>>>> &coredump->fault_info;
>>>> +
>>>> + drm_printf(&p, "\n[%s] Page fault observed for GPU
>>>> family:%d\n",
>>>> + fault_info->vmhub ? "mmhub" : "gfxhub",
>>>> + coredump->adev->family);
>>>> + drm_printf(&p, "Faulty page starting at address 0x%016llx\n",
>>>> + fault_info->addr);
>>>> + drm_printf(&p, "Protection fault status register:0x%x\n",
>>>> + fault_info->status);
>>>> + }
>>>> +
>>>> if (coredump->reset_vram_lost)
>>>> - drm_printf(&p, "VRAM is lost due to GPU reset!\n");
>>>> + drm_printf(&p, "\nVRAM is lost due to GPU reset!\n");
>>>> if (coredump->adev->reset_info.num_regs) {
>>>> drm_printf(&p, "AMDGPU register dumps:\nOffset:
>>>> Value:\n");
>>>> @@ -253,6 +265,7 @@ void amdgpu_coredump(struct amdgpu_device
>>>> *adev, bool vram_lost,
>>>> if (job) {
>>>> s_job = &job->base;
>>>> coredump->ring = to_amdgpu_ring(s_job->sched);
>>>> + coredump->fault_info = job->vm->fault_info;
>>>
>>> That's illegal. The VM pointer might already be stale at this point.
>>>
>>> I think you need to add the fault info of the last fault globally in
>>> the VRAM manager or move this to the process info Shashank is
>>> working on.
>>> Are you saying that during the reset or otherwise a vm which is part
>>> of this job could have been freed and we might have a NULL
>>> dereference or invalid reference? Till now based on the resets and
>>> pagefaults that i have created till now using the same app which we
>>> are using for IH overflow i am able to get the valid vm only.
>>>
>>> Assuming amdgpu_vm is freed for this job or stale, are you
>>> suggesting to update this information in adev-> vm_manager along
>>> with existing per vm fault_info or only in vm_manager ?
>
> Good question. having it both in the VM as well as the VM manager
> sounds like the simplest option for now.
Let me update the patch then with information in VM manager.
Regards
Sunil
>
> Regards,
> Christian.
>
>>>
>>> Regards,
>>> Christian.
>>>
>>>> }
>>>> coredump->adev = adev;
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>>> index 60522963aaca..3197955264f9 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>>> @@ -98,6 +98,7 @@ struct amdgpu_coredump_info {
>>>> struct timespec64 reset_time;
>>>> bool reset_vram_lost;
>>>> struct amdgpu_ring *ring;
>>>> + struct amdgpu_vm_fault_info fault_info;
>>>> };
>>>> #endif
>>>
>
Powered by blists - more mailing lists