archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <>
To: Anshuman Khandual <>,
Cc: Mark Rutland <>,,,, Steven Price <>,
	Andrew Morton <>,
	Michal Hocko <>,
	Robin Murphy <>,
	Ard Biesheuvel <>
Subject: Re: [PATCH] arm64/mm: Validate hotplug range before creating linear mapping
Date: Wed, 7 Oct 2020 10:39:02 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

>> We do have __add_pages()->check_hotplug_memory_addressable() where we
>> already check against MAX_PHYSMEM_BITS.
> Initially, I thought about check_hotplug_memory_addressable() but the
> existing check that asserts end of hotplug wrt MAX_PHYSMEM_BITS, is
> generic in nature. AFAIK the linear mapping problem is arm64 specific,
> hence I was not sure whether to add an arch specific callback which
> will give platform an opportunity to weigh in for these ranges.

Also on s390x, the range where you can create an identity mapping depends on
- early kernel setup
- kasan

(I assume it's the same for all archs)

See arch/s390/mm/vmem.c:vmem_add_mapping(), which contains similar
checks (VMEM_MAX_PHYS).

> But hold on, check_hotplug_memory_addressable() only gets called from
> __add_pages() after linear mapping creation in arch_add_memory(). How
> would it help ? We need some thing for add_memory(), its variants and
> also possibly for memremap_pages() when it calls arch_add_memory().

Good point. We chose that place for simplicity when adding it (I was
favoring calling it at two places back then). Now, we might have good
reason to move the checks further up the call chain.

Most probably,

struct range memhp_get_addressable_range(bool need_mapping)

Would make sense, to deal with memremap_pages() without identity mappings.

We have two options:

1. Generalize the checks, check early in applicable functions. Have a
single way to get applicable ranges, both in callers, and inside the

2. Keep the checks where they are. Add memhp_get_addressable_range() so
callers can figure limits out. It's less clear what the relation between
the different checks is. And it's likely if things change at one place
that we miss the other place.

>> struct range memhp_get_addressable_range(void)
>> {
>> 	const u64 max_phys = (1ull << (MAX_PHYSMEM_BITS + 1)) - 1;
>> 	struct range range = arch_get_mappable_range();
> What would you suggest as the default fallback range if a platform
> does not define this callback.

Just the largest possible range until we implement them. IIRC, an s390x
version should be easy to add.

>> 	if (range.start > max_phys) {
>> 		range.start = 0;
>> 		range.end = 0;
>> 	}
>> 	range.end = max_t(u64, range.end, max_phys);
> min_t instead ?

Yeah :)

>> 	return range;
>> }
>> That, we can use in check_hotplug_memory_addressable(), and also allow
>> add_memory*() users to make use of it.
> So this check would happen twice during a hotplug ?

Right now it's like calling a function with wrong arguments - you just
don't have a clue what valid arguments are, because non-obvious errors
(besides -ENOMEM, which is a temporary error) pop up deep down the call

For example, virito-mem would use it to detect during device
initialization the usable device range, and warn the user accordingly.
It currently manually checks for MAX_PHYSMEM_BITS, but that's just ugly.
Failing at random add_memory() calls (permanently!) is not so nice.

In case of DIMMs, we could use it to detect if adding parts of a DIMM
won't work (and warn the user early). We could try to add as much as


David / dhildenb

linux-arm-kernel mailing list

  reply	other threads:[~2020-10-07  8:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-17  8:46 [PATCH] arm64/mm: Validate hotplug range before creating linear mapping Anshuman Khandual
2020-09-28 20:35 ` Will Deacon
2020-09-29  8:04   ` Anshuman Khandual
2020-09-29 15:22     ` Will Deacon
2020-09-30  8:02       ` Anshuman Khandual
2020-09-30 11:01         ` Ard Biesheuvel
2020-10-06  6:28           ` Anshuman Khandual
2020-10-06  6:35         ` Anshuman Khandual
2020-10-12  7:29           ` Ard Biesheuvel
2020-10-14  5:06             ` Anshuman Khandual
2020-10-14  6:37               ` Ard Biesheuvel
2020-10-06 15:34 ` David Hildenbrand
2020-10-07  2:50   ` Anshuman Khandual
2020-10-07  8:39     ` David Hildenbrand [this message]
2020-10-19 11:23       ` Anshuman Khandual
2020-10-19 14:58         ` David Hildenbrand

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).