Received-SPF: pass (google.com: domain of linux-kernel+bounces-34531-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <713eaa19-33b9-40fe-a3a5-69e45d671399@amd.com>
Date: Mon, 22 Jan 2024 19:40:14 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [BUG] BUG: kernel NULL pointer dereference at
 ttm_device_init+0xb4
Content-Language: en-US
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
 Linus Torvalds <torvalds@linux-foundation.org>,
 Felix Kuehling <Felix.Kuehling@amd.com>,
 =?UTF-8?Q?Christian_K=C3=B6nig?= <christian.koenig@amd.com>,
 dri-devel@lists.freedesktop.org
References: <20240122180605.28daf23a@gandalf.local.home>
 <918ddc13-1cf8-4199-b5be-5719a023de42@amd.com>
 <20240122193453.6096d45f@gandalf.local.home>
From: "Bhardwaj, Rajneesh" <rajneesh.bhardwaj@amd.com>
In-Reply-To: <20240122193453.6096d45f@gandalf.local.home>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?dzl6bXg5am1mMWMrU0t4dS9FaFNpUzdHNXgyMUViUVorcVlJRW1mU0pjZmRM?=
 =?utf-8?B?ODZGbE5HcnlmV3c2NnlldGVzRlZCUmo4a1FKdllPdC9tTHR4S0ZwWUo5alU3?=
 =?utf-8?B?WHFac2tPaWgrZHJIeUZFWXBGbDZEWmJzTldZWnFBUDRDdWNDaDZtSVNWSEhQ?=
 =?utf-8?B?R0RXbEpRdUhIWlFXd0N1Q3Z5M3FDMC9kVjcyenJjUUlFTEdEUmJYU2lQSnY2?=
 =?utf-8?B?enUveDlzOFVRalpYNk56Q1U5OXpUTlpaR0ZNcGJiZ1RCN25XZXMyVkVDa2RY?=
 =?utf-8?B?U2RyUFkxZVpSSm5mRjJlSFRWNUhMYnpyL3daSXVEZ3FBa1hBRmg1SDNpdEV5?=
 =?utf-8?B?TURFNE9hVEltOG1nZFN0OEdzbzJCN0Z4RDZJV0daaVJJYWozSUhyRDI3d0tR?=
 =?utf-8?B?WmRBVEVibVJwT1Jya0tSeWMvbHNJcWNsOUtmeGNLQXRNQWQ5Vmg5alN5WERz?=
 =?utf-8?B?NjVBSi9ON210WXcxd1EyeTJtWFVjL041bXZCRHJvUGJmODhHbEh4WHdxajdn?=
 =?utf-8?B?Vk5JZGpRcXpHVDFSTXd0dFJSa1NJREhyMFY5T2lMZWhpOGFYbXlnTmRnd3p1?=
 =?utf-8?B?SmY5Z1h4U09QbmYwVXRQRkxaMHc2TW9PeTliRU5nMVNvUi9mRTNGZERsQUdQ?=
 =?utf-8?B?V0NOZjZQOFp4elNTUXB0bEFWNzFuaWtHV2dTaG8xbitHMWxQYlVFOWZWMDR1?=
 =?utf-8?B?a1pOSitTbHErYkx3bGgrUVZUUVNkK2ZONlhiYUhPYTVrL0svY3A3S2lTc21j?=
 =?utf-8?B?TVFnMm81bnRrT1FDRVpZbFN5TWhNQ0NZSEx1TG5qZVFXZWkvcmcxcjFzTlNl?=
 =?utf-8?B?UC9lQjBHK2NTNDlHZ1g2d0E0RXFYZTlGZm9zS0ZzM2lRVmF2bFA3WVNQci9r?=
 =?utf-8?B?bE9OeHpLSjhpWlRPaG9tRVFPSUFtMXNsLzNDNkR2MU1SSGRKdW1UcVBMUWUv?=
 =?utf-8?B?cVcwb3ZjcnJmTEYvYTlNRUNzcUR1MlZXb01vUU5IaC9HcmUrSDQ2Uk96VFJw?=
 =?utf-8?B?cTArME4rVFZjN1N1TmdYV0pRWW9tY0EydnJTOFd6eGRIMFJYNjR2b3NCb1JT?=
 =?utf-8?B?L2ZQMytFY2M0bzVmeXFSMklrYVJDK1ltU2dFSzRVTGozSXJmSXZDZWYyV2Q3?=
 =?utf-8?B?eUJ2RjA3WHNCTUswWW1JcEdNZXZqZE13R0xVZWpTSzR2Mm5hVkhGWE9qNC83?=
 =?utf-8?B?Qm1QVE1DMk4wdnJ2SGlJM1l1L0dyZjZuZkJwYmhzTDVSSWptdzl6bFlvZG9q?=
 =?utf-8?B?R2F0WS9ERDROSGFRZzFxUmNZdFJUbUdmRUZoSmk1eTczbVFFb1B1RGhCRTY3?=
 =?utf-8?B?Rm9mRml1NXJQWFYxNUMwSkFQUkFKb3l1amNjekdNT043TDBpeG5PYkJGdkk3?=
 =?utf-8?B?M20wZmlNNkl0YTdzNU1SNDdhZzRZK0FwcGxuNXJMRmxGUzNJR0s2K0NYcVgy?=
 =?utf-8?B?SUVxVTl5aUN0NlFrL1BaUHd6SDhCL3EzQ1hvQmlMa3ZSY3BJWW01NXlvQThL?=
 =?utf-8?B?NWRacW01RFVyN1ZwbHMybGcrMkZzbTM5TWFnWVpzVXZWV3Y0WnQvU1BwYktQ?=
 =?utf-8?B?eUtxUWpxd0J0SDZZU2piU1JsOCtBQ3dodk9kWkphRmhCOE5YcTVuTGxXb3ht?=
 =?utf-8?B?dlFqSjRCaFJpaDhveTZveE9kR2FaWlBOLzRXUUpXMzJGNTRmc0xObXNkL09V?=
 =?utf-8?B?QW40UEF1UzZTbHU5REppMnNlNWF4WXMxUnJ1NklNcnd4YW1EMkdzeExHelJW?=
 =?utf-8?B?WUdyYzV0OVNOTCtaT3YrNzE2eTNOK0FkMlFKNW5kWVhpNHNFSnkzcHVYdkZw?=
 =?utf-8?B?RHY0SDFCVVZ2TzBqUUh4dmIrb0xDNWt6S2hQZEJWOWd4QXF0SC8yZUUrNGFi?=
 =?utf-8?B?QnZuTUdhSnJCdjA5T0x1Q0o3RENraDZZLzg0VmtFZHFQWm5GQlJOV0U5MVdP?=
 =?utf-8?B?eDNuSUpYeDNSNG0yR3hYZG92OXFqS2FVaUtWWUJDTWdHSjNHdjNDUTM2SlNu?=
 =?utf-8?B?SnpsUVdDdVptSzZCTjhFcTFrN0VlTno5MktQa0FnZDN6cmRsVEozRkpIZHk2?=
 =?utf-8?B?NUV3cnJBSDl0MEZlcUo2Vmg2akU4VVViMWF1RW44dy9XZTE0aW9vbHU2bjNW?=
 =?utf-8?Q?K1RtHvSbkYlc/0sAXBGBTcSa9?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4d7893d0-9e87-454b-6eec-08dc1babdf62
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5933.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2024 00:40:18.2033
 (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: 18efQUroYKi7belIjIAfLdRrJI8vxy5bl/XOuSVXIrExvMEHBQjuiJ1wHZAlYyp1syX7kUgma5Fast08luoEQA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB8184


On 1/22/2024 7:34 PM, Steven Rostedt wrote:
> On Mon, 22 Jan 2024 19:29:41 -0500
> "Bhardwaj, Rajneesh" <rajneesh.bhardwaj@amd.com> wrote:
>
>> In one of my previous revisions of this patch when I was experimenting,
>> I used something like below. Wonder if that could work in your case
>> and/or in general.
>>
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
>> b/drivers/gpu/drm/ttm/ttm_device.c
>>
>> index 43e27ab77f95..4c3902b94be4 100644
>>
>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>
>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>
>> @@ -195,6 +195,7 @@ int ttm_device_init(struct ttm_device *bdev, struct
>> ttm_device_funcs *funcs,
>>
>> bool use_dma_alloc, bool use_dma32){
>>
>> struct ttm_global *glob = &ttm_glob;
>>
>> +bool node_has_cpu = false;
>>
>> int ret;
>>
>> if (WARN_ON(vma_manager == NULL))
>>
>> @@ -213,7 +214,12 @@ int ttm_device_init(struct ttm_device *bdev, struct
>> ttm_device_funcs *funcs,
>>
>> bdev->funcs = funcs;
>>
>> ttm_sys_man_init(bdev);
>>
>> -ttm_pool_init(&bdev->pool, dev, NUMA_NO_NODE, use_dma_alloc, use_dma32);
>>
>> +
>>
>> +node_has_cpu = node_state(dev->numa_node, N_CPU);
> Considering that qxl_ttm_init() passes in dev = NULL, the above would blow
> up just the same.


I agree, I think we need something like you suggested i.e.

+	ttm_pool_init(&bdev->pool, dev, dev ? dev_to_node(dev) : NUMA_NO_NODE,
+		      use_dma_alloc, use_dma32);


I am not quite sure if the above node_has_cpu change will be a better 
solution in general, along with the NULL pointer check as you suggested. 
If you prefer that, then I can send a fix otherwise, your fix looks good 
to me.


>
> -- Steve
>
>
>> +if (node_has_cpu)
>>
>> +ttm_pool_init(&bdev->pool, dev, dev->numa_node, use_dma_alloc, use_dma32);
>>
>> +else
>>
>> +ttm_pool_init(&bdev->pool, dev, NUMA_NO_NODE, use_dma_alloc,
>>
>> +use_dma32);
>>
>> bdev->vma_manager = vma_manager;
>>
>> spin_lock_init(&bdev->lru_lock);
>>
>>
>>> -- Steve