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: <20230517-preacher-primer-f41020b3376a@wendy> <CAHVXubhMLgb54_7zV2yFuGPoMKCkUXwozHbDvghc7kQqNLK-JA@mail.gmail.com>
 <CAAhSdy3tKAk1xjinwnSWan0ivdDapcLvSb+hGNynPFZMUsoB9A@mail.gmail.com>
 <fe8d716c-fb4f-1f3f-6c69-de1d8b9fb6af@ghiti.fr> <CAK9=C2X1BjZCHfYM33pZQtavu7yRqxwsypWL5OWj79bJrnDMQg@mail.gmail.com>
 <CAOnJCULpa-TJuG=TtCDOxOdUviZzWheLE-GMiU1r7GWaKn0nuQ@mail.gmail.com>
 <20230525-guacamole-swimmer-68048a73baac@wendy> <CAK9=C2WUyLxZwQO37cN-i+V+A3yxmEoaj=uE8yR8nseYTDW7oQ@mail.gmail.com>
 <20230525-postnasal-monopoly-98adb96ffaa1@wendy> <CAAhSdy06nQh4H1FP_K_-VF462mhj+F2M=4AV4QSCUGe5XVqX0g@mail.gmail.com>
 <20230525-shrapnel-precut-26500fca4a48@wendy>
In-Reply-To: <20230525-shrapnel-precut-26500fca4a48@wendy>
From:   Anup Patel <anup@brainfault.org>
Date:   Thu, 25 May 2023 19:29:46 +0530
Message-ID: <CAAhSdy3SqeLdAfaojUki=ht21nr4ZUPMkW_t9M6ntQCt6Ds4Nw@mail.gmail.com>
Subject: Re: Bug report: kernel paniced when system hibernates
To:     Conor Dooley <conor.dooley@microchip.com>
Cc:     Anup Patel <apatel@ventanamicro.com>,
        Atish Patra <atishp@atishpatra.org>,
        Alexandre Ghiti <alex@ghiti.fr>, robh@kernel.org,
        Alexandre Ghiti <alexghiti@rivosinc.com>,
        jeeheng.sia@starfivetech.com, linux-kernel@vger.kernel.org,
        palmer@rivosinc.com, leyfoon.tan@starfivetech.com,
        mason.huo@starfivetech.com, Guo Ren <guoren@kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Song Shuai <suagrfillet@gmail.com>,
        linux-riscv@lists.infradead.org,
        Andrew Jones <ajones@ventanamicro.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, May 25, 2023 at 7:26=E2=80=AFPM Conor Dooley <conor.dooley@microchi=
p.com> wrote:
>
> On Thu, May 25, 2023 at 07:13:11PM +0530, Anup Patel wrote:
> > On Thu, May 25, 2023 at 7:08=E2=80=AFPM Conor Dooley <conor.dooley@micr=
ochip.com> wrote:
> > >
> > > On Thu, May 25, 2023 at 06:51:28PM +0530, Anup Patel wrote:
> > >
> > > > > We should only rely on this node name for known bad versions of o=
pensbi
> > > > > IMO. Going forward, if something needs to be reserved for firmwar=
e, the
> > > > > firmware should make sure that it is reserved by using the proper=
ty for
> > > > > that purpose :)
> > >
> > > > There is no issue with OpenSBI since it does the right thing by mar=
king
> > > > memory as reserved in the DT. This real issue is with the kernel ha=
ndling
> > > > of reserved memory for hibernate.
> > >
> > > I don't think we are talking about the same thing here. I meant the
> > > no-map property which OpenSBI does not set.
> >
> > Yes, we are talking about the same thing. It's not just OpenSBI not
> > setting no-map property in reserved memory node because other
> > SBI implementations would be doing the same thing (i.e. not setting
> > no-map property)
>
> Other SBI implementations doing the same thing doesn't make it any more
> correct though, right?

Like multiple folks suggested, we need DT binding for distinguishing
firmware reserved memory from other reserved memory. Until that
happens we should either mark hibernate support as experimental
or revert it.

Regards,
Anup

>
> > > > Like Atish mentioned, not just OpenSBI, there will be other entitie=
s
> > > > (like TSM) or some other M-mode firmware which will also reserve
> > > > memory in DT/ACPI so clearly kernel needs a SBI implementation
> > > > independent way of handling reserved memory for hibernate.
> > >
> > > > > > Another option is to use compatible string or label property to=
 indicate
> > > > > > that this memory region is not to be saved/restored during hibe=
rnation.
> > > > > > This can be documented in RISC-V DT bindings as well as the boo=
ting guide
> > > > > > doc that alex was talking about.
> > > > >
> > > > > Sure, a dt-binding for sbi reserved regions doesn't immediately s=
ound
> > > > > like an awful idea... But we still have to work around the borked
> > > > > firmware - be that disabling hibernation or using the mmode_resv =
node
> > > > > when we know that the version of OpenSBI is one of those with the
> > > > > problem.
> > >
> > > Did you skip over this? I was agreeing that defining a common binding=
 for
> > > sbi reserved regions was a good idea.