Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Wed, 01 May 2019 11:43:00 -0700
From:   Sodagudi Prasad <psodagud@codeaurora.org>
To:     Sudeep Holla <sudeep.holla@arm.com>
Cc:     julien.thierry@arm.com, will.deacon@arm.com,
        catalin.marinas@arm.com, mark.rutland@arm.com,
        lorenzo.pieralisi@arm.com, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: PSCI version 1.1 and SYSTEM_RESET2
In-Reply-To: <20190501094953.GA21851@e107155-lin>
References: <24970f7101952f347bd4046c9a980473@codeaurora.org>
 <efee74624f986a358b8986ae3085fba2@codeaurora.org>
 <20190501094953.GA21851@e107155-lin>
Message-ID: <3ceb06c36ecb745e2befaeaefe49be19@codeaurora.org>
User-Agent: Roundcube Webmail/1.2.5
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2019-05-01 02:49, Sudeep Holla wrote:
> On Tue, Apr 30, 2019 at 05:07:31PM -0700, Sodagudi Prasad wrote:
>> On 2019-04-30 14:44, Sodagudi Prasad wrote:
>> +Sudeep
>> 
>> > Hi Mark/Will,
>> >
>> > I would like to understand whether ARM linux community have plans to
>> > support PSCI version 1.1 or not.
>> > PSCI_1_1 specification introduced support for SYSTEM_RESET2 command
>> > and this new command helps mobile devices to SYSTEM_WARM_RESET
>> > support. Rebooting devices with warm reboot helps to capture the
>> > snapshot of the ram contents for post-mortem analysis.
>> 
>> I think, there is a recent discussion from Sudeep for the 
>> SYSTEM_RESET2
>> support.
>> https://patchwork.kernel.org/patch/10884345/
>> 
> 
> This has landed in -next, and hopefully must appear in v5.2
> 
>> 
>> Hi Sudeep,
>> 
>> I was going through your discussion in the below list -
>> https://lore.kernel.org/lkml/d73d3580-4ec1-a281-4585-5c776fc08c79@xilinx.com/
>> 
>> There is no provision to set up reboot mode dynamically instead kernel
>> command line parameter.
>> Looking for options to reboot device with warm reboot option when 
>> kernel
>> crashed.
>> 
>> panic() --> emergency_restart() --> machine_emergency_restart() -->
>> machine_restart(NULL);
>> 
>> It would nice if there is a config option to reboot the device either 
>> in
>> warm or cold in the case of kernel panic.
> 
> I presume you prefer to do warm boot in case of panic to get a dump of
> the memory to inspect ? If so, is kexec/kdump not the mechanism to
> achieve that ?

Hi Sudeep,

Thanks for your response and sharing details about your patch.
<Sudeep>  If so, is kexec/kdump not the mechanism to achieve that?
Qualcomm is having vendor specific solution to capture ram contents and 
for offline analysis.

> 
> I am just trying to understand the use case. Xilinx asked for the same
> but never got to understand their use case.

Here is the background -
Usually, power off drivers are overriding arm_pm_restart and 
pm_power_off callbacks and registering with reboot notifier with  some 
priority for the reboot operations.  Here is the Qualcomm poweroff 
driver for reference.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/power/reset/msm-poweroff.c

Before vendor chip set specific power off driver is probed, 
arm_pm_restart functions pointer holds the psci_sys_reset function. Once 
vendor power off driver is probed,  vendor drivers can override the 
arm_pm_restart function pointer.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/firmware/psci.c#n562

Once vendor driver is probed, drivers can take care of devices warm or 
hard reset configuration part properly.  But there is a window from 
start_kernel() to vendor specific driver probed, devices are getting 
cold resets even if kernel crashed.  This is due to arm_pm_restart 
points to psci_sys_reset function by default.  Is this problem clear 
now?

Qualcomm downstream kernel has a lot of use cases with respect device 
reset sequence and the downstream driver is much different from upstream 
drivers. I think, the above-mentioned problem is common for all the 
chipset vendors and it is not specific Qualcomm use cases.  I have one 
downstream solution to this problem but thought to bring up this problem 
to the upstream community for a common solution, so that all the vendors 
can use it.

I have modified below flow to avoid cold restart in the case of early 
kernel panic.
panic() --> emergency_restart() --> machine_emergency_restart() --> 
machine_restart(NULL);

-Thanks, Prasad

> 
> --
> Regards,
> Sudeep

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
Linux Foundation Collaborative Project