From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 37F271107 for ; Mon, 28 Aug 2023 21:35:30 +0000 (UTC) Received: from pps.filterd (m0279867.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 37SLXZBh000646; Mon, 28 Aug 2023 21:35:12 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=qcppdkim1; bh=QX0IWt4Uo65a2ZO0nHly3/Zk1oxm5PDDbCMTrurBS8w=; b=LfWFB3Ljlm/8g8PK+3/AA1WFGlOwxDRvP/WJ6jJUj65h3YLD5JXUm/fU5gG1WLyraVov TP2DHn+UK6fZj/1DDMc2JboBUzzZmaliTeSD8BSZybX+uUwyK+6eu0SrRETriwP16AQz j6hCRYMRowQLSiXUbCaKnKPy0SL9Qyvq9l6SUnBzNgM7qdZ5y6DuS7WmMN1n+ryut5Ls 9CZNp+Q4SBY/cd1dGxEr2Y0e7gtxq45BaV4mU7ZkxJVI1s0SflW0dE/pvaWMdy+3R886 po4N45yy3FpdHaAnuyauATRF4eDUwCC3hdRKOifRCmC2GYGPNXMSPoNpvQzBYdBhkg+9 2Q== Received: from nasanppmta01.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3ss3fr00dn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Aug 2023 21:35:11 +0000 Received: from nasanex01a.na.qualcomm.com (nasanex01a.na.qualcomm.com [10.52.223.231]) by NASANPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 37SLZBnF002730 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Aug 2023 21:35:11 GMT Received: from [10.110.112.60] (10.80.80.8) by nasanex01a.na.qualcomm.com (10.52.223.231) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.36; Mon, 28 Aug 2023 14:35:10 -0700 Message-ID: Date: Mon, 28 Aug 2023 14:35:09 -0700 Precedence: bulk X-Mailing-List: iommu@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.2 Subject: Re: [PATCH] iommu/arm-smmu-qcom: NULL pointer check for driver data Content-Language: en-US To: Rob Clark CC: , , , , , , , , , , , , , , , References: <20230816225509.11070-1-quic_aprasann@quicinc.com> From: Aravind Vijayakumar In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01a.na.qualcomm.com (10.52.223.231) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: Zh__iUlPR0h4jW3lq5ydSAoJjAQ-h5_n X-Proofpoint-ORIG-GUID: Zh__iUlPR0h4jW3lq5ydSAoJjAQ-h5_n X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.601,FMLib:17.11.176.26 definitions=2023-08-28_18,2023-08-28_04,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 mlxscore=0 adultscore=0 spamscore=0 phishscore=0 bulkscore=0 mlxlogscore=999 impostorscore=0 suspectscore=0 lowpriorityscore=0 malwarescore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2308100000 definitions=main-2308280186 On 8/16/2023 6:01 PM, Rob Clark wrote: > On Wed, Aug 16, 2023 at 3:55 PM Aravind Vijayakumar > wrote: >> The driver_data is NULL when qcom_adreno_smmu_init_context() >> is called before the dev_set_drvdata() from the client driver >> and is resulting in kernel crash. >> >> So add a null pointer check to handle the scenario >> where the client driver for the GPU SMMU device would >> be setting the driver data after the smmu client device >> probe is done and not necessarily before that. The function >> qcom_adreno_smmu_init_context() assumes that the client >> driver always set the driver data using dev_set_drvdata() >> before the smmu client device probe, but this assumption >> is not always true. >> >> Signed-off-by: Aravind Vijayakumar >> --- >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> index c71afda79d64..5323f82264ca 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> @@ -231,6 +231,9 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain, >> */ >> >> priv = dev_get_drvdata(dev); >> + if (!priv) >> + return 0; > could this -EPROBE_DEFER instead, or something like that? I think you > patch as proposed would result in per-process gpu pgtables silently > failing > > BR, > -R Thanks for the review comments. Returning -EPROBE_DEFER wont work because the probe of the client driver (which sets the driver data) will never get triggered. However, the probe of the client driver succeeds if we return -ENODATA. would that be acceptable? Regards, Aravind >> + >> priv->cookie = smmu_domain; >> priv->get_ttbr1_cfg = qcom_adreno_smmu_get_ttbr1_cfg; >> priv->set_ttbr0_cfg = qcom_adreno_smmu_set_ttbr0_cfg; >> -- >> 2.40.1 >>