From:   Kazuki <kazukih0205@gmail.com>
Date:   Wed, 8 Feb 2023 04:48:18 +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,
        Sudeep Holla <sudeep.holla@arm.com>,
        "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: <20230207194818.exskn3dhyzqwr32v@kazuki-mac>
References: <20230204152747.drte4uitljzngdt6@kazuki-mac>
 <20230206101239.dret3fv65cnzpken@bogus>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230206101239.dret3fv65cnzpken@bogus>
Precedence: bulk

On Mon, Feb 06, 2023 at 10:12:39AM +0000, Sudeep Holla wrote:
> Hi Kazaki,
> 
> On Sun, Feb 05, 2023 at 12:27:47AM +0900, Kazuki wrote:
> > 
> > Hi everyone,
> > 
> > s2idle is blocked on machines without proper cpuidle support here
> > in kernel/sched/idle.c:
> > 
> > > if (cpuidle_not_available(drv, dev)) {
> > > 	tick_nohz_idle_stop_tick();
> > 
> > > 	default_idle_call();
> > > 	goto exit_idle;
> > > }
> > 
> > > /*
> > >  * Suspend-to-idle ("s2idle") is a system state in which all user space
> > >  * has been frozen, all I/O devices have been suspended and the only
> > 
> > However, there are 2 problems with this approach:
> > 
> > 1. The suspend framework does not expect this, and continues to suspend the
> > machine, which causes machines without proper cpuidle support to break when
> > suspending
> 
> 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.
> 
> > 2. Suspend actually works on ARM64 machines even without proper
> > cpuidle (PSCI cpuidle) since they support wfi, so the assumption here is wrong
> > on such machines
> >
> 
> Sorry I am bit confused here. Your point (2) contradicts the $subject.
drivers/cpuidle/cpuidle.c:

bool cpuidle_not_available(struct cpuidle_driver *drv,
			   struct cpuidle_device *dev)
{
	return off || !initialized || !drv || !dev || !dev->enabled;
}

The cpuidle framework reports ARM64 devices without PSCI cpuidle as
"cpuidle not available" even when they support wfi, which causes suspend
to fail, which shouldn't be happening since they do support idling.
> 
> > I'm not exactly sure how to figure this out, and my attempts have all led to an
> > unbootable kernel, so I've cc'ed the relevant people and hopefully we can find a
> > solution to this problem.
> >
> 
> Again, since s2idle is userspace driven, I don't understand what do you
> mean by unbootable kernel in the context of s2idle.
Sorry, I meant "attempts to fix this bug have all led to an unbootable
kernel."
> 
> -- 
> Regards,
> Sudeep
Thanks,
Kazuki