linux-kernel - Re: [PATCH 4/6] ACPI: LPSS: Fix ->suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJZ5v0iqnKMfWewZv=QnJHojBebuoCBuw0iO7QJ=xhAeymhY+w@mail.gmail.com>
Date:   Sun, 30 Jun 2019 11:48:18 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux PCI <linux-pci@...r.kernel.org>,
        Linux ACPI <linux-acpi@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Bjorn Helgaas <helgaas@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        "Robert R. Howell" <RHowell@...o.edu>
Subject: Re: [PATCH 4/6] ACPI: LPSS: Fix ->suspend_late callbacks handling

On Sun, Jun 30, 2019 at 12:02 AM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Sat, Jun 29, 2019 at 1:34 PM Hans de Goede <hdegoede@...hat.com> wrote:
> >
> > Hi Rafael,
> >
> > On 29-06-19 11:50, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > >
> > > If the resume_from_noirq flag is set in dev_desc, the ->suspend_late
> > > callback provided by the device driver will be invoked at the "noirq"
> > > stage of system suspend, via acpi_lpss_do_suspend_late(), which is
> > > incorrect.
> > >
> > > To fix that, drop acpi_lpss_do_suspend_late() and rearrange
> > > acpi_lpss_suspend_late() to call pm_generic_suspend_late()
> > > directly, before calling acpi_lpss_suspend(), in analogy with
> > > acpi_subsys_suspend_late().
> >
> > Ah now I see the logic in your previous test-patch.
> >
> > I'm afraid that this is going to break things though, the calling
> > of the device-driver's suspend-late method at noirq time is
> > *intentional* !
>
> But it is a bug too.

Well, not strictly a bug, but a departure from a clearly established convention.

What happens is that the driver provides ->suspend_late and
->resume_early, because the upper layer normally would remove power
and power up the device at those stages, respectively.  However, due
to dependencies between devices, the upper layer kind of works around
its own limitation.

That is not straightforward at all.

I will retain this setup in the current patch series, but going
forward it would be good to clean it up.  I wonder if using
non-PM-runtime device links to represent the dependencies would work
in this case.