| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <762bb5f4-f9f5-4ebc-84d3-5465df3ffbf6@amd.com>
Date: Mon, 27 Nov 2023 14:52:51 +0100
From: Christian König <christian.koenig@....com>
To: Danilo Krummrich <dakr@...hat.com>
Cc: mripard@...nel.org, airlied@...il.com, daniel@...ll.ch,
frank.binns@...tec.com, donald.robson@...tec.com,
matt.coster@...tec.com, sarah.walker@...tec.com,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH drm-misc-next 4/5] drm/gpuvm: fall back to
drm_exec_lock_obj()
Am 25.11.23 um 00:40 schrieb Danilo Krummrich:
> Hi Christian,
>
> do you think it makes sense to handle this in drm_exec_prepare_obj() or
> even dma_resv_reserve_fences() even?
IIRC for drm_exec the discussion has gone a bit back and forth between
handling 0 and having a separate function which doesn't allocate fences.
We ended up with the solution using separate calls since you either know
that you don't need fences (because you for example only need to look
something up) or you need fences and eventually calculate the number you
need dynamically and if you then end up with 0 it's a bug.
So to sum it up the conclusion was that it's more defensive to complain
about 0 fences to reserve (which reminds me that
dma_resv_reserve_fences() still needs to get a warning for 0 fences as
well).
Regards,
Christian.
>
> - Danilo
>
> On 11/25/23 00:36, Danilo Krummrich wrote:
>> Fall back to drm_exec_lock_obj() if num_fences is zero for the
>> drm_gpuvm_prepare_* function family.
>>
>> Otherwise dma_resv_reserve_fences() would actually allocate slots even
>> though num_fences is zero.
>>
>> Cc: Christian König <christian.koenig@....com>
>> Signed-off-by: Danilo Krummrich <dakr@...hat.com>
>> ---
>> drivers/gpu/drm/drm_gpuvm.c | 36 +++++++++++++++++++++++++++++++++---
>> include/drm/drm_gpuvm.h | 23 +++--------------------
>> 2 files changed, 36 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
>> index 54f5e8851de5..d1d1c2379e44 100644
>> --- a/drivers/gpu/drm/drm_gpuvm.c
>> +++ b/drivers/gpu/drm/drm_gpuvm.c
>> @@ -1085,6 +1085,36 @@ drm_gpuvm_put(struct drm_gpuvm *gpuvm)
>> }
>> EXPORT_SYMBOL_GPL(drm_gpuvm_put);
>> +static int
>> +exec_prepare_obj(struct drm_exec *exec, struct drm_gem_object *obj,
>> + unsigned int num_fences)
>> +{
>> + return num_fences ? drm_exec_prepare_obj(exec, obj, num_fences) :
>> + drm_exec_lock_obj(exec, obj);
>> +}
>> +
>> +/**
>> + * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
>> + * @gpuvm: the &drm_gpuvm
>> + * @exec: the &drm_exec context
>> + * @num_fences: the amount of &dma_fences to reserve
>> + *
>> + * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object.
>> + *
>> + * Using this function directly, it is the drivers responsibility to
>> call
>> + * drm_exec_init() and drm_exec_fini() accordingly.
>> + *
>> + * Returns: 0 on success, negative error code on failure.
>> + */
>> +int
>> +drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
>> + struct drm_exec *exec,
>> + unsigned int num_fences)
>> +{
>> + return exec_prepare_obj(exec, gpuvm->r_obj, num_fences);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_vm);
>> +
>> static int
>> __drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
>> struct drm_exec *exec,
>> @@ -1095,7 +1125,7 @@ __drm_gpuvm_prepare_objects(struct drm_gpuvm
>> *gpuvm,
>> int ret = 0;
>> for_each_vm_bo_in_list(gpuvm, extobj, &extobjs, vm_bo) {
>> - ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences);
>> + ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
>> if (ret)
>> break;
>> }
>> @@ -1116,7 +1146,7 @@ drm_gpuvm_prepare_objects_locked(struct
>> drm_gpuvm *gpuvm,
>> drm_gpuvm_resv_assert_held(gpuvm);
>> list_for_each_entry(vm_bo, &gpuvm->extobj.list,
>> list.entry.extobj) {
>> - ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences);
>> + ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
>> if (ret)
>> break;
>> @@ -1186,7 +1216,7 @@ drm_gpuvm_prepare_range(struct drm_gpuvm
>> *gpuvm, struct drm_exec *exec,
>> drm_gpuvm_for_each_va_range(va, gpuvm, addr, end) {
>> struct drm_gem_object *obj = va->gem.obj;
>> - ret = drm_exec_prepare_obj(exec, obj, num_fences);
>> + ret = exec_prepare_obj(exec, obj, num_fences);
>> if (ret)
>> return ret;
>> }
>> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
>> index f94fec9a8517..b3f82ec7fb17 100644
>> --- a/include/drm/drm_gpuvm.h
>> +++ b/include/drm/drm_gpuvm.h
>> @@ -544,26 +544,9 @@ struct drm_gpuvm_exec {
>> } extra;
>> };
>> -/**
>> - * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
>> - * @gpuvm: the &drm_gpuvm
>> - * @exec: the &drm_exec context
>> - * @num_fences: the amount of &dma_fences to reserve
>> - *
>> - * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object.
>> - *
>> - * Using this function directly, it is the drivers responsibility to
>> call
>> - * drm_exec_init() and drm_exec_fini() accordingly.
>> - *
>> - * Returns: 0 on success, negative error code on failure.
>> - */
>> -static inline int
>> -drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
>> - struct drm_exec *exec,
>> - unsigned int num_fences)
>> -{
>> - return drm_exec_prepare_obj(exec, gpuvm->r_obj, num_fences);
>> -}
>> +int drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
>> + struct drm_exec *exec,
>> + unsigned int num_fences);
>> int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
>> struct drm_exec *exec,
>
Powered by blists - more mailing lists