On 2022-12-13 02:00, Christian König wrote: > Am 13.12.22 um 00:44 schrieb Luben Tuikov: >> On 2022-12-12 14:19, Christian König wrote: >>> Am 12.12.22 um 18:48 schrieb Luben Tuikov: >>>> Fix amdgpu_bo_validate_size() to check whether the TTM domain manager for the >>>> requested memory exists, and to allow for non-exclusive domain allocations, as >>>> there would be if the domain is a mask, e.g. AMDGPU_GEM_DOMAIN_VRAM | >>>> AMDGPU_GEM_DOMAIN_GTT. >>>> >>>> Cc: Alex Deucher <Alexander.Deucher@xxxxxxx> >>>> Cc: Christian König <christian.koenig@xxxxxxx> >>>> Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 19 +++++++------------ >>>> 1 file changed, 7 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>>> index fd3ab4b5e5bb1f..e0f103f0ec2178 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>>> @@ -448,31 +448,26 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev, >>>> >>>> /* >>>> * If GTT is part of requested domains the check must succeed to >>>> - * allow fall back to GTT >>>> + * allow fall back to GTT. >>>> + * >>>> + * Note that allocations can request from either domain. For >>>> + * this reason, check either in non-exclusive way, and if >>>> + * neither satisfies, fail the validation. >>> That's not correct, the original logic was completely intentional. >>> >>> If both VRAM and GTT are specified it's valid if the size fits only into >>> GTT. >> Given that this patch fixes a kernel oops, should this patch then fail the validation, >> i.e. return false? > > It should be sufficient if a BO fits into the GTT domain for size > validation. If we haven't initialized the GTT domain and end up here we > should probably just ignore it. By "should probably just ignore it" do you mean "return true;" or "return false;"? > >> >> This would then fail, in amdgpu_ttm_reserve_tmr(): >> >> ret = amdgpu_bo_create_kernel_at(adev, >> adev->gmc.real_vram_size - adev->mman.discovery_tmr_size, >> adev->mman.discovery_tmr_size, >> AMDGPU_GEM_DOMAIN_VRAM | >> AMDGPU_GEM_DOMAIN_GTT, > > As I said before using amdgpu_bo_create_kernel_at() with VRAM|GTT > doesn't make any sense at all. We should probably drop the domain > parameter altogether. I agree. We can do this in another patch. Regards, Luben > > Regards, > Christian. > >> &adev->mman.discovery_memory, >> NULL); >> >> Regards, >> Luben >> >
