Hi all, Today's linux-next merge of the drm tree got a conflict in: drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c between commits: 47cd31185090b ("drm/amdgpu: fix missing check in vm_flush()") e3a721753f60c ("drm/amdgpu: skip clearing empty freed VM list on GEM close") b2ff0595c31cc ("drm/amdgpu: always emit the job vm fence") 04cc4aa3617b0 ("drm/amdgpu: fix lifetime issue of amdgpu_vm_get_task_info_pasid()") 4e28aa8f7ee26 ("drm/drm_exec: avoid indirect goto") from the origin tree and commits: 04b48274e9852 ("drm/amdgpu: keep PRT mappings off the vm_bo state lists") cb1e657ccac89 ("drm/amdgpu: handle GDS and SPM without a VM fence") 54a118f1d7e18 ("drm/amdgpu: fix missing check in vm_flush()") 90163c8f3b73a ("drm/amdgpu/gfx7: Fixup emitting SWITCH_BUFFER packets") 66f46209fd5ea ("drm/amdgpu: Drop vm_manager PASID to VM mapping") b0ae60ea3f3f1 ("drm/amdgpu: Resolve VM through DRM PASID ownership") 8ba869e852d4f ("drm/amdgpu: skip clearing empty freed VM list on GEM close") bc639a9eadc75 ("drm/amdgpu: always emit the job vm fence") 9d01579f3f868 ("drm/amdgpu: fix lifetime issue of amdgpu_vm_get_task_info_pasid()") from the drm tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. diff --combined drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 1baad7624f1f9,71050a86bcc3a..0000000000000 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@@ -766,18 -766,22 +766,22 @@@ bool amdgpu_vm_need_pipeline_sync(struc * @ring: ring to use for flush * @job: related job * @need_pipe_sync: is pipe sync needed + * @emit_spm_needed: does the caller need to emit spm + * @emit_gds_needed: does the caller need to emit gds * * Emit a VM flush when it is necessary. */ void amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job, - bool need_pipe_sync) + bool need_pipe_sync, bool *emit_spm_needed, + bool *emit_gds_needed) { struct amdgpu_device *adev = ring->adev; struct amdgpu_isolation *isolation = &adev->isolation[ring->xcp_id]; unsigned vmhub = ring->vm_hub; struct amdgpu_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub]; struct amdgpu_vmid *id = &id_mgr->ids[job->vmid]; - bool spm_update_needed = job->spm_update_needed; + bool spm_update_needed = adev->gfx.rlc.funcs->update_spm_vmid && + job->spm_update_needed; bool gds_switch_needed = ring->funcs->emit_gds_switch && job->gds_switch_needed; bool vm_flush_needed = job->vm_needs_flush; @@@ -785,6 -789,7 +789,7 @@@ bool pasid_mapping_needed = false; struct dma_fence *fence = NULL; unsigned int patch = 0; + bool emit_fence; if (amdgpu_vmid_had_gpu_reset(adev, id)) { gds_switch_needed = true; @@@ -811,6 -816,17 +816,17 @@@ ring->funcs->emit_cleaner_shader && job->base.s_fence && &job->base.s_fence->scheduled == isolation->spearhead; + emit_fence = vm_flush_needed || pasid_mapping_needed || + cleaner_shader_needed; + + *emit_spm_needed = spm_update_needed; + if (spm_update_needed && emit_fence) + *emit_spm_needed = false; + + *emit_gds_needed = gds_switch_needed; + if (gds_switch_needed && emit_fence) + *emit_gds_needed = false; + if (!vm_flush_needed && !gds_switch_needed && !need_pipe_sync && !cleaner_shader_needed && !spm_update_needed) return; @@@ -845,22 -861,22 +861,22 @@@ if (pasid_mapping_needed) amdgpu_gmc_emit_pasid_mapping(ring, job->vmid, job->pasid); - if (spm_update_needed && adev->gfx.rlc.funcs->update_spm_vmid) - adev->gfx.rlc.funcs->update_spm_vmid(adev, ring->xcc_id, ring, job->vmid); + if (emit_fence) { + if (spm_update_needed) + adev->gfx.rlc.funcs->update_spm_vmid(adev, ring->xcc_id, ring, job->vmid); - if (ring->funcs->emit_gds_switch && - gds_switch_needed) { - amdgpu_ring_emit_gds_switch(ring, job->vmid, job->gds_base, - job->gds_size, job->gws_base, - job->gws_size, job->oa_base, - job->oa_size); + if (gds_switch_needed) + amdgpu_ring_emit_gds_switch(ring, job->vmid, job->gds_base, + job->gds_size, job->gws_base, + job->gws_size, job->oa_base, + job->oa_size); + + amdgpu_fence_emit(ring, job->hw_vm_fence, 0); + fence = &job->hw_vm_fence->base; + /* get a ref for the job */ + dma_fence_get(fence); } - amdgpu_fence_emit(ring, job->hw_vm_fence, 0); - fence = &job->hw_vm_fence->base; - /* get a ref for the job */ - dma_fence_get(fence); - if (vm_flush_needed) { mutex_lock(&id_mgr->lock); dma_fence_put(id->last_flush); @@@ -893,7 -909,12 +909,12 @@@ amdgpu_ring_patch_cond_exec(ring, patch); - /* the double SWITCH_BUFFER here *cannot* be skipped by COND_EXEC */ + /* + * Sync CE with ME to prevent CE from fetching the next CE IB + * before the context switch is done. This is emitted before + * the first IB of a job submission after a context switch. + * The double SWITCH_BUFFER here *cannot* be skipped by COND_EXEC. + */ if (ring->funcs->emit_switch_buffer) { amdgpu_ring_emit_switch_buffer(ring); amdgpu_ring_emit_switch_buffer(ring); @@@ -1387,7 -1408,13 +1408,13 @@@ int amdgpu_vm_bo_update(struct amdgpu_d amdgpu_vm_bo_evicted(&bo_va->base); else amdgpu_vm_bo_idle(&bo_va->base); - } else { + } else if (bo) { + /* + * A PRT/sparse mapping has no BO and is kept off the vm_bo + * state lists (see amdgpu_vm_bo_base_init()); putting it on the + * idle list here would let amdgpu_vm_handle_moved() dereference + * the NULL bo after a reset. + */ amdgpu_vm_bo_idle(&bo_va->base); } @@@ -2507,14 -2534,16 +2534,16 @@@ amdgpu_vm_get_task_info_vm(struct amdgp struct amdgpu_task_info * amdgpu_vm_get_task_info_pasid(struct amdgpu_device *adev, u32 pasid) { + struct amdgpu_fpriv *fpriv; struct amdgpu_task_info *ti; struct amdgpu_vm *vm; unsigned long flags; - xa_lock_irqsave(&adev->vm_manager.pasids, flags); - vm = xa_load(&adev->vm_manager.pasids, pasid); + amdgpu_pasid_lock(&flags); + fpriv = amdgpu_pasid_get_fpriv_locked(pasid); + vm = fpriv ? &fpriv->vm : NULL; ti = amdgpu_vm_get_task_info_vm(vm); - xa_unlock_irqrestore(&adev->vm_manager.pasids, flags); + amdgpu_pasid_unlock(flags); return ti; } @@@ -2555,7 -2584,6 +2584,6 @@@ void amdgpu_vm_set_task_info(struct amd * @adev: amdgpu_device pointer * @vm: requested vm * @xcp_id: GPU partition selection id - * @pasid: the pasid the VM is using on this GPU * * Init @vm fields. * @@@ -2563,7 -2591,7 +2591,7 @@@ * 0 for success, error for failure. */ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, - int32_t xcp_id, uint32_t pasid) + int32_t xcp_id) { struct amdgpu_bo *root_bo; struct amdgpu_bo_vm *root; @@@ -2638,26 -2666,12 +2666,12 @@@ if (r) dev_dbg(adev->dev, "Failed to create task info for VM\n"); - /* Store new PASID in XArray (if non-zero) */ - if (pasid != 0) { - r = xa_err(xa_store_irq(&adev->vm_manager.pasids, pasid, vm, GFP_KERNEL)); - if (r < 0) - goto error_free_root; - - vm->pasid = pasid; - } - amdgpu_bo_unreserve(vm->root.bo); amdgpu_bo_unref(&root_bo); return 0; error_free_root: - /* If PASID was partially set, erase it from XArray before failing */ - if (vm->pasid != 0) { - xa_erase_irq(&adev->vm_manager.pasids, vm->pasid); - vm->pasid = 0; - } amdgpu_vm_pt_free_root(adev, vm); amdgpu_bo_unreserve(vm->root.bo); amdgpu_bo_unref(&root_bo); @@@ -2764,11 -2778,6 +2778,6 @@@ void amdgpu_vm_fini(struct amdgpu_devic root = amdgpu_bo_ref(vm->root.bo); amdgpu_bo_reserve(root, true); - /* Remove PASID mapping before destroying VM */ - if (vm->pasid != 0) { - xa_erase_irq(&adev->vm_manager.pasids, vm->pasid); - vm->pasid = 0; - } dma_fence_wait(vm->last_unlocked, false); dma_fence_put(vm->last_unlocked); dma_fence_wait(vm->last_tlb_flush, false); @@@ -2864,8 -2873,6 +2873,6 @@@ void amdgpu_vm_manager_init(struct amdg #else adev->vm_manager.vm_update_mode = 0; #endif - - xa_init_flags(&adev->vm_manager.pasids, XA_FLAGS_LOCK_IRQ); } /** @@@ -2877,9 -2884,6 +2884,6 @@@ */ void amdgpu_vm_manager_fini(struct amdgpu_device *adev) { - WARN_ON(!xa_empty(&adev->vm_manager.pasids)); - xa_destroy(&adev->vm_manager.pasids); - amdgpu_vmid_mgr_fini(adev); amdgpu_pasid_mgr_cleanup(); } @@@ -2936,14 -2940,16 +2940,16 @@@ struct amdgpu_vm *amdgpu_vm_lock_by_pas u32 pasid, struct drm_exec *exec) { unsigned long irqflags; + struct amdgpu_fpriv *fpriv; struct amdgpu_bo *root; struct amdgpu_vm *vm; int r; - xa_lock_irqsave(&adev->vm_manager.pasids, irqflags); - vm = xa_load(&adev->vm_manager.pasids, pasid); - root = vm ? amdgpu_bo_ref(vm->root.bo) : NULL; - xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags); + amdgpu_pasid_lock(&irqflags); + fpriv = amdgpu_pasid_get_fpriv_locked(pasid); + vm = fpriv ? &fpriv->vm : NULL; + root = vm && vm->root.bo ? amdgpu_bo_ref(vm->root.bo) : NULL; + amdgpu_pasid_unlock(irqflags); if (!root) return NULL; @@@ -2955,11 -2961,17 +2961,17 @@@ } /* Double check that the VM still exists */ - xa_lock_irqsave(&adev->vm_manager.pasids, irqflags); - vm = xa_load(&adev->vm_manager.pasids, pasid); - if (vm && vm->root.bo != root) + amdgpu_pasid_lock(&irqflags); + fpriv = amdgpu_pasid_get_fpriv_locked(pasid); + if (!fpriv) { vm = NULL; - xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags); + } else { + vm = &fpriv->vm; + if (vm->root.bo != root) + vm = NULL; + } + amdgpu_pasid_unlock(irqflags); + if (!vm) { drm_exec_unlock_obj(exec, &root->tbo.base); amdgpu_bo_unref(&root); @@@ -3011,8 -3023,6 +3023,8 @@@ bool amdgpu_vm_handle_fault(struct amdg is_compute_context = vm->is_compute_context; if (is_compute_context) { + __label__ drm_exec_retry; + /* Release the root PD lock since svm_range_restore_pages * might try to take it. * TODO: rework svm_range_restore_pages so that this isn't @@@ -3158,12 -3168,14 +3170,14 @@@ void amdgpu_vm_update_fault_cache(struc uint32_t status, unsigned int vmhub) { + struct amdgpu_fpriv *fpriv; struct amdgpu_vm *vm; unsigned long flags; - xa_lock_irqsave(&adev->vm_manager.pasids, flags); + amdgpu_pasid_lock(&flags); - vm = xa_load(&adev->vm_manager.pasids, pasid); + fpriv = amdgpu_pasid_get_fpriv_locked(pasid); + vm = fpriv ? &fpriv->vm : NULL; /* Don't update the fault cache if status is 0. In the multiple * fault case, subsequent faults will return a 0 status which is * useless for userspace and replaces the useful fault status, so @@@ -3196,7 -3208,7 +3210,7 @@@ WARN_ONCE(1, "Invalid vmhub %u\n", vmhub); } } - xa_unlock_irqrestore(&adev->vm_manager.pasids, flags); + amdgpu_pasid_unlock(flags); } void amdgpu_vm_print_task_info(struct amdgpu_device *adev,