From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 57598C52D7C for ; Mon, 12 Aug 2024 18:17:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:CC:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=9Nfz751ACTLyecrv87bbUTwqsi6Lp/AjbSXzAYjq7/o=; b=zCS0t8Nxr1BIbN1tK5T4Cipvxq hD3Cqw/mMNG15VN+/5B88BPg8uUP53+JnRDjr3U6rQ6QJ+iNZriIKRXqyqieSgYpE93kLtoT8WPlN IsO5Tdo65eb5Jrgh8fmFXNd0aHbR56OtpB/VWTY7GsYlhDBh6qBjSCyH5VAuzekAsKzsjNiGh/H7m 9uDwo6gLev0xcCND1+EmlytivV12n7rfj7dcUEkaffVIUyIRcr+Tcu93fT4T+MOSaPNbJeET6b9ou Kt7yA0zQOSUvmvZjSQD4D90CYcZy1Ev3WsvFWtTKV/b40vePPj8hFsS1iz8JbU/vbt4B0wboXhPn0 hFQDeaqw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdZbR-00000001AJE-1XZG; Mon, 12 Aug 2024 18:17:09 +0000 Received: from mx0b-0031df01.pphosted.com ([205.220.180.131]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdZap-00000001AAI-3W5i for linux-arm-kernel@lists.infradead.org; Mon, 12 Aug 2024 18:16:33 +0000 Received: from pps.filterd (m0279869.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 47CDSbqR017504; Mon, 12 Aug 2024 18:16:21 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= 9Nfz751ACTLyecrv87bbUTwqsi6Lp/AjbSXzAYjq7/o=; b=M80I8SF8/nOImR79 IPyyEQcbiyhDOvMSishoFSeblQ/6NRZJVMuhir8x4oOKuTSAn2MIxE1z+mFepJRj NiIFSBivBSIvAmFagHS1zl/BXjDacbVC03NlSwBm/J1bOc0x24pnQ9habYAvyf4q ND8MXOAPIdYTXLYHTBOgM1YqSBtAOmMsXMj0xb7j3vKG35EqYbbrsbMq5Ggslzky NEzJ7tydbstkAzqgVWMvviQbCrbze5leUCr16xm8y9H/RFycaoCdnRVYIGMEkihg RmQ5Cek9IeztYxNTRw7CmBk60zZeVeQWIR6MbP+/AZ1adVNw2FU6SxsPZDKolRa/ Mc2M6w== Received: from nalasppmta05.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 40x1d4d1t1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 12 Aug 2024 18:16:20 +0000 (GMT) Received: from nalasex01c.na.qualcomm.com (nalasex01c.na.qualcomm.com [10.47.97.35]) by NALASPPMTA05.qualcomm.com (8.18.1.2/8.18.1.2) with ESMTPS id 47CIGJ8n005714 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 12 Aug 2024 18:16:19 GMT Received: from [10.216.3.204] (10.80.80.8) by nalasex01c.na.qualcomm.com (10.47.97.35) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.9; Mon, 12 Aug 2024 11:16:12 -0700 Message-ID: <28c8bc92-4a55-8a07-1ece-333316d78410@quicinc.com> Date: Mon, 12 Aug 2024 23:46:08 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH v5 3/4] firmware: psci: Read and use vendor reset types To: Elliot Berman , Lorenzo Pieralisi CC: Bjorn Andersson , Konrad Dybcio , Sebastian Reichel , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Vinod Koul , Andy Yan , Mark Rutland , "Bartosz Golaszewski" , Satya Durga Srinivasu Prabhala , Melody Olvera , , , , Florian Fainelli , , , References: <20240617-arm-psci-system_reset2-vendor-reboots-v5-0-086950f650c8@quicinc.com> <20240617-arm-psci-system_reset2-vendor-reboots-v5-3-086950f650c8@quicinc.com> <20240807103245593-0700.eberman@hu-eberman-lv.qualcomm.com> <20240809090339647-0700.eberman@hu-eberman-lv.qualcomm.com> Content-Language: en-US From: Shivendra Pratap In-Reply-To: <20240809090339647-0700.eberman@hu-eberman-lv.qualcomm.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01c.na.qualcomm.com (10.47.97.35) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: ollusWuQeFA-k-mEjm9dK-3rv8W2pqPr X-Proofpoint-GUID: ollusWuQeFA-k-mEjm9dK-3rv8W2pqPr X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16 definitions=2024-08-12_12,2024-08-12_02,2024-05-17_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 impostorscore=0 adultscore=0 phishscore=0 suspectscore=0 mlxscore=0 mlxlogscore=999 malwarescore=0 bulkscore=0 priorityscore=1501 clxscore=1011 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2407110000 definitions=main-2408120135 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240812_111632_024610_5CFAE904 X-CRM114-Status: GOOD ( 26.57 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 8/9/2024 10:28 PM, Elliot Berman wrote: > On Fri, Aug 09, 2024 at 03:30:38PM +0200, Lorenzo Pieralisi wrote: >> On Wed, Aug 07, 2024 at 11:10:50AM -0700, Elliot Berman wrote: >> >> [...] >> >>>>> +static void psci_vendor_sys_reset2(unsigned long action, void *data) >>>> >>>> 'action' is unused and therefore it is not really needed. >>>> >>>>> +{ >>>>> + const char *cmd = data; >>>>> + unsigned long ret; >>>>> + size_t i; >>>>> + >>>>> + for (i = 0; i < num_psci_reset_params; i++) { >>>>> + if (!strcmp(psci_reset_params[i].mode, cmd)) { >>>>> + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), >>>>> + psci_reset_params[i].reset_type, >>>>> + psci_reset_params[i].cookie, 0); >>>>> + pr_err("failed to perform reset \"%s\": %ld\n", >>>>> + cmd, (long)ret); >>>>> + } >>>>> + } >>>>> +} >>>>> + >>>>> static int psci_sys_reset(struct notifier_block *nb, unsigned long action, >>>>> void *data) >>>>> { >>>>> + if (data && num_psci_reset_params) >>>> >>>> So, reboot_mode here is basically ignored; if there is a vendor defined >>>> reset, we fire it off. >>>> >>>> I think Mark mentioned his concerns earlier related to REBOOT_* mode and >>>> reset type (granted, the context was different): >>>> >>>> https://lore.kernel.org/all/20200320120105.GA36658@C02TD0UTHF1T.local/ >>>> >>>> I would like to understand if this is the right thing to do before >>>> accepting this patchset. >>>> >>> >>> I don't have any concerns to move this part below checking reboot_mode. >>> Or, I could add reboot_mode == REBOOT_COLD check. >> >> The question is how can we map vendor specific reboot magic to Linux >> reboot modes sensibly in generic PSCI code - that's by definition >> vendor specific. >> > > I don't think it's a reasonable thing to do. "reboot bootloader" or > "reboot edl" don't make sense to the Linux reboot modes. > > I believe the Linux reboot modes enum is oriented to perspective of > Linux itself and the vendor resets are oriented towards behavior of the > SoC. > > Thanks, > Elliot > Agree. from perspective of linux reboot modes, kernel's current implementation in reset path is like: __ #1 If reboot_mode is WARM/SOFT and PSCI_SYSRESET2 is supported Call PSCI - SYSTEM_RESET2 - ARCH RESET #2 ELSE Call PSCI - SYSTEM_RESET COLD RESET ___ ARM SPECS for PSCI SYSTEM_RESET2 This function extends SYSTEM_RESET. It provides: • ARCH RESET: set Bit[31] to 0 = > This is already in place in condition #1. • vendor-specific resets: set Bit[31] to 1. = > current patchset adds this part before kernel's reboot_mode reset at #0. In current patchset, we see a condition added at #0-psci_vendor_reset2 being called before kernel’s current reboot_mode condition and it can take any action only if all below conditions are satisfied. - PSCI SYSTEM_RESET2 is supported. - psci dt node defines an entry "bootloader" as a reboot-modes. - User issues reboot with a command say - (reboot bootloader). - If vendor reset fails, default reboot mode will execute as is. Don't see if we will skip or break the kernel reboot_mode flow with this patch. Also if user issues reboot and is supported on SOC vendor reset psci node, should cmd take precedence over kernel reboot mode enum? may be yes?