linux-kernel - Re: [PATCH 11/12] PM, libnvdimm: Add syscore_quiesced() callback for firmware activation

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAJZ5v0gL66d7fo8nmKFUm6dSBe8bSRymcOfW9YeePUJ9U=3p-g@mail.gmail.com>
Date:   Fri, 26 Jun 2020 16:23:29 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Vishal Verma <vishal.l.verma@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Ira Weiny <ira.weiny@...el.com>, Pavel Machek <pavel@....cz>,
        Len Brown <len.brown@...el.com>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 11/12] PM, libnvdimm: Add syscore_quiesced() callback for
 firmware activation

On Fri, Jun 26, 2020 at 2:07 AM Dan Williams <dan.j.williams@...el.com> wrote:
>
> The runtime firmware activation capability of Intel NVDIMM devices
> requires memory transactions to be disabled for 100s of microseconds.
> This timeout is large enough to cause in-flight DMA to fail and other
> application detectable timeouts. Arrange for firmware activation to be
> executed while the system is "quiesced" all suspend operations have
> completed successfully.
>
> Note that the placement of syscore_quiesced() before
> suspend_disable_secondary_cpus() and the "TEST_PLATFORM" early exit in
> suspend_enter():
>
>         if (suspend_test(TEST_PLATFORM))
>                 goto Platform_wake;
>
> ...is a deliberate tradeoff. suspend_disable_secondary_cpus() causes
> violence to drivers with many interrupts allocated (server-class network
> adapters for example). So, allow for triggering firmware-activation
> without requiring all irq vectors to be routed (oversubscribed) to a
> single CPU.

So while the other patches in the series look good to me, this is kind
of questionable for reasons described in my reply to the intro
message.

Cheers!