On Sun, 25 Jun 2023 15:15:14 PDT (-0700), Conor Dooley wrote:
Hey,
On Sun, Jun 25, 2023 at 11:09:21PM +0800, Song Shuai wrote:
Sorry for the delayed reply,
It wasn't really delayed at all actually, you replied within an hour or
so, AFAICT.
My tinylab email went something wrong, I'll use gmail in this thread.
在 2023/6/25 22:18, Conor Dooley 写道:
> On Sun, Jun 25, 2023 at 10:09:29PM +0800, Song Shuai wrote:
> > This reverts commit ed309ce522185583b163bd0c74f0d9f299fe1826.
> >
> > With the commit 3335068f8721 ("riscv: Use PUD/P4D/PGD pages for the
> > linear mapping") reverted, the MIN_MEMBLOCK_ADDR points the kernel
> > load address which was placed at a PMD boundary.
>
> > And firmware always
> > correctly mark resident memory, or memory protected with PMP as
> > per the devicetree specification and/or the UEFI specification.
>
> But this is not true? The versions of OpenSBI that you mention in your
> cover letter do not do this.
> Please explain.
>
At this time, OpenSbi [v0.8,v1.3) and edk2(RiscVVirt) indeed don't obey the
DT/UEFI spec. This statement is excerpted from "Reserved memory for resident
firmware" part from the upcoming riscv/boot.rst. It isn't accurate for now.
How about deleting this one?
It is incorrect, so it will need to be removed, yes.
Unfortunately writing a doc does not fix the existing implementations :(
Actually with 3335068f8721 reverted, the change of MIN_MEMBLOCK_ADDR can
avoid the mapping of firmware memory, I will make it clear in the next
version.
To be honest, I'd like to see this revert as the final commit in a
series that deals with the problem by actually reserving the regions,
rather than a set of reverts that go back to how we were.
I was hoping that someone who cares about hibernation support would be
interested in working on that - *cough* starfive *cough*, although maybe
they just fixed their OpenSBI and moved on.
If there were no volunteers, my intention was to add a firmware erratum
that would probe the SBI implementation & version IDs, and add a firmware
erratum that'd parse the DT for the offending regions and reserve them.
Is there any actual use case for hibernation on these boards? Maybe
it's simpler to just add a "reserved regions actually work" sort of
property and then have new firmware set it -- that way we can avoid
sorting through all the old stuff nobody cares about and just get on
with fixing the stuff people use.
Cheers,
Conor.
> > So those regions will not be mapped in the linear mapping and they
> > can be safely saved/restored by hibernation.
> >
> > Signed-off-by: Song Shuai <songshuaishuai@xxxxxxxxxxx>
> > ---
> > arch/riscv/Kconfig | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 5966ad97c30c..17b5fc7f54d4 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -800,11 +800,8 @@ menu "Power management options"
> > source "kernel/power/Kconfig"
> > -# Hibernation is only possible on systems where the SBI implementation has
> > -# marked its reserved memory as not accessible from, or does not run
> > -# from the same memory as, Linux
> > config ARCH_HIBERNATION_POSSIBLE
> > - def_bool NONPORTABLE
> > + def_bool y
> > config ARCH_HIBERNATION_HEADER
> > def_bool HIBERNATION
> > --
> > 2.20.1
