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 20:26:59 +0100	[thread overview]
Message-ID: <20161124192659.GH2213@rric.localdomain> (raw)
In-Reply-To: <20161124150918.GF2213@rric.localdomain>

Ard,

> > >> On 24 November 2016 at 13:51, Robert Richter <robert.richter@cavium.com> wrote:
> > >> > On 24.11.16 13:44:31, Ard Biesheuvel wrote:

> > >> Regions containing firmware tables are owned by the firmware, and it
> > >> is the firmware that tells us which memory attributes we are allowed
> > >> to use. If those attributes include WB, it is perfectly legal to use a
> > >> cacheable mapping. That does *not* mean they should be covered by the
> > >> linear mapping. The linear mapping is read-write-non-exec, for
> > >> instance, and we may prefer to use a read-only mapping and/or
> > >> executable mapping.
> > >
> > > Ok, I am going to fix try_ram_remap().

I revisited the code and it is working well already since:

 e7cd190385d1 arm64: mark reserved memblock regions explicitly in iomem

Now, try_ram_remap() is only called if the region to be mapped is
entirely in IORESOURCE_SYSTEM_RAM. This is only true for normal mem
ranges and not NOMAP mem. region_intersects() then returns
REGION_INTERSECTS and calls try_ram_remap(). For the NOMAP memory case
REGION_DISJOINT would be returned and thus arch_memremap_wb() being
called directly. Before the e7cd190385d1 change try_ram_remap() was
called also for nomap regions.

So we can leave memremap() as it is and just apply this patch
unmodified. What do you think? Please ack.

I am going to prepare the pfn_is_ram() change in addition to this
patch, but that should not be required for this fix to work correcly.

Thanks,

-Robert

WARNING: multiple messages have this Message-ID (diff)
From: Robert Richter <robert.richter-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
To: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
	Robert Richter <rric-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>,
	David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>,
	Hanjun Guo <hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
Date: Thu, 24 Nov 2016 20:26:59 +0100	[thread overview]
Message-ID: <20161124192659.GH2213@rric.localdomain> (raw)
In-Reply-To: <20161124150918.GF2213-vWBEXY7mpu582hYKe6nXyg@public.gmane.org>

Ard,

> > >> On 24 November 2016 at 13:51, Robert Richter <robert.richter-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org> wrote:
> > >> > On 24.11.16 13:44:31, Ard Biesheuvel wrote:

> > >> Regions containing firmware tables are owned by the firmware, and it
> > >> is the firmware that tells us which memory attributes we are allowed
> > >> to use. If those attributes include WB, it is perfectly legal to use a
> > >> cacheable mapping. That does *not* mean they should be covered by the
> > >> linear mapping. The linear mapping is read-write-non-exec, for
> > >> instance, and we may prefer to use a read-only mapping and/or
> > >> executable mapping.
> > >
> > > Ok, I am going to fix try_ram_remap().

I revisited the code and it is working well already since:

 e7cd190385d1 arm64: mark reserved memblock regions explicitly in iomem

Now, try_ram_remap() is only called if the region to be mapped is
entirely in IORESOURCE_SYSTEM_RAM. This is only true for normal mem
ranges and not NOMAP mem. region_intersects() then returns
REGION_INTERSECTS and calls try_ram_remap(). For the NOMAP memory case
REGION_DISJOINT would be returned and thus arch_memremap_wb() being
called directly. Before the e7cd190385d1 change try_ram_remap() was
called also for nomap regions.

So we can leave memremap() as it is and just apply this patch
unmodified. What do you think? Please ack.

I am going to prepare the pfn_is_ram() change in addition to this
patch, but that should not be required for this fix to work correcly.

Thanks,

-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 20:26:59 +0100	[thread overview]
Message-ID: <20161124192659.GH2213@rric.localdomain> (raw)
In-Reply-To: <20161124150918.GF2213@rric.localdomain>

Ard,

> > >> On 24 November 2016 at 13:51, Robert Richter <robert.richter@cavium.com> wrote:
> > >> > On 24.11.16 13:44:31, Ard Biesheuvel wrote:

> > >> Regions containing firmware tables are owned by the firmware, and it
> > >> is the firmware that tells us which memory attributes we are allowed
> > >> to use. If those attributes include WB, it is perfectly legal to use a
> > >> cacheable mapping. That does *not* mean they should be covered by the
> > >> linear mapping. The linear mapping is read-write-non-exec, for
> > >> instance, and we may prefer to use a read-only mapping and/or
> > >> executable mapping.
> > >
> > > Ok, I am going to fix try_ram_remap().

I revisited the code and it is working well already since:

 e7cd190385d1 arm64: mark reserved memblock regions explicitly in iomem

Now, try_ram_remap() is only called if the region to be mapped is
entirely in IORESOURCE_SYSTEM_RAM. This is only true for normal mem
ranges and not NOMAP mem. region_intersects() then returns
REGION_INTERSECTS and calls try_ram_remap(). For the NOMAP memory case
REGION_DISJOINT would be returned and thus arch_memremap_wb() being
called directly. Before the e7cd190385d1 change try_ram_remap() was
called also for nomap regions.

So we can leave memremap() as it is and just apply this patch
unmodified. What do you think? Please ack.

I am going to prepare the pfn_is_ram() change in addition to this
patch, but that should not be required for this fix to work correcly.

Thanks,

-Robert

  reply	other threads:[~2016-11-24 20:01 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
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 [this message]
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=20161124192659.GH2213@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.