All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Richter <robert.richter@cavium.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>,
	Robert Richter <rric@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	David Daney <david.daney@cavium.com>,
	Hanjun Guo <hanjun.guo@linaro.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
Date: Thu, 24 Nov 2016 14:42:38 +0100	[thread overview]
Message-ID: <20161124134238.GI10776@rric.localdomain> (raw)
In-Reply-To: <CAKv+Gu8aWpz5vXSWdKHwL5Xk2aWyQBOoR1WHOnVPaWZOwqojBQ@mail.gmail.com>

On 23.11.16 21:25:06, Ard Biesheuvel wrote:
> Why? MEMREMAP_WB is used often, among other things for mapping
> firmware tables, which are marked as NOMAP, so in these cases, the
> linear address is not mapped.

If fw tables are mapped wb, that is wrong and needs a separate fix.

> > If you think pfn_valid() is wrong here, I am happy to send a patch
> > that fixes this by using page_is_ram(). In any case, the worst case
> > that may happen is to behave the same as v4.4, we might fix then the
> > wrong use of pfn_valid() where it is not correctly used to check for
> > ram.
> >
> 
> page_is_ram() uses string comparisons to look for regions called
> 'System RAM'. Is that something we can tolerate for each pfn_valid()
> calll?
> 
> Perhaps the solution is to reimplement page_is_ram() for arm64 using
> memblock_is_memory() instead, But that still means we need to modify
> the generic memremap() code first to switch to it before changing the
> arm64 implementation of pfn_valid

No, that's not the solution. pfn_valid() should just check if there is
a valid struct page, as other archs do. And if there is a mis-use of
pfn_valid() to check for ram, only that calls should be fixed to use
page_is_ram(), however this is implemented, or something appropriate.
But I don't see any problematic code, and if so, I will fix that.

-Robert

WARNING: multiple messages have this Message-ID (diff)
From: Robert Richter <robert.richter@cavium.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	"linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
	Robert Richter <rric@kernel.org>,
	David Daney <david.daney@cavium.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Hanjun Guo <hanjun.guo@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
Date: Thu, 24 Nov 2016 14:42:38 +0100	[thread overview]
Message-ID: <20161124134238.GI10776@rric.localdomain> (raw)
In-Reply-To: <CAKv+Gu8aWpz5vXSWdKHwL5Xk2aWyQBOoR1WHOnVPaWZOwqojBQ@mail.gmail.com>

On 23.11.16 21:25:06, Ard Biesheuvel wrote:
> Why? MEMREMAP_WB is used often, among other things for mapping
> firmware tables, which are marked as NOMAP, so in these cases, the
> linear address is not mapped.

If fw tables are mapped wb, that is wrong and needs a separate fix.

> > If you think pfn_valid() is wrong here, I am happy to send a patch
> > that fixes this by using page_is_ram(). In any case, the worst case
> > that may happen is to behave the same as v4.4, we might fix then the
> > wrong use of pfn_valid() where it is not correctly used to check for
> > ram.
> >
> 
> page_is_ram() uses string comparisons to look for regions called
> 'System RAM'. Is that something we can tolerate for each pfn_valid()
> calll?
> 
> Perhaps the solution is to reimplement page_is_ram() for arm64 using
> memblock_is_memory() instead, But that still means we need to modify
> the generic memremap() code first to switch to it before changing the
> arm64 implementation of pfn_valid

No, that's not the solution. pfn_valid() should just check if there is
a valid struct page, as other archs do. And if there is a mis-use of
pfn_valid() to check for ram, only that calls should be fixed to use
page_is_ram(), however this is implemented, or something appropriate.
But I don't see any problematic code, and if so, I will fix that.

-Robert

