Re: Bug report: kernel paniced when system hibernates

[Anup Patel]

On Thu, May 25, 2023 at 7:26 PM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote:
>
> On Thu, May 25, 2023 at 07:13:11PM +0530, Anup Patel wrote:
> > On Thu, May 25, 2023 at 7:08 PM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> 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 opensbi
> > > > > IMO. Going forward, if something needs to be reserved for firmware, the
> > > > > firmware should make sure that it is reserved by using the property for
> > > > > that purpose :)
> > >
> > > > There is no issue with OpenSBI since it does the right thing by marking
> > > > memory as reserved in the DT. This real issue is with the kernel handling
> > > > 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 entities
> > > > (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 hibernation.
> > > > > > This can be documented in RISC-V DT bindings as well as the booting guide
> > > > > > doc that alex was talking about.
> > > > >
> > > > > Sure, a dt-binding for sbi reserved regions doesn't immediately sound
> > > > > 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.