Am 23.07.24 um 11:04 schrieb Yin, ZhenGuo (Chris):
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Christian
I prepared this patch because we met a page fault after gpu reset in SRIOV triggered by quark.
After investigation, I found that the page table did not get restored after gpu reset.
I just tried to use vm_update_mode=0 to disable VM update by CPU, and this issue still exists.
CPU based page table updates are not supported under SRIOV.
-[Christian] When VRAM is lost any submission using the VM entity will fail and so result in a new page table generation.
I believe that you are referring this piece of code in function amdgpu_job_run():
/* Skip job if VRAM is lost and never resubmit gangs */
if (job->generation != amdgpu_vm_generation(adev, job->vm) ||
(job->job_run_counter && job->gang_submit))
dma_fence_set_error(finished, -ECANCELED);
I agree that the submission from the old ctx will be set as ECANCELED and be skipped.
But if the application then creates a new ctx and submit a new job, both new_ctx->generation and new_job->generation will be initiated as the updated one.
ctx->generation = amdgpu_vm_generation(mgr->adev, &fpriv->vm);
(*job)->generation = amdgpu_vm_generation(adev, vm);
And the job will be executed, hence there will be a page fault due to VRAM lost.
Please correct me if I have some misunderstanding.
I still can not see why any submission using the VM entity will fail due to VRAM lost.
Ah! My assumption was that you will also always have a VM page table job
which would be canceled.
But that isn't the case, instead you have only a context which is
re-created.
In that case the approach here is valid, but let me comment on the patch
itself.
Regards,
Christian.
Best,
Zhenguo
Cloud-GPU Core team, SRDC
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Tuesday, July 23, 2024 3:13 PM
To: Yin, ZhenGuo (Chris) <ZhenGuo.Yin@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Koenig, Christian <Christian.Koenig@xxxxxxx>
Subject: Re: [PATCH v2] drm/amdgpu: reset vm state machine after gpu reset(vram lost)
Am 23.07.24 um 05:05 schrieb ZhenGuo Yin:
[Why]
Page table of compute VM in the VRAM will lost after gpu reset.
VRAM won't be restored since compute VM has no shadows.
[How]
Use higher 32-bit of vm->generation to record a vram_lost_counter.
Reset the VM state machine when vm->genertaion is not equal to
re-generation token.
v2: Check vm->generation instead of calling drm_sched_entity_error in
amdgpu_vm_validate.
I've just double checked the logic again and as far as I can see this patch here is still completely superfluous.
When VRAM is lost any submission using the VM entity will fail and so result in a new page table generation.
What isn't handled are CPU based page table updates, but for those we could potentially change the condition inside the CPU page tables code.
Regards,
Christian.
Signed-off-by: ZhenGuo Yin <zhenguo.yin@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3abfa66d72a2..9e2f84c166e7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -434,7 +434,7 @@ uint64_t amdgpu_vm_generation(struct amdgpu_device *adev, struct amdgpu_vm *vm)
if (!vm)
return result;
- result += vm->generation;
+ result += (vm->generation & 0xFFFFFFFF);
/* Add one if the page tables will be re-generated on next CS */
if (drm_sched_entity_error(&vm->delayed))
++result;
@@ -467,9 +467,12 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
struct amdgpu_bo *shadow;
struct amdgpu_bo *bo;
int r;
+ uint32_t vram_lost_counter = atomic_read(&adev->vram_lost_counter);
- if (drm_sched_entity_error(&vm->delayed)) {
- ++vm->generation;
+ if (vm->generation != amdgpu_vm_generation(adev, vm)) {
+ if (drm_sched_entity_error(&vm->delayed))
+ ++vm->generation;
+ vm->generation = (u64)vram_lost_counter << 32 | (vm->generation &
+0xFFFFFFFF);
amdgpu_vm_bo_reset_state_machine(vm);
amdgpu_vm_fini_entities(vm);
r = amdgpu_vm_init_entities(adev, vm); @@ -2439,7 +2442,7 @@ int
amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
vm->last_update = dma_fence_get_stub();
vm->last_unlocked = dma_fence_get_stub();
vm->last_tlb_flush = dma_fence_get_stub();
- vm->generation = 0;
+ vm->generation = (u64)atomic_read(&adev->vram_lost_counter) << 32;
mutex_init(&vm->eviction_lock);
vm->evicting = false;