WARNING: multiple messages have this Message-ID (diff)
From: robert.richter@cavium.com (Robert Richter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
Date: Thu, 24 Nov 2016 14:42:38 +0100	[thread overview]
Message-ID: <20161124134238.GI10776@rric.localdomain> (raw)
In-Reply-To: <CAKv+Gu8aWpz5vXSWdKHwL5Xk2aWyQBOoR1WHOnVPaWZOwqojBQ@mail.gmail.com>

On 23.11.16 21:25:06, Ard Biesheuvel wrote:
> Why? MEMREMAP_WB is used often, among other things for mapping
> firmware tables, which are marked as NOMAP, so in these cases, the
> linear address is not mapped.

If fw tables are mapped wb, that is wrong and needs a separate fix.

> > If you think pfn_valid() is wrong here, I am happy to send a patch
> > that fixes this by using page_is_ram(). In any case, the worst case
> > that may happen is to behave the same as v4.4, we might fix then the
> > wrong use of pfn_valid() where it is not correctly used to check for
> > ram.
> >
> 
> page_is_ram() uses string comparisons to look for regions called
> 'System RAM'. Is that something we can tolerate for each pfn_valid()
> calll?
> 
> Perhaps the solution is to reimplement page_is_ram() for arm64 using
> memblock_is_memory() instead, But that still means we need to modify
> the generic memremap() code first to switch to it before changing the
> arm64 implementation of pfn_valid

No, that's not the solution. pfn_valid() should just check if there is
a valid struct page, as other archs do. And if there is a mis-use of
pfn_valid() to check for ram, only that calls should be fixed to use
page_is_ram(), however this is implemented, or something appropriate.
But I don't see any problematic code, and if so, I will fix that.

-Robert

  reply	other threads:[~2016-11-24 13:58 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-06  9:52 [PATCH] arm64: mm: Fix memmap to be initialized for the entire section Robert Richter
2016-10-06  9:52 ` Robert Richter
2016-10-06  9:52 ` Robert Richter
2016-10-06 10:00 ` Ard Biesheuvel
2016-10-06 10:00   ` Ard Biesheuvel
2016-10-06 10:00   ` Ard Biesheuvel
2016-10-06 16:11   ` Robert Richter
2016-10-06 16:11     ` Robert Richter
2016-10-06 16:11     ` Robert Richter
2016-10-17 18:58     ` Robert Richter
2016-10-17 18:58       ` Robert Richter
2016-10-17 18:58       ` Robert Richter
2016-10-27 16:01       ` Will Deacon
2016-10-27 16:01         ` Will Deacon
2016-10-27 16:01         ` Will Deacon
2016-10-28  9:19         ` Robert Richter
2016-10-28  9:19           ` Robert Richter
2016-10-28  9:19           ` Robert Richter
2016-11-07 21:05           ` Will Deacon
2016-11-07 21:05             ` Will Deacon
2016-11-07 21:05             ` Will Deacon
2016-11-09 19:51             ` Robert Richter
2016-11-09 19:51               ` Robert Richter
2016-11-09 19:51               ` Robert Richter
2016-11-17 14:25               ` Will Deacon
2016-11-17 14:25                 ` Will Deacon
2016-11-17 14:25                 ` Will Deacon
2016-11-17 15:18                 ` Robert Richter
2016-11-17 15:18                   ` Robert Richter
2016-11-17 15:18                   ` Robert Richter
2016-11-20 17:07                   ` Ard Biesheuvel
2016-11-20 17:07                     ` Ard Biesheuvel
2016-11-20 17:07                     ` Ard Biesheuvel
2016-11-23 21:15                     ` Robert Richter
2016-11-23 21:15                       ` Robert Richter
2016-11-23 21:15                       ` Robert Richter
2016-11-23 21:25                       ` Ard Biesheuvel
2016-11-23 21:25                         ` Ard Biesheuvel
2016-11-23 21:25                         ` Ard Biesheuvel
2016-11-24 13:42                         ` Robert Richter [this message]
2016-11-24 13:42                           ` Robert Richter
2016-11-24 13:42                           ` Robert Richter
2016-11-24 13:44                           ` Ard Biesheuvel
2016-11-24 13:44                             ` Ard Biesheuvel
2016-11-24 13:44                             ` Ard Biesheuvel
2016-11-24 13:51                             ` Robert Richter
2016-11-24 13:51                               ` Robert Richter
2016-11-24 13:51                               ` Robert Richter
2016-11-24 13:58                               ` Ard Biesheuvel
2016-11-24 13:58                                 ` Ard Biesheuvel
2016-11-24 13:58                                 ` Ard Biesheuvel
2016-11-24 14:11                                 ` Robert Richter
2016-11-24 14:11                                   ` Robert Richter
2016-11-24 14:11                                   ` Robert Richter
2016-11-24 14:23                                   ` Ard Biesheuvel
2016-11-24 14:23                                     ` Ard Biesheuvel
2016-11-24 14:23                                     ` Ard Biesheuvel
2016-11-24 15:09                                     ` Robert Richter
2016-11-24 15:09                                       ` Robert Richter
2016-11-24 15:09                                       ` Robert Richter
2016-11-24 19:26                                       ` Robert Richter
2016-11-24 19:26                                         ` Robert Richter
2016-11-24 19:26                                         ` Robert Richter
2016-11-24 19:42                                         ` Ard Biesheuvel
2016-11-24 19:42                                           ` Ard Biesheuvel
2016-11-24 19:42                                           ` Ard Biesheuvel
2016-11-25 11:29                                           ` Robert Richter
2016-11-25 11:29                                             ` Robert Richter
2016-11-25 11:29                                             ` Robert Richter
2016-11-25 12:28                                             ` Ard Biesheuvel
2016-11-25 12:28                                               ` Ard Biesheuvel
2016-11-25 12:28                                               ` Ard Biesheuvel
2016-11-25 17:01                                               ` Ard Biesheuvel
2016-11-25 17:01                                                 ` Ard Biesheuvel
2016-11-25 17:01                                                 ` Ard Biesheuvel
2016-10-18 10:18     ` Mark Rutland
2016-10-18 10:18       ` Mark Rutland
2016-10-18 10:18       ` Mark Rutland
2016-10-18 15:02       ` Robert Richter
2016-10-18 15:02         ` Robert Richter
2016-10-18 15:02         ` Robert Richter
2016-10-10 15:33 ` David Daney
2016-10-10 15:33   ` David Daney
2016-11-01 16:55 ` Robert Richter
2016-11-01 16:55   ` Robert Richter
2016-11-01 16:55   ` Robert Richter

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=20161124134238.GI10776@rric.localdomain \
    --to=robert.richter@cavium.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=david.daney@cavium.com \
    --cc=hanjun.guo@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=rric@kernel.org \
    --cc=will.deacon@arm.com \
    /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.