All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>,
	Guillaume Tucker <guillaume.tucker@collabora.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Nicolas Pitre <nico@fluxnic.net>,
	kernelci-results@groups.io
Subject: Re: next/master bisection: baseline.login on rk3288-rock2-square
Date: Thu, 04 Feb 2021 12:26:44 +0000	[thread overview]
Message-ID: <090003e6f825de1f8460c0e987e14577@kernel.org> (raw)
In-Reply-To: <CAMj1kXF6SLXN3HQAG3SyOujX5MPCSrLG-k82iNz=61HjaiEEVw@mail.gmail.com>

On 2021-02-04 10:55, Ard Biesheuvel wrote:
> (cc Marc)
> 
> On Thu, 4 Feb 2021 at 11:48, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
>> 
>> On Thu, Feb 04, 2021 at 11:27:16AM +0100, Ard Biesheuvel wrote:
>> > Hi Russell,
>> >
>> > If Guillaume is willing to do the experiment, and it fixes the issue,
>> > it proves that rk3288 is relying on the flush before the MMU is
>> > disabled, and so in that case, the fix is trivial, and we can just
>> > apply it.
>> >
>> > If the experiment fails (which would mean rk3288 does not tolerate the
>> > cache maintenance being performed after cache off), it is going to be
>> > hairy, and so it will definitely take more time.
>> >
>> > So in the latter case (or if Guillaume does not get back to us), I
>> > think reverting my queued fix is the only sane option. But in that
>> > case, may I suggest that we queue the revert of the original by-VA
>> > change for v5.12 so it gets lots of coverage in -next, and allows us
>> > an opportunity to come up with a proper fix in the same timeframe, and
>> > backport the revert and the subsequent fix as a pair? Otherwise, we'll
>> > end up in the situation where v5.10.x until today has by-va, v5.10.x-y
>> > has set/way, and v5.10y+ has by-va again. (I don't think we care about
>> > anything before that, given that v5.4 predates any of this)
>> 
>> I'm suggesting dropping your fix (9052/1) and reverting
>> "ARM: decompressor: switch to by-VA cache maintenance for v7 cores"
>> which gets us to a point where _both_ regressions are fixed.
>> 
> 
> I understand, but we don't know whether doing so might regress other
> platforms that were added in the mean time.
> 
>> I'm of the opinion that the by-VA patch was incorrect when it was
>> merged (it caused a regression), and it's only a performance
>> improvement.
> 
> It is a correctness improvement, not a performance improvement.
> 
> Without that change, the 32-bit ARM kernel cannot boot bare metal on
> platforms with a system cache such as 8040 or SynQuacer, and can only
> boot under KVM on such systems because of the special handling of
> set/way instructions by the host.

I agree. With set/way CMOs, there is no way to reach the PoC if
it beyond the system cache, leading to an unbootable kernel.
This is actually pretty well documented in the architecture,
and it did bite us for the first time on XGene-1, 7 years ago.

In retrospect, having KVM to handle set/way CMOs in was a mistake,
as it just papered over the problem for the sake of running older
32bit guests. It violated the principle of KVM/arm being strictly
architectural and provided unrealistic expectations. I'll take the
blame for this.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: kernelci-results@groups.io,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Nicolas Pitre <nico@fluxnic.net>,
	Guillaume Tucker <guillaume.tucker@collabora.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Russell King - ARM Linux admin <linux@armlinux.org.uk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: next/master bisection: baseline.login on rk3288-rock2-square
Date: Thu, 04 Feb 2021 12:26:44 +0000	[thread overview]
Message-ID: <090003e6f825de1f8460c0e987e14577@kernel.org> (raw)
In-Reply-To: <CAMj1kXF6SLXN3HQAG3SyOujX5MPCSrLG-k82iNz=61HjaiEEVw@mail.gmail.com>

On 2021-02-04 10:55, Ard Biesheuvel wrote:
> (cc Marc)
> 
> On Thu, 4 Feb 2021 at 11:48, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
>> 
>> On Thu, Feb 04, 2021 at 11:27:16AM +0100, Ard Biesheuvel wrote:
>> > Hi Russell,
>> >
>> > If Guillaume is willing to do the experiment, and it fixes the issue,
>> > it proves that rk3288 is relying on the flush before the MMU is
>> > disabled, and so in that case, the fix is trivial, and we can just
>> > apply it.
>> >
>> > If the experiment fails (which would mean rk3288 does not tolerate the
>> > cache maintenance being performed after cache off), it is going to be
>> > hairy, and so it will definitely take more time.
>> >
>> > So in the latter case (or if Guillaume does not get back to us), I
>> > think reverting my queued fix is the only sane option. But in that
>> > case, may I suggest that we queue the revert of the original by-VA
>> > change for v5.12 so it gets lots of coverage in -next, and allows us
>> > an opportunity to come up with a proper fix in the same timeframe, and
>> > backport the revert and the subsequent fix as a pair? Otherwise, we'll
>> > end up in the situation where v5.10.x until today has by-va, v5.10.x-y
>> > has set/way, and v5.10y+ has by-va again. (I don't think we care about
>> > anything before that, given that v5.4 predates any of this)
>> 
>> I'm suggesting dropping your fix (9052/1) and reverting
>> "ARM: decompressor: switch to by-VA cache maintenance for v7 cores"
>> which gets us to a point where _both_ regressions are fixed.
>> 
> 
> I understand, but we don't know whether doing so might regress other
> platforms that were added in the mean time.
> 
>> I'm of the opinion that the by-VA patch was incorrect when it was
>> merged (it caused a regression), and it's only a performance
>> improvement.
> 
> It is a correctness improvement, not a performance improvement.
> 
> Without that change, the 32-bit ARM kernel cannot boot bare metal on
> platforms with a system cache such as 8040 or SynQuacer, and can only
> boot under KVM on such systems because of the special handling of
> set/way instructions by the host.

