Received-SPF: pass (google.com: domain of linux-kernel+bounces-95149-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <bd6a70dc-d710-498e-b4ed-35c6106cd898@amd.com>
Date: Thu, 7 Mar 2024 14:07:15 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] drm/amdgpu: add vm fault information to devcoredump
Content-Language: en-US
To: =?UTF-8?Q?Christian_K=C3=B6nig?= <christian.koenig@amd.com>,
 Sunil Khatri <sunil.khatri@amd.com>, Alex Deucher
 <alexander.deucher@amd.com>, Shashank Sharma <shashank.sharma@amd.com>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
 linux-kernel@vger.kernel.org, Mukul Joshi <mukul.joshi@amd.com>,
 Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
References: <20240306181937.3551648-1-sunil.khatri@amd.com>
 <20240306181937.3551648-2-sunil.khatri@amd.com>
 <f61edcbe-938f-4c48-920e-64c8352e87f4@amd.com>
From: "Khatri, Sunil" <sukhatri@amd.com>
In-Reply-To: <f61edcbe-938f-4c48-920e-64c8352e87f4@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?UjY3eGt5Wmo4ZHh3UEhrejJtRlFQMVdJTnlaUktrRmxaVDl3TEQvR21pRUVI?=
 =?utf-8?B?aVcrV2xUcm9UTmZkbUcwbUlOUVRPaWsrckJXRWZRZE1haGVFV0p3Ykd3REYv?=
 =?utf-8?B?d0FiQXJvT01naW5MNnVZZ0x0VDRyb25LMVdnQTVXSnRJZ1pRekprTG16eFBF?=
 =?utf-8?B?OThrQUI5L3BDdyt5dEtEQU5MTkJna3d0bkhCbG54ZUpSTnhkYnJZM2t3bTZL?=
 =?utf-8?B?YlcrM3hqMlY1NVhQeHZ0UVc3ME96Q0lVN3o3NGo3andCNXovZkdZQTJ3NU16?=
 =?utf-8?B?Y3EvOVlhSi9ST1NJOEh6c0pHZ1lOblN1UEZrWGZBVzRUTGN5N3BvNFNXWll3?=
 =?utf-8?B?Nms5aG83WTRHMUo4TDN5TW8rcE4rVmhlSzV4YkFzc3ZMR0YrTmM2RVk4YTFG?=
 =?utf-8?B?Rkxwcm5JUEZNUlpTRnVhT1NQQTJhOGgrM1c0TCs5MVlvUW1FakR2dmJaQnkz?=
 =?utf-8?B?VlVNcGp4ZTR6QjlKcmZyM2R5WXpzVUpFS2ZjUXZ3ZzJoaXFESndFRWMzbU1z?=
 =?utf-8?B?dCtwM3J1aUJjSmE1V25TOGpRdkFUUEhLdlJQSDZLWVVCUlV5RzBnKzJsRUhm?=
 =?utf-8?B?R3R4ZnloZXhab1A2aEF4YVBXRXcwWm9WYWg1cUIwSlpUdWRWZmh1cUZrbjFj?=
 =?utf-8?B?UHk3eDVjZXF2RWs5d0Nqb01BbnZpaS9LREx3bURoTHdNWklLSWg0MWpUd0h0?=
 =?utf-8?B?d3J2NnkxQ1BTcEtieVRaa2RjZVYzQ3pXcmZIRlNhckFMK0crU0ppTlZ5WVBC?=
 =?utf-8?B?ZUJnWjhrc2tSQ1JOanZZME1pMW5vOGdLVFBSVjByQWlsaU5hQVp2RTJINnU4?=
 =?utf-8?B?VmtCYXpSaUJ3KzFKRzBGNWc5b2ZzQjF5NnRoZXNMRVZFSTRmS3ZxeHFPSXRJ?=
 =?utf-8?B?T3VKYmxjUnh5SzhFMmt6RUdWaG1Sb3E5N1RLcDRobXQzbGFXNzQ3L2Q3b0Rv?=
 =?utf-8?B?ZWUvSG1ROUZCUmxpVXArazlnek51YWhWcVRJeEJ4WWtVa3RqazFzYnh3Mzha?=
 =?utf-8?B?VlVqZHNGLzNyOVJvenJ2REd5bFY0WDRZa014MWZsMUh1aU9Cb0t3cnkrQWZ1?=
 =?utf-8?B?b2pUMGtLZVkyNzJQVDRsV2dHRkg4Ly82ajZzdFJkL2ZVeWQxYUJlZ0Y0djdT?=
 =?utf-8?B?TkJCd3g0M01JZWpjSVNSNEQ3WFdTdUdGMjRxQTM3enFaOFZPczltY29lUFV0?=
 =?utf-8?B?a0UrZlBGWC9qMGVOMWxFN1RLWmtyby9rc1dLdVZCckQwVVg0c3FjL0NFb08w?=
 =?utf-8?B?cXB6alJkUThYRWlpRy9WcWlOVmY0aExvSkFvbWhhSmNDeGdDaHJkc2lyeExo?=
 =?utf-8?B?YS83OWR0TXhDbG5jWjBQMEdSTlRWUkFlRGIrYjJDTGRmRDROMmNQL05rNldZ?=
 =?utf-8?B?U0Znb2dENWVCeks1Y3RZcVpnOTdRcmE2QmdxOGU3QkhMNDZYTFoyS29Vd2pH?=
 =?utf-8?B?WHJzalNKUlJPWTZhMHY5b3IxK2FhYlhmTHR1UEI1MkFSS1o3VUwrYkI4V0U4?=
 =?utf-8?B?dTZwSExsUjZyMEVuOTgrTXNxVDRCNWZ5QUNtZWdkWGkxZm81cUp1cDRLTzRL?=
 =?utf-8?B?WktiU0hhWFo4aks3UGVnUlpnaTlrckw3d0pVbmt6SDlKa1cyRmoxRDNONktH?=
 =?utf-8?B?Sy9tKzJKTk9BOHZseEExUXp2K1pnTjVhaFU2bk4vd3oyS09xZDJlcFJIU0Nl?=
 =?utf-8?B?TFo5Qy84Zzh1cUZBWWlPaHpzanBIdDk2dWtNNTRqQTdvSjIzWTRzNVJHZDdx?=
 =?utf-8?B?UXBySkp1VGl4dzcxK0xWbjZ1NXpNMUZuUDZsc1M1SnJwWkJGdDJleW8rajZh?=
 =?utf-8?B?bGZwNjR3TnN5VHRVZkZLQ0J2K2lVUkRUaXZrc1k2M3lDd1RmSWJBYmhsSlpT?=
 =?utf-8?B?N01TUGdUMjUzbmFTQjh3amN0UUtOS1B0ZW9XUXdLc05mZ2M5WHgxQlcreWpW?=
 =?utf-8?B?RXk0Z0NEWGU5VWxOTXhxYmJWYVBLcnhsNTZ1ZWxTQmxHWldUeTIzSGVFc0hv?=
 =?utf-8?B?WE5Va1dleEZhSWtPSHJEWkRMeGhPV0k3SU5hQzdLaDZka0JZeTY3U2E1RzlB?=
 =?utf-8?B?LzNlS1BOY2NHV3pHbjYxc2VGdVBmbHRwamp5cFBsYkNJMEdRbGVPcVU3STNL?=
 =?utf-8?Q?HgYIk4mBZI0mE/SM8PpESZYU4?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 20973e5d-50b8-48d6-57dc-08dc3e81d010
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB5596.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2024 08:37:24.5854
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: rRCwWpl8vnOAAYn+fkxEhd3vyVzHxFGyYMruOQzVBlPAjnOVuTCCk+MHJVgf9PJ6ymdBTTwTy5x4WFJ7LDaX7w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7236


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@amd.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 ?
>
> 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
>