linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Anshuman Khandual <anshuman.khandual@arm.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	David Hildenbrand <david@redhat.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Steven Price <steven.price@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Will Deacon <will@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Robin Murphy <robin.murphy@arm.com>
Subject: Re: [PATCH] arm64/mm: Validate hotplug range before creating linear mapping
Date: Wed, 14 Oct 2020 10:36:32 +0530	[thread overview]
Message-ID: <1ee71de5-9e16-b9de-6ea0-f17dc9a494ac@arm.com> (raw)
In-Reply-To: <CAMj1kXFszo8SO7eAn0FEO+AQUHV9HZyukUi7=-udKyK+mCNVRw@mail.gmail.com>



On 10/12/2020 12:59 PM, Ard Biesheuvel wrote:
> On Tue, 6 Oct 2020 at 08:36, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>>
>>
>> On 09/30/2020 01:32 PM, Anshuman Khandual wrote:
>>> But if __is_lm_address() checks against the effective linear range instead
>>> i.e [_PAGE_OFFSET(vabits_actual)..(PAGE_END - 1)], it can be used for hot
>>> plug physical range check there after. Perhaps something like this, though
>>> not tested properly.
>>>
>>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>>> index afa722504bfd..6da046b479d4 100644
>>> --- a/arch/arm64/include/asm/memory.h
>>> +++ b/arch/arm64/include/asm/memory.h
>>> @@ -238,7 +238,10 @@ static inline const void *__tag_set(const void *addr, u8 tag)
>>>   * space. Testing the top bit for the start of the region is a
>>>   * sufficient check and avoids having to worry about the tag.
>>>   */
>>> -#define __is_lm_address(addr)  (!(((u64)addr) & BIT(vabits_actual - 1)))
>>> +static inline bool __is_lm_address(unsigned long addr)
>>> +{
>>> +       return ((addr >= _PAGE_OFFSET(vabits_actual)) && (addr <= (PAGE_END - 1)));
>>> +}
>>>
>>>  #define __lm_to_phys(addr)     (((addr) + physvirt_offset))
>>>  #define __kimg_to_phys(addr)   ((addr) - kimage_voffset)
>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>> index d59ffabb9c84..5750370a7e8c 100644
>>> --- a/arch/arm64/mm/mmu.c
>>> +++ b/arch/arm64/mm/mmu.c
>>> @@ -1451,8 +1451,7 @@ static bool inside_linear_region(u64 start, u64 size)
>>>          * address range mapped by the linear map, the start address should
>>>          * be calculated using vabits_actual.
>>>          */
>>> -       return ((start >= __pa(_PAGE_OFFSET(vabits_actual)))
>>> -                       && ((start + size) <= __pa(PAGE_END - 1)));
>>> +       return __is_lm_address(__va(start)) && __is_lm_address(__va(start + size));
>>>  }
>>>
>>>  int arch_add_memory(int nid, u64 start, u64 size,
>>
>> Will/Ard,
>>
>> Any thoughts about this ? __is_lm_address() now checks for a range instead
>> of a bit. This will be compatible later on, even if linear mapping range
>> changes from current lower half scheme.
>>
> 
> As I'm sure you have noticed, I sent out some patches that get rid of
> physvirt_offset, and which simplify __is_lm_address() to only take
> compile time constants into account (unless KASAN is enabled). This
> means that in the 52-bit VA case, __is_lm_address() does not
> distinguish between virtual addresses that can be mapped by the
> hardware and ones that cannot.

Yeah, though was bit late in getting to the series. So with that change
there might be areas in the linear mapping which cannot be addressed by
the hardware and hence should also need be checked apart from proposed
linear mapping coverage test, during memory hotplug ?

> 
> In the memory hotplug case, we need to decide whether the added memory
> will appear in the addressable area, which is a different question. So
> it makes sense to duplicate some of the logic that exists in
> arm64_memblock_init() (or factor it out) to decide whether this newly
> added memory will appear in the addressable window or not.

It seems unlikely that any hotplug agent (e.g. firmware) will ever push
through a memory range which is not accessible in the hardware but then
it is not impossible either. In summary, arch_add_memory() should check

1. Range can be covered inside linear mapping
2. Range is accessible by the hardware

Before the VA space organization series, (2) was not necessary as it was
contained inside (1) ?

> 
> So I think your original approach makes more sense here, although I
> think you want '(start + size - 1) <= __pa(PAGE_END - 1)' in the
> comparison above (and please drop the redundant parens)
> 

Sure, will accommodate these changes.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-10-14  5:08 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 [this message]
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
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:
  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=1ee71de5-9e16-b9de-6ea0-f17dc9a494ac@arm.com \
    --to=anshuman.khandual@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=ardb@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=david@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robin.murphy@arm.com \
    --cc=steven.price@arm.com \
    --cc=will@kernel.org \
    /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 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).