I agree. With set/way CMOs, there is no way to reach the PoC if
it beyond the system cache, leading to an unbootable kernel.
This is actually pretty well documented in the architecture,
and it did bite us for the first time on XGene-1, 7 years ago.

In retrospect, having KVM to handle set/way CMOs in was a mistake,
as it just papered over the problem for the sake of running older
32bit guests. It violated the principle of KVM/arm being strictly
architectural and provided unrealistic expectations. I'll take the
blame for this.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-02-04 12:27 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <601b773a.1c69fb81.9f381.a32a@mx.google.com>
2021-02-04  8:43 ` next/master bisection: baseline.login on rk3288-rock2-square Guillaume Tucker
2021-02-04  8:43   ` Guillaume Tucker
2021-02-04  9:07   ` Ard Biesheuvel
2021-02-04  9:07     ` Ard Biesheuvel
2021-02-04 10:06     ` Russell King - ARM Linux admin
2021-02-04 10:06       ` Russell King - ARM Linux admin
2021-02-04 10:27       ` Ard Biesheuvel
2021-02-04 10:27         ` Ard Biesheuvel
2021-02-04 10:33         ` Guillaume Tucker
2021-02-04 10:33           ` Guillaume Tucker
2021-02-04 11:32           ` Guillaume Tucker
2021-02-04 11:32             ` Guillaume Tucker
2021-02-04 11:44             ` Russell King - ARM Linux admin
2021-02-04 11:44               ` Russell King - ARM Linux admin
2021-02-04 12:09               ` Ard Biesheuvel
2021-02-04 12:09                 ` Ard Biesheuvel
2021-02-04 15:42             ` Ard Biesheuvel
2021-02-04 15:42               ` Ard Biesheuvel
2021-02-04 15:53               ` Guillaume Tucker
2021-02-04 15:53                 ` Guillaume Tucker
2021-02-04 16:01                 ` Ard Biesheuvel
2021-02-04 16:01                   ` Ard Biesheuvel
2021-02-04 18:06                   ` Nick Desaulniers
2021-02-04 18:06                     ` Nick Desaulniers
2021-02-04 18:12                     ` Nathan Chancellor
2021-02-04 18:12                       ` Nathan Chancellor
2021-02-04 18:23                       ` Nick Desaulniers
2021-02-04 18:23                         ` Nick Desaulniers
2021-02-04 21:31                         ` Guillaume Tucker
2021-02-04 21:31                           ` Guillaume Tucker
2021-02-04 21:50                           ` Russell King - ARM Linux admin
2021-02-04 21:50                             ` Russell King - ARM Linux admin
2021-02-05  8:21                           ` Ard Biesheuvel
2021-02-05  8:21                             ` Ard Biesheuvel
2021-02-05 12:05                             ` Ard Biesheuvel
2021-02-05 12:05                               ` Ard Biesheuvel
2021-02-06 13:10                               ` Guillaume Tucker
2021-02-06 13:10                                 ` Guillaume Tucker
2021-02-06 13:12                                 ` Ard Biesheuvel
2021-02-06 13:12                                   ` Ard Biesheuvel
2021-02-04 21:09                   ` Guillaume Tucker
2021-02-04 21:09                     ` Guillaume Tucker
2021-02-04 10:47         ` Russell King - ARM Linux admin
2021-02-04 10:47           ` Russell King - ARM Linux admin
2021-02-04 10:55           ` Ard Biesheuvel
2021-02-04 10:55             ` Ard Biesheuvel
2021-02-04 12:26             ` Marc Zyngier [this message]
2021-02-04 12:26               ` Marc Zyngier
2021-02-04 14:09               ` Russell King - ARM Linux admin
2021-02-04 14:09                 ` Russell King - ARM Linux admin
2021-02-04 14:25                 ` Ard Biesheuvel
2021-02-04 14:25                   ` Ard Biesheuvel
2021-02-04 14:36                   ` Russell King - ARM Linux admin
2021-02-04 14:36                     ` Russell King - ARM Linux admin
2021-02-04 15:52                     ` Ard Biesheuvel
2021-02-04 15:52                       ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=090003e6f825de1f8460c0e987e14577@kernel.org \
    --to=maz@kernel.org \
    --cc=ardb@kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=guillaume.tucker@collabora.com \
    --cc=kernelci-results@groups.io \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=nico@fluxnic.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.