From:   Kazuki <kazukih0205@gmail.com>
Date:   Wed, 8 Feb 2023 23:43:27 +0900
To:     Sudeep Holla <sudeep.holla@arm.com>
Cc:     linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Hector Martin <marcan@marcan.st>,
        Sven Peter <sven@svenpeter.dev>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>
Subject: Re: s2idle breaks on machines without cpuidle support
Message-ID: <20230208144327.3ftjxnquwhsdykfc@kazuki-mac>
References: <20230204152747.drte4uitljzngdt6@kazuki-mac>
 <20230206101239.dret3fv65cnzpken@bogus>
 <20230207194818.exskn3dhyzqwr32v@kazuki-mac>
 <20230208103511.w7jzxw6spy6humdn@bogus>
 <20230208112031.sdfuluajjerf4wwp@kazuki-mac>
 <20230208141658.kede5ylqk4zqvrnj@bogus>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230208141658.kede5ylqk4zqvrnj@bogus>
Precedence: bulk

On Wed, Feb 08, 2023 at 02:16:58PM +0000, Sudeep Holla wrote:
> On Wed, Feb 08, 2023 at 08:20:31PM +0900, Kazuki wrote:
> > On Wed, Feb 08, 2023 at 10:35:11AM +0000, Sudeep Holla wrote:
> > > On Wed, Feb 08, 2023 at 04:48:18AM +0900, Kazuki wrote:
> > > > On Mon, Feb 06, 2023 at 10:12:39AM +0000, Sudeep Holla wrote:
> > > > >
> > > > > What do you mean by break ? More details on the observation would be helpful.
> > > > For example, CLOCK_MONOTONIC doesn't stop even after suspend since
> > > > these chain of commands don't get called.
> > > >
> > > > call_cpuidle_s2idle->cpuidle_enter_s2idle->enter_s2idle_proper->tick_freeze->sched_clock_suspend (Function that pauses CLOCK_MONOTONIC)
> > > >
> > > > Which in turn causes programs like systemd to crash since it doesn't
> > > > expect this.
> > >
> > > Yes expected IIUC. The per-cpu timers and counters continue to tick in
> > > WFI and hence CLOCK_MONOTONIC can't stop.
> > Yes, but it shouldn't be the case when suspending[1]. Currently that's what
> > happens when we enter s2idle without a cpuidle driver. This doesn't seem
> > to happen with S3 sleep [2].
> >
> 
> Correct, but check the requirements to use syscore operations(mainly
> syscore_suspend/resume where only one CPU is online with interrupts
> disabled. In case of s2idle, all CPUs are idling and not offlined as
> required by the syscore operations and hence it can't be used.
> 
> I was about ask you earlier as why can't you implement just system
> suspend in PSCI where the last cpu just calls WFI if you are interested
> in system sleep state. Or you can implement CPU_SUSPEND with an additional
> retention state which enters PSCI implementation just to make sure there is
> an active cpuidle driver and the s2idle state machinery works as expected.
The machine I have (Macbook with Apple M1) doesn't have PSCI.
> It is built with those requirements and trying to work it out for WFI without
> any idle driver or firmware implementation to back it up is just not going
> to work.
Any reason why that's the case?

I guess we should ensure that systems without a cpuidle driver
will not suspend maybe around here then.

kernel/power/main.c:

static ssize_t mem_sleep_store(struct kobject *kobj, struct kobj_attribute *attr,
			       const char *buf, size_t n)
{
	suspend_state_t state;
	int error;

	error = pm_autosleep_lock();
	if (error)
		return error;

	if (pm_autosleep_state() > PM_SUSPEND_ON) {
		error = -EBUSY;
		goto out;
	}

	state = decode_suspend_state(buf, n);
	if (state < PM_SUSPEND_MAX && state > PM_SUSPEND_ON)
		mem_sleep_current = state;
	else
		error = -EINVAL;

 out:
	pm_autosleep_unlock();
	return error ? error : n;
}
> 
> --
> Regards,
> Sudeep
Thanks,
Kazuki