Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>
Cc:     linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v1] kernel/reboot: Change registration order of legacy
 power-off handler
In-Reply-To: <e597fd8e-d0d1-dd1c-b889-86cfca60f0f6@collabora.com>
References: <20220524212118.425702-1-dmitry.osipenko@collabora.com>
 <8735gjq365.fsf@mpe.ellerman.id.au>
 <e597fd8e-d0d1-dd1c-b889-86cfca60f0f6@collabora.com>
Date:   Mon, 06 Jun 2022 23:06:23 +1000
Message-ID: <87r142ndps.fsf@mpe.ellerman.id.au>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:
> Hi Michael,
>
> On 6/5/22 05:01, Michael Ellerman wrote:
>> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:
>>> We're unconditionally registering sys-off handler for the legacy
>>> pm_power_off() callback, this causes problem for platforms that don't
>>> use power-off handlers at all and should be halted. Now reboot syscall
>>> assumes that there is a power-off handler installed and tries to power
>>> off system instead of halting it.
>>>
>>> To fix the trouble, move the handler's registration to the reboot syscall
>>> and check the pm_power_off() presence.
>>
>> I'm seeing a qemu virtual machine (ppce500) fail to power off using the
>> gpio-poweroff driver. I bisected it to this commit.
>>
>> I think the problem is that the machine is going via kernel_power_off(),
>> not sys_reboot(), and so legacy_pm_power_off() has not been registered.
>>
>> If I just put the core_initcall back then it works as before. Not sure
>> if that's a safe change in general though.
>
> Thank you very much for the testing and reporting the problem! I see now the two more cases that were missed previously:
>
> 1. There is the orderly_poweroff() used by some drivers.
> 2. PowerPC may invoke do_kernel_power_off() directly from xmon code.
>
> Could you please test this change:

That works, thanks.

I tested both sysrq-o and the xmon power off path.

I couldn't come up with an easy way to test the orderly_poweroff()
path, but it boils down to basically the same code in the end.

Tested-by: Michael Ellerman <mpe@ellerman.id.au>

cheers