Re: [PATCH 2/3] PM: hibernate: Add platform_enter hibernation test level

[F. R. A. Prado]

On Tue, 2026-05-26 at 13:28 +0200, Rafael J. Wysocki wrote:
> On Mon, May 18, 2026 at 11:34 PM Nícolas F. R. A. Prado
> <nfraprado@xxxxxxxxxxxxx> wrote:
> > 
> > Add a 'platform_enter' hibernation test level to allow testing the
> > full
> > hibernation path up until the last point of return in
> > hibernation_platform_enter().
> > 
> > In particular this allows seeing print messages from the code run
> > after
> > the hibernation image is created on platforms that don't have an
> > easily
> > accessible serial console, since these messages are not present
> > upon
> > hibernation resume.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx>
> > ---
> >  kernel/power/hibernate.c | 13 +++++++++++--
> >  kernel/power/main.c      |  1 +
> >  kernel/power/power.h     |  1 +
> >  3 files changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> > index 001e4556b137..15cb7a090770 100644
> > --- a/kernel/power/hibernate.c
> > +++ b/kernel/power/hibernate.c
> > @@ -75,6 +75,7 @@ enum {
> >  static int hibernation_mode = HIBERNATION_SHUTDOWN;
> > 
> >  bool freezer_test_done;
> > +bool platform_enter_test_done;
> > 
> >  static const struct platform_hibernation_ops *hibernation_ops;
> > 
> > @@ -636,6 +637,11 @@ int hibernation_platform_enter(void)
> >                 goto Power_up;
> >         }
> > 
> > +       if (hibernation_test(TEST_PLATFORM_ENTER)) {
> > +               platform_enter_test_done = true;
> > +               goto Power_up;
> > +       }
> > +
> >         hibernation_ops->enter();
> >         /* We should never get here */
> >         while (1);
> > @@ -700,6 +706,8 @@ static int power_down(void)
> >                 break;
> >         case HIBERNATION_PLATFORM:
> >                 error = hibernation_platform_enter();
> > +               if (platform_enter_test_done)
> > +                       goto exit;
> >                 if (error == -EAGAIN || error == -EBUSY) {
> >                         events_check_enabled = false;
> >                         pr_info("Wakeup event detected during
> > hibernation, rolling back.\n");
> > @@ -848,7 +856,7 @@ int hibernate(void)
> >                         else
> >                                 error = power_down();
> >                 }
> > -               if (error) {
> > +               if (error || platform_enter_test_done) {
> >                         /* recover any devices that refused to thaw
> > */
> >                         dpm_resume_suspended_devices(PMSG_RECOVER);
> >                 }
> > @@ -870,8 +878,9 @@ int hibernate(void)
> >         }
> >         thaw_processes();
> > 
> > -       /* Don't bother checking whether freezer_test_done is true
> > */
> > +       /* Don't bother checking whether any of the test_done flags
> > are true */
> >         freezer_test_done = false;
> > +       platform_enter_test_done = false;
> >   Exit:
> >         filesystems_thaw();
> >         pm_notifier_call_chain(PM_POST_HIBERNATION);
> > diff --git a/kernel/power/main.c b/kernel/power/main.c
> > index 7f53ef65b789..04f0c429519c 100644
> > --- a/kernel/power/main.c
> > +++ b/kernel/power/main.c
> > @@ -273,6 +273,7 @@ static const char * const
> > pm_tests[__TEST_AFTER_LAST] = {
> >         [TEST_PLATFORM] = "platform",
> >         [TEST_DEVICES] = "devices",
> >         [TEST_FREEZER] = "freezer",
> > +       [TEST_PLATFORM_ENTER] = "platform_enter",
> >  };
> > 
> >  static ssize_t pm_test_show(struct kobject *kobj, struct
> > kobj_attribute *attr,
> > diff --git a/kernel/power/power.h b/kernel/power/power.h
> > index 7ccd709af93f..0d33d0e46ed0 100644
> > --- a/kernel/power/power.h
> > +++ b/kernel/power/power.h
> > @@ -259,6 +259,7 @@ enum {
> >         TEST_PLATFORM,
> >         TEST_DEVICES,
> >         TEST_FREEZER,
> > +       TEST_PLATFORM_ENTER,
> >         /* keep last */
> >         __TEST_AFTER_LAST
> >  };
> > 
> 
> In the first place, test levels are designed to be applicable to both
> system suspend and hibernation and I don't see how this test can be
> applied to system suspend.

Indeed this is specific to system hibernation, in fact, to the
"platform" hibernation method. I hadn't noticed the test levels were
designed to be generic.

That said I still think hibernation-specific (and even hibernation-
method-specific) test levels would be a useful addition since they
would allow validating the steps between the end of hibernation image
creation and system power down are working correctly.

As far as I can tell the reason against that is the current uAPI?

Would having test levels listed in pm_test that are only applicable for
certain sleep states (and method combinations) not be acceptable even
if they are clearly named? eg "disk-platform-platform_enter"?

Another option could be having a separate file to list the test levels
that are particular to each sleep state and method.

Or perhaps we could add another test entry in /sys/power/disk like
test_resume. However that would only work for tests that don't depend
on a specific hibernation method, so not useful for this specific
patch, but could still be an option.

> 
> Second, the last device power off transition during hibernation may
> not be reversible and adding a test that goes past a point of no
> return is not particularly useful.

On my platform, this stage is definitely reversible, and is definitely
useful, since it allowed me to benchmark the hibernation image creation
process which would otherwise be impossible.

This is a test that the user chooses to run. Even if it doesn't work on
some platforms, it can simply not be run on those, while being useful
on the platforms that do support it.

Thanks,
Nícolas