Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230607034403.2885-1-james.liu@hpe.com> <CAJZ5v0hTsEzFKvRf-MHpUxyJdFDUqc2ZL63s6BkyJyFtEzxvhw@mail.gmail.com>
 <ZIGbBF+GxHAlTqGk@ILEIUN5Z4B.asiapacific.hpqcorp.net> <CAJZ5v0iR+CAOwip0zPV1uug-0duJXFS4YMxwNWDntUnK6a+oQQ@mail.gmail.com>
 <64dbd730.050a0220.46601.d780SMTPIN_ADDED_BROKEN@mx.google.com>
In-Reply-To: <64dbd730.050a0220.46601.d780SMTPIN_ADDED_BROKEN@mx.google.com>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Thu, 17 Aug 2023 13:08:31 +0200
Message-ID: <CAJZ5v0i3MSrAQj1qbky_aJTCE2b_9Hkq2tyVMkuV_NPaUwa_cQ@mail.gmail.com>
Subject: Re: [PATCH v1] ACPI: reboot: Increase the delay to avoid racing after
 writing to ACPI RESET_REG on AMD Milan platforms.
To:     James Liu <james.liu@hpe.com>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>,
        linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Tue, Aug 15, 2023 at 9:51 PM James Liu <james.liu@hpe.com> wrote:
>
> On Mon, Jun 12, 2023 at 06:57:01PM +0200, Rafael J. Wysocki wrote:
> > On Thu, Jun 8, 2023 at 11:14 AM James Liu <james.liu@hpe.com> wrote:
> > >
> > > On Wed, Jun 07, 2023 at 01:19:42PM +0200, Rafael J. Wysocki wrote:
> > > > On Wed, Jun 7, 2023 at 5:44 AM James Liu <james.liu@hpe.com> wrote:
> > > > >
> > > > > For AMD Milan platforms, the delay of 15ms is insufficient to avoid racing
> > > > > of reboot mechanisms. That said, the AMD Milan processors don't reboot
> > > > > in 15ms after invoking acpi_reset().
> > > > >
> > > > > The proposed 50ms delay can effectively work around this issue.
> > > > > This extended delay aligns better with ACPI v6.4 (i.e., sec. 4.8.4.6),
> > > > > which indicates that ideally OSPM should execute spin loops on the CPUs
> > > > > in the system following a write to this register.
> > > > >
> > > > > Signed-off-by: James Liu <james.liu@hpe.com>
> > > >
> > > > Why do you want to affect everyone (including guest kernels running in
> > > > virtual machines AFAICS) in order to address a problem specific to one
> > > > platform?
> > >
> > > I hoped to address this issue for any platform requiring a longer delay to
> > > complete ACPI reset in time for any (maybe silicon-level) reasons. Some AMD Milan
> > > platforms were the cases that we've found so far.
> >
> > Do you know about any other?
>
> So far, no. Thus, I wont' proceed with anything until I find the same syndrome
> on next-gen platforms (e.g., AMD Genoa). Now, as you say, it is satisfying to
> document this quirk properly.
>
> >
> > > Except that, since ACPI spec indicates there should be a spin loop (long enough)
> > > after the write instruction to Reset Register, I thought it should be no harms to
> > > the other systems which well consider this spin loop when they claim to support
> > > ACPI reboot.
> > >
> > > Btw, I am just curious, why is the virtual machine mentioned here?
> >
> > The new delay would be over 3 times larger, so some users might be
> > surprised by it at least potentially.
>
> Got it. Just in case, if we really need to increase the delay to address it for
> certain amount of platforms, in experience, how long is the delay acceptable so
> that VM users will not be surprised?

Honestly, I don't know.

Also, it is not quite clear to me from the changelog what the problem
with the Milan platforms vs the reboot delay is and I don't think that
the second paragraph (regarding the ACPI specification compliance) is
relevant for this patch at all.