Date: Fri, 2 Dec 2016 08:11:57 +0100
From: Robert Richter <robert.richter@cavium.com>
To: James Morse <james.morse@arm.com>
CC: Will Deacon <will.deacon@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Ard Biesheuvel <ard.biesheuvel@linaro.org>,
        David Daney <david.daney@cavium.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Hanjun Guo <hanjun.guo@linaro.org>,
        <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] arm64: mm: Fix memmap to be initialized for the
 entire section
Message-ID: <20161202071157.GQ2213@rric.localdomain>
References: <1480530091-1092-1-git-send-email-rrichter@cavium.com>
 <20161201164538.GB1236@arm.com>
 <58405D5F.90805@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <58405D5F.90805@arm.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Dec 2016 07:12:17.7894 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR07MB2341
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1441
Lines: 34

James,

On 01.12.16 17:26:55, James Morse wrote:
> On 01/12/16 16:45, Will Deacon wrote:
> > Thanks for sending out the new patch. Whilst I'm still a bit worried about
> > changing pfn_valid like this, I guess we'll just have to fix up any callers
> > which suffer from this change.
> 
> Hibernate's core code falls foul of this. This patch causes a panic when copying
> memory to build the 'image'[0].
> saveable_page() in kernel/power/snapshot.c broadly assumes that pfn_valid()
> pages can be accessed.
> 
> Fortunately the core code exposes pfn_is_nosave() which we can extend to catch
> 'nomap' pages, but only if they are also marked as PageReserved().
> 
> Are there any side-effects of marking all the nomap regions with
> mark_page_reserved()? (it doesn't appear to be the case today).

Reserving the page adds it to the memory management which is what we
would like to avoid for NOMAP pages. I don't believe we should do
this. Since NOMAP is to some degree now core functionality I would
rather implement pfn_is_nomap() that defaults to pfn_is_valid() but
calls memblock_is_nomap() for arm64 or does something equivalent.

The question arises what to do with that mem at all. There could be
mappings by the kernel, e.g. of acpi tables. We can't assume the mem
regions still come out the same from the BIOS during resume. Do we
need to save the mem? I can't answer that as I don't know much about
hibernation yet.

Thanks,

-Robert