diff mbox series

[5.11,225/254] arm64/mm: define arch_get_mappable_range()

Message ID 20210329075640.480623043@linuxfoundation.org
State New
Headers show
Series None | expand

Commit Message

Greg Kroah-Hartman March 29, 2021, 7:59 a.m. UTC
From: Anshuman Khandual <anshuman.khandual@arm.com>


[ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

This overrides arch_get_mappable_range() on arm64 platform which will be
used with recently added generic framework.  It drops
inside_linear_region() and subsequent check in arch_add_memory() which are
no longer required.  It also adds a VM_BUG_ON() check that would ensure
that mhp_range_allowed() has already been called.

Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

Reviewed-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>
Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
Cc: teawater <teawaterz@linux.alibaba.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Wei Yang <richard.weiyang@linux.alibaba.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Signed-off-by: Sasha Levin <sashal@kernel.org>

---
 arch/arm64/mm/mmu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

-- 
2.30.1

Comments

Naresh Kamboju March 29, 2021, 9:35 a.m. UTC | #1
On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>

> From: Anshuman Khandual <anshuman.khandual@arm.com>

>

> [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

>

> This overrides arch_get_mappable_range() on arm64 platform which will be

> used with recently added generic framework.  It drops

> inside_linear_region() and subsequent check in arch_add_memory() which are

> no longer required.  It also adds a VM_BUG_ON() check that would ensure

> that mhp_range_allowed() has already been called.

>

> Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> Reviewed-by: David Hildenbrand <david@redhat.com>

> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> Cc: Will Deacon <will@kernel.org>

> Cc: Ard Biesheuvel <ardb@kernel.org>

> Cc: Mark Rutland <mark.rutland@arm.com>

> Cc: Heiko Carstens <hca@linux.ibm.com>

> Cc: Jason Wang <jasowang@redhat.com>

> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> Cc: "Michael S. Tsirkin" <mst@redhat.com>

> Cc: Michal Hocko <mhocko@kernel.org>

> Cc: Oscar Salvador <osalvador@suse.de>

> Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> Cc: teawater <teawaterz@linux.alibaba.com>

> Cc: Vasily Gorbik <gor@linux.ibm.com>

> Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> Signed-off-by: Sasha Levin <sashal@kernel.org>

> ---

>  arch/arm64/mm/mmu.c | 15 +++++++--------

>  1 file changed, 7 insertions(+), 8 deletions(-)

>

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> index 6f0648777d34..92b3be127796 100644

> --- a/arch/arm64/mm/mmu.c

> +++ b/arch/arm64/mm/mmu.c

> @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

>         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

>  }

>

> -static bool inside_linear_region(u64 start, u64 size)

> +struct range arch_get_mappable_range(void)

>  {

> +       struct range mhp_range;

> +

>         /*

>          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

>          * accommodating both its ends but excluding PAGE_END. Max physical

>          * range which can be mapped inside this linear mapping range, must

>          * also be derived from its end points.

>          */

> -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> -              (start + size - 1) <= __pa(PAGE_END - 1);

> +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> +       mhp_range.end =  __pa(PAGE_END - 1);

> +       return mhp_range;

>  }

>

>  int arch_add_memory(int nid, u64 start, u64 size,

> @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

>  {

>         int ret, flags = 0;

>

> -       if (!inside_linear_region(start, size)) {

> -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> -               return -EINVAL;

> -       }

> -

> +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

>         if (rodata_full || debug_pagealloc_enabled())

>                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;


The stable rc 5.10 and 5.11 builds failed for arm64 architecture
due to below warnings / errors,

> Anshuman Khandual <anshuman.khandual@arm.com>

>     arm64/mm: define arch_get_mappable_range()



  arch/arm64/mm/mmu.c: In function 'arch_add_memory':
  arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function
'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?
[-Werror=implicit-function-declaration]
    VM_BUG_ON(!mhp_range_allowed(start, size, true));
               ^
  include/linux/build_bug.h:30:63: note: in definition of macro
'BUILD_BUG_ON_INVALID'
   #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
                                                                 ^
  arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'
    VM_BUG_ON(!mhp_range_allowed(start, size, true));
    ^~~~~~~~~

Build link,
https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText
https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

--
Linaro LKFT
https://lkft.linaro.org
Greg Kroah-Hartman March 29, 2021, 10:12 a.m. UTC | #2
On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:
> On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> <gregkh@linuxfoundation.org> wrote:

> >

> > From: Anshuman Khandual <anshuman.khandual@arm.com>

> >

> > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> >

> > This overrides arch_get_mappable_range() on arm64 platform which will be

> > used with recently added generic framework.  It drops

> > inside_linear_region() and subsequent check in arch_add_memory() which are

> > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > that mhp_range_allowed() has already been called.

> >

> > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > Reviewed-by: David Hildenbrand <david@redhat.com>

> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > Cc: Will Deacon <will@kernel.org>

> > Cc: Ard Biesheuvel <ardb@kernel.org>

> > Cc: Mark Rutland <mark.rutland@arm.com>

> > Cc: Heiko Carstens <hca@linux.ibm.com>

> > Cc: Jason Wang <jasowang@redhat.com>

> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > Cc: Michal Hocko <mhocko@kernel.org>

> > Cc: Oscar Salvador <osalvador@suse.de>

> > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > Cc: teawater <teawaterz@linux.alibaba.com>

> > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > ---

> >  arch/arm64/mm/mmu.c | 15 +++++++--------

> >  1 file changed, 7 insertions(+), 8 deletions(-)

> >

> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > index 6f0648777d34..92b3be127796 100644

> > --- a/arch/arm64/mm/mmu.c

> > +++ b/arch/arm64/mm/mmu.c

> > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> >  }

> >

> > -static bool inside_linear_region(u64 start, u64 size)

> > +struct range arch_get_mappable_range(void)

> >  {

> > +       struct range mhp_range;

> > +

> >         /*

> >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> >          * accommodating both its ends but excluding PAGE_END. Max physical

> >          * range which can be mapped inside this linear mapping range, must

> >          * also be derived from its end points.

> >          */

> > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > +       mhp_range.end =  __pa(PAGE_END - 1);

> > +       return mhp_range;

> >  }

> >

> >  int arch_add_memory(int nid, u64 start, u64 size,

> > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> >  {

> >         int ret, flags = 0;

> >

> > -       if (!inside_linear_region(start, size)) {

> > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > -               return -EINVAL;

> > -       }

> > -

> > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> >         if (rodata_full || debug_pagealloc_enabled())

> >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> 

> The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> due to below warnings / errors,

> 

> > Anshuman Khandual <anshuman.khandual@arm.com>

> >     arm64/mm: define arch_get_mappable_range()

> 

> 

>   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

>   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> [-Werror=implicit-function-declaration]

>     VM_BUG_ON(!mhp_range_allowed(start, size, true));

>                ^

>   include/linux/build_bug.h:30:63: note: in definition of macro

> 'BUILD_BUG_ON_INVALID'

>    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

>                                                                  ^

>   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

>     VM_BUG_ON(!mhp_range_allowed(start, size, true));

>     ^~~~~~~~~

> 

> Build link,

> https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull


thanks, will go drop this, and the patch that was after it in the
series, from both trees and will push out a -rc2.

Note, I used tuxbuild before doing this release, and it does not show
this error in the arm64 defconfigs.  What config did you use to trigger
this?

thanks,

greg k-h
Anders Roxell March 29, 2021, 11:06 a.m. UTC | #3
On Mon, 29 Mar 2021 at 12:13, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>

> On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > <gregkh@linuxfoundation.org> wrote:

> > >

> > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > >

> > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > >

> > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > used with recently added generic framework.  It drops

> > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > that mhp_range_allowed() has already been called.

> > >

> > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > Cc: Will Deacon <will@kernel.org>

> > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > Cc: Jason Wang <jasowang@redhat.com>

> > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > Cc: Michal Hocko <mhocko@kernel.org>

> > > Cc: Oscar Salvador <osalvador@suse.de>

> > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > ---

> > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > >

> > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > index 6f0648777d34..92b3be127796 100644

> > > --- a/arch/arm64/mm/mmu.c

> > > +++ b/arch/arm64/mm/mmu.c

> > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > >  }

> > >

> > > -static bool inside_linear_region(u64 start, u64 size)

> > > +struct range arch_get_mappable_range(void)

> > >  {

> > > +       struct range mhp_range;

> > > +

> > >         /*

> > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > >          * range which can be mapped inside this linear mapping range, must

> > >          * also be derived from its end points.

> > >          */

> > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > +       return mhp_range;

> > >  }

> > >

> > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > >  {

> > >         int ret, flags = 0;

> > >

> > > -       if (!inside_linear_region(start, size)) {

> > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > -               return -EINVAL;

> > > -       }

> > > -

> > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > >         if (rodata_full || debug_pagealloc_enabled())

> > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> >

> > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > due to below warnings / errors,

> >

> > > Anshuman Khandual <anshuman.khandual@arm.com>

> > >     arm64/mm: define arch_get_mappable_range()

> >

> >

> >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > [-Werror=implicit-function-declaration]

> >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> >                ^

> >   include/linux/build_bug.h:30:63: note: in definition of macro

> > 'BUILD_BUG_ON_INVALID'

> >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> >                                                                  ^

> >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> >     ^~~~~~~~~

> >

> > Build link,

> > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

>

> thanks, will go drop this, and the patch that was after it in the

> series, from both trees and will push out a -rc2.

>

> Note, I used tuxbuild before doing this release, and it does not show

> this error in the arm64 defconfigs. What config did you use to trigger

> this?


We have a build with CONFIG_MEMORY_HOTPLUG=y enabled too.

This is a way to reproduce it locally:
tuxmake --runtime podman --target-arch arm64 --toolchain gcc --kconfig
defconfig --kconfig-add CONFIG_MEMORY_HOTPLUG=y

Cheers,
Anders
Greg Kroah-Hartman March 29, 2021, 11:35 a.m. UTC | #4
On Mon, Mar 29, 2021 at 01:06:47PM +0200, Anders Roxell wrote:
> On Mon, 29 Mar 2021 at 12:13, Greg Kroah-Hartman

> <gregkh@linuxfoundation.org> wrote:

> >

> > On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > > <gregkh@linuxfoundation.org> wrote:

> > > >

> > > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > > >

> > > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > > >

> > > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > > used with recently added generic framework.  It drops

> > > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > > that mhp_range_allowed() has already been called.

> > > >

> > > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > > Cc: Will Deacon <will@kernel.org>

> > > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > > Cc: Jason Wang <jasowang@redhat.com>

> > > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > > Cc: Michal Hocko <mhocko@kernel.org>

> > > > Cc: Oscar Salvador <osalvador@suse.de>

> > > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > > ---

> > > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > > >

> > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > > index 6f0648777d34..92b3be127796 100644

> > > > --- a/arch/arm64/mm/mmu.c

> > > > +++ b/arch/arm64/mm/mmu.c

> > > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > > >  }

> > > >

> > > > -static bool inside_linear_region(u64 start, u64 size)

> > > > +struct range arch_get_mappable_range(void)

> > > >  {

> > > > +       struct range mhp_range;

> > > > +

> > > >         /*

> > > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > > >          * range which can be mapped inside this linear mapping range, must

> > > >          * also be derived from its end points.

> > > >          */

> > > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > > +       return mhp_range;

> > > >  }

> > > >

> > > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > > >  {

> > > >         int ret, flags = 0;

> > > >

> > > > -       if (!inside_linear_region(start, size)) {

> > > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > > -               return -EINVAL;

> > > > -       }

> > > > -

> > > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > >         if (rodata_full || debug_pagealloc_enabled())

> > > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> > >

> > > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > > due to below warnings / errors,

> > >

> > > > Anshuman Khandual <anshuman.khandual@arm.com>

> > > >     arm64/mm: define arch_get_mappable_range()

> > >

> > >

> > >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> > >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > > [-Werror=implicit-function-declaration]

> > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > >                ^

> > >   include/linux/build_bug.h:30:63: note: in definition of macro

> > > 'BUILD_BUG_ON_INVALID'

> > >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> > >                                                                  ^

> > >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > >     ^~~~~~~~~

> > >

> > > Build link,

> > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

> >

> > thanks, will go drop this, and the patch that was after it in the

> > series, from both trees and will push out a -rc2.

> >

> > Note, I used tuxbuild before doing this release, and it does not show

> > this error in the arm64 defconfigs. What config did you use to trigger

> > this?

> 

> We have a build with CONFIG_MEMORY_HOTPLUG=y enabled too.

> 

> This is a way to reproduce it locally:

> tuxmake --runtime podman --target-arch arm64 --toolchain gcc --kconfig

> defconfig --kconfig-add CONFIG_MEMORY_HOTPLUG=y


Ah, that wasn't expected, but makes sense, thanks.

Does 'allmodconfig' also trigger that?  Maybe I'll go add that to my
build tests for arm64...

thanks,

greg k-h
Ard Biesheuvel March 29, 2021, 1:08 p.m. UTC | #5
On Mon, 29 Mar 2021 at 12:12, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>

> On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > <gregkh@linuxfoundation.org> wrote:

> > >

> > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > >

> > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > >

> > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > used with recently added generic framework.  It drops

> > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > that mhp_range_allowed() has already been called.

> > >

> > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > Cc: Will Deacon <will@kernel.org>

> > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > Cc: Jason Wang <jasowang@redhat.com>

> > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > Cc: Michal Hocko <mhocko@kernel.org>

> > > Cc: Oscar Salvador <osalvador@suse.de>

> > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > ---

> > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > >

> > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > index 6f0648777d34..92b3be127796 100644

> > > --- a/arch/arm64/mm/mmu.c

> > > +++ b/arch/arm64/mm/mmu.c

> > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > >  }

> > >

> > > -static bool inside_linear_region(u64 start, u64 size)

> > > +struct range arch_get_mappable_range(void)

> > >  {

> > > +       struct range mhp_range;

> > > +

> > >         /*

> > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > >          * range which can be mapped inside this linear mapping range, must

> > >          * also be derived from its end points.

> > >          */

> > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > +       return mhp_range;

> > >  }

> > >

> > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > >  {

> > >         int ret, flags = 0;

> > >

> > > -       if (!inside_linear_region(start, size)) {

> > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > -               return -EINVAL;

> > > -       }

> > > -

> > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > >         if (rodata_full || debug_pagealloc_enabled())

> > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> >

> > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > due to below warnings / errors,

> >

> > > Anshuman Khandual <anshuman.khandual@arm.com>

> > >     arm64/mm: define arch_get_mappable_range()

> >

> >

> >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > [-Werror=implicit-function-declaration]

> >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> >                ^

> >   include/linux/build_bug.h:30:63: note: in definition of macro

> > 'BUILD_BUG_ON_INVALID'

> >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> >                                                                  ^

> >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> >     ^~~~~~~~~

> >

> > Build link,

> > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

>

> thanks, will go drop this, and the patch that was after it in the

> series, from both trees and will push out a -rc2.

>


Why were these picked up in the first place? I don't see any fixes or
cc:stable tags, and the commit log clearly describes that the change
is preparatory work for enabling arm64 support into a recently
introduced generic framework.

-- 
Ard.
Greg Kroah-Hartman March 29, 2021, 1:42 p.m. UTC | #6
On Mon, Mar 29, 2021 at 03:08:52PM +0200, Ard Biesheuvel wrote:
> On Mon, 29 Mar 2021 at 12:12, Greg Kroah-Hartman

> <gregkh@linuxfoundation.org> wrote:

> >

> > On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > > <gregkh@linuxfoundation.org> wrote:

> > > >

> > > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > > >

> > > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > > >

> > > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > > used with recently added generic framework.  It drops

> > > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > > that mhp_range_allowed() has already been called.

> > > >

> > > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > > Cc: Will Deacon <will@kernel.org>

> > > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > > Cc: Jason Wang <jasowang@redhat.com>

> > > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > > Cc: Michal Hocko <mhocko@kernel.org>

> > > > Cc: Oscar Salvador <osalvador@suse.de>

> > > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > > ---

> > > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > > >

> > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > > index 6f0648777d34..92b3be127796 100644

> > > > --- a/arch/arm64/mm/mmu.c

> > > > +++ b/arch/arm64/mm/mmu.c

> > > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > > >  }

> > > >

> > > > -static bool inside_linear_region(u64 start, u64 size)

> > > > +struct range arch_get_mappable_range(void)

> > > >  {

> > > > +       struct range mhp_range;

> > > > +

> > > >         /*

> > > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > > >          * range which can be mapped inside this linear mapping range, must

> > > >          * also be derived from its end points.

> > > >          */

> > > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > > +       return mhp_range;

> > > >  }

> > > >

> > > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > > >  {

> > > >         int ret, flags = 0;

> > > >

> > > > -       if (!inside_linear_region(start, size)) {

> > > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > > -               return -EINVAL;

> > > > -       }

> > > > -

> > > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > >         if (rodata_full || debug_pagealloc_enabled())

> > > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> > >

> > > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > > due to below warnings / errors,

> > >

> > > > Anshuman Khandual <anshuman.khandual@arm.com>

> > > >     arm64/mm: define arch_get_mappable_range()

> > >

> > >

> > >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> > >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > > [-Werror=implicit-function-declaration]

> > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > >                ^

> > >   include/linux/build_bug.h:30:63: note: in definition of macro

> > > 'BUILD_BUG_ON_INVALID'

> > >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> > >                                                                  ^

> > >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > >     ^~~~~~~~~

> > >

> > > Build link,

> > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

> >

> > thanks, will go drop this, and the patch that was after it in the

> > series, from both trees and will push out a -rc2.

> >

> 

> Why were these picked up in the first place? I don't see any fixes or

> cc:stable tags, and the commit log clearly describes that the change

> is preparatory work for enabling arm64 support into a recently

> introduced generic framework.


This was needed for a follow-on patch in the series that fixed an issue.
Specifically it was commit ee7febce0519 ("arm64: mm: correct the inside
linear map range during hotplug check")

thanks,

greg k-h
Ard Biesheuvel March 29, 2021, 1:49 p.m. UTC | #7
(+ Pavel)

On Mon, 29 Mar 2021 at 15:42, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>

> On Mon, Mar 29, 2021 at 03:08:52PM +0200, Ard Biesheuvel wrote:

> > On Mon, 29 Mar 2021 at 12:12, Greg Kroah-Hartman

> > <gregkh@linuxfoundation.org> wrote:

> > >

> > > On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > > > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > > > <gregkh@linuxfoundation.org> wrote:

> > > > >

> > > > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > >

> > > > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > > > >

> > > > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > > > used with recently added generic framework.  It drops

> > > > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > > > that mhp_range_allowed() has already been called.

> > > > >

> > > > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > > > Cc: Will Deacon <will@kernel.org>

> > > > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > > > Cc: Jason Wang <jasowang@redhat.com>

> > > > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > > > Cc: Michal Hocko <mhocko@kernel.org>

> > > > > Cc: Oscar Salvador <osalvador@suse.de>

> > > > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > > > ---

> > > > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > > > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > > > >

> > > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > > > index 6f0648777d34..92b3be127796 100644

> > > > > --- a/arch/arm64/mm/mmu.c

> > > > > +++ b/arch/arm64/mm/mmu.c

> > > > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > > > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > > > >  }

> > > > >

> > > > > -static bool inside_linear_region(u64 start, u64 size)

> > > > > +struct range arch_get_mappable_range(void)

> > > > >  {

> > > > > +       struct range mhp_range;

> > > > > +

> > > > >         /*

> > > > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > > > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > > > >          * range which can be mapped inside this linear mapping range, must

> > > > >          * also be derived from its end points.

> > > > >          */

> > > > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > > > +       return mhp_range;

> > > > >  }

> > > > >

> > > > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > > > >  {

> > > > >         int ret, flags = 0;

> > > > >

> > > > > -       if (!inside_linear_region(start, size)) {

> > > > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > > > -               return -EINVAL;

> > > > > -       }

> > > > > -

> > > > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > >         if (rodata_full || debug_pagealloc_enabled())

> > > > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> > > >

> > > > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > > > due to below warnings / errors,

> > > >

> > > > > Anshuman Khandual <anshuman.khandual@arm.com>

> > > > >     arm64/mm: define arch_get_mappable_range()

> > > >

> > > >

> > > >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> > > >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > > > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > > > [-Werror=implicit-function-declaration]

> > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > >                ^

> > > >   include/linux/build_bug.h:30:63: note: in definition of macro

> > > > 'BUILD_BUG_ON_INVALID'

> > > >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> > > >                                                                  ^

> > > >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > >     ^~~~~~~~~

> > > >

> > > > Build link,

> > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

> > >

> > > thanks, will go drop this, and the patch that was after it in the

> > > series, from both trees and will push out a -rc2.

> > >

> >

> > Why were these picked up in the first place? I don't see any fixes or

> > cc:stable tags, and the commit log clearly describes that the change

> > is preparatory work for enabling arm64 support into a recently

> > introduced generic framework.

>

> This was needed for a follow-on patch in the series that fixed an issue.

> Specifically it was commit ee7febce0519 ("arm64: mm: correct the inside

> linear map range during hotplug check")

>


Yeah, but during the discussion of that patch [0], we pointed out that
it needed to be rebased because of these new changes. So trying to
backport this rebased version is obviously not the right approach:
Pavel's original patch would be much more suitable for that.

Could we have annotated this patch in a better way to make this more obvious?


[0] https://lore.kernel.org/linux-arm-kernel/20210215192237.362706-2-pasha.tatashin@soleen.com/
Greg Kroah-Hartman March 29, 2021, 1:51 p.m. UTC | #8
On Mon, Mar 29, 2021 at 03:49:19PM +0200, Ard Biesheuvel wrote:
> (+ Pavel)

> 

> On Mon, 29 Mar 2021 at 15:42, Greg Kroah-Hartman

> <gregkh@linuxfoundation.org> wrote:

> >

> > On Mon, Mar 29, 2021 at 03:08:52PM +0200, Ard Biesheuvel wrote:

> > > On Mon, 29 Mar 2021 at 12:12, Greg Kroah-Hartman

> > > <gregkh@linuxfoundation.org> wrote:

> > > >

> > > > On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > > > > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > > > > <gregkh@linuxfoundation.org> wrote:

> > > > > >

> > > > > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > >

> > > > > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > > > > >

> > > > > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > > > > used with recently added generic framework.  It drops

> > > > > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > > > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > > > > that mhp_range_allowed() has already been called.

> > > > > >

> > > > > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > > > > Cc: Will Deacon <will@kernel.org>

> > > > > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > > > > Cc: Jason Wang <jasowang@redhat.com>

> > > > > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > > > > Cc: Michal Hocko <mhocko@kernel.org>

> > > > > > Cc: Oscar Salvador <osalvador@suse.de>

> > > > > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > > > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > > > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > > > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > > > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > > > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > > > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > > > > ---

> > > > > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > > > > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > > > > >

> > > > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > > > > index 6f0648777d34..92b3be127796 100644

> > > > > > --- a/arch/arm64/mm/mmu.c

> > > > > > +++ b/arch/arm64/mm/mmu.c

> > > > > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > > > > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > > > > >  }

> > > > > >

> > > > > > -static bool inside_linear_region(u64 start, u64 size)

> > > > > > +struct range arch_get_mappable_range(void)

> > > > > >  {

> > > > > > +       struct range mhp_range;

> > > > > > +

> > > > > >         /*

> > > > > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > > > > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > > > > >          * range which can be mapped inside this linear mapping range, must

> > > > > >          * also be derived from its end points.

> > > > > >          */

> > > > > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > > > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > > > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > > > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > > > > +       return mhp_range;

> > > > > >  }

> > > > > >

> > > > > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > > > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > > > > >  {

> > > > > >         int ret, flags = 0;

> > > > > >

> > > > > > -       if (!inside_linear_region(start, size)) {

> > > > > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > > > > -               return -EINVAL;

> > > > > > -       }

> > > > > > -

> > > > > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > > >         if (rodata_full || debug_pagealloc_enabled())

> > > > > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> > > > >

> > > > > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > > > > due to below warnings / errors,

> > > > >

> > > > > > Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > >     arm64/mm: define arch_get_mappable_range()

> > > > >

> > > > >

> > > > >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> > > > >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > > > > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > > > > [-Werror=implicit-function-declaration]

> > > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > >                ^

> > > > >   include/linux/build_bug.h:30:63: note: in definition of macro

> > > > > 'BUILD_BUG_ON_INVALID'

> > > > >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> > > > >                                                                  ^

> > > > >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> > > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > >     ^~~~~~~~~

> > > > >

> > > > > Build link,

> > > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

> > > >

> > > > thanks, will go drop this, and the patch that was after it in the

> > > > series, from both trees and will push out a -rc2.

> > > >

> > >

> > > Why were these picked up in the first place? I don't see any fixes or

> > > cc:stable tags, and the commit log clearly describes that the change

> > > is preparatory work for enabling arm64 support into a recently

> > > introduced generic framework.

> >

> > This was needed for a follow-on patch in the series that fixed an issue.

> > Specifically it was commit ee7febce0519 ("arm64: mm: correct the inside

> > linear map range during hotplug check")

> >

> 

> Yeah, but during the discussion of that patch [0], we pointed out that

> it needed to be rebased because of these new changes. So trying to

> backport this rebased version is obviously not the right approach:

> Pavel's original patch would be much more suitable for that.

> 

> Could we have annotated this patch in a better way to make this more obvious?


Yes, given that there was no annotation on the patch at all to let us
know this :)

You can say things like "do not apply to stable trees" or "needs total
rework for older kernels" or other fun such things that when we read
them, we know to ask for help.  As it is, the patch provided nothing so
we guessed and got it wrong...

thanks,

greg k-h
Pasha Tatashin March 29, 2021, 1:53 p.m. UTC | #9
On Mon, Mar 29, 2021 at 9:51 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>

> On Mon, Mar 29, 2021 at 03:49:19PM +0200, Ard Biesheuvel wrote:

> > (+ Pavel)

> >

> > On Mon, 29 Mar 2021 at 15:42, Greg Kroah-Hartman

> > <gregkh@linuxfoundation.org> wrote:

> > >

> > > On Mon, Mar 29, 2021 at 03:08:52PM +0200, Ard Biesheuvel wrote:

> > > > On Mon, 29 Mar 2021 at 12:12, Greg Kroah-Hartman

> > > > <gregkh@linuxfoundation.org> wrote:

> > > > >

> > > > > On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > > > > > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > > > > > <gregkh@linuxfoundation.org> wrote:

> > > > > > >

> > > > > > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > > >

> > > > > > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > > > > > >

> > > > > > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > > > > > used with recently added generic framework.  It drops

> > > > > > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > > > > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > > > > > that mhp_range_allowed() has already been called.

> > > > > > >

> > > > > > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > > > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > > > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > > > > > Cc: Will Deacon <will@kernel.org>

> > > > > > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > > > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > > > > > Cc: Jason Wang <jasowang@redhat.com>

> > > > > > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > > > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > > > > > Cc: Michal Hocko <mhocko@kernel.org>

> > > > > > > Cc: Oscar Salvador <osalvador@suse.de>

> > > > > > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > > > > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > > > > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > > > > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > > > > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > > > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > > > > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > > > > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > > > > > ---

> > > > > > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > > > > > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > > > > > >

> > > > > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > > > > > index 6f0648777d34..92b3be127796 100644

> > > > > > > --- a/arch/arm64/mm/mmu.c

> > > > > > > +++ b/arch/arm64/mm/mmu.c

> > > > > > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > > > > > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > > > > > >  }

> > > > > > >

> > > > > > > -static bool inside_linear_region(u64 start, u64 size)

> > > > > > > +struct range arch_get_mappable_range(void)

> > > > > > >  {

> > > > > > > +       struct range mhp_range;

> > > > > > > +

> > > > > > >         /*

> > > > > > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > > > > > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > > > > > >          * range which can be mapped inside this linear mapping range, must

> > > > > > >          * also be derived from its end points.

> > > > > > >          */

> > > > > > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > > > > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > > > > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > > > > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > > > > > +       return mhp_range;

> > > > > > >  }

> > > > > > >

> > > > > > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > > > > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > > > > > >  {

> > > > > > >         int ret, flags = 0;

> > > > > > >

> > > > > > > -       if (!inside_linear_region(start, size)) {

> > > > > > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > > > > > -               return -EINVAL;

> > > > > > > -       }

> > > > > > > -

> > > > > > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > > > >         if (rodata_full || debug_pagealloc_enabled())

> > > > > > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> > > > > >

> > > > > > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > > > > > due to below warnings / errors,

> > > > > >

> > > > > > > Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > > >     arm64/mm: define arch_get_mappable_range()

> > > > > >

> > > > > >

> > > > > >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> > > > > >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > > > > > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > > > > > [-Werror=implicit-function-declaration]

> > > > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > > >                ^

> > > > > >   include/linux/build_bug.h:30:63: note: in definition of macro

> > > > > > 'BUILD_BUG_ON_INVALID'

> > > > > >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> > > > > >                                                                  ^

> > > > > >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> > > > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > > >     ^~~~~~~~~

> > > > > >

> > > > > > Build link,

> > > > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > > > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

> > > > >

> > > > > thanks, will go drop this, and the patch that was after it in the

> > > > > series, from both trees and will push out a -rc2.

> > > > >

> > > >

> > > > Why were these picked up in the first place? I don't see any fixes or

> > > > cc:stable tags, and the commit log clearly describes that the change

> > > > is preparatory work for enabling arm64 support into a recently

> > > > introduced generic framework.

> > >

> > > This was needed for a follow-on patch in the series that fixed an issue.

> > > Specifically it was commit ee7febce0519 ("arm64: mm: correct the inside

> > > linear map range during hotplug check")

> > >

> >

> > Yeah, but during the discussion of that patch [0], we pointed out that

> > it needed to be rebased because of these new changes. So trying to

> > backport this rebased version is obviously not the right approach:

> > Pavel's original patch would be much more suitable for that.

> >

> > Could we have annotated this patch in a better way to make this more obvious?

>

> Yes, given that there was no annotation on the patch at all to let us

> know this :)

>

> You can say things like "do not apply to stable trees" or "needs total

> rework for older kernels" or other fun such things that when we read

> them, we know to ask for help.  As it is, the patch provided nothing so

> we guessed and got it wrong...


I will send the patch for stable trees with the commit id included as requested.

Pasha

>

> thanks,

>

> greg k-h
Greg Kroah-Hartman March 29, 2021, 1:59 p.m. UTC | #10
On Mon, Mar 29, 2021 at 09:53:10AM -0400, Pavel Tatashin wrote:
> On Mon, Mar 29, 2021 at 9:51 AM Greg Kroah-Hartman

> <gregkh@linuxfoundation.org> wrote:

> >

> > On Mon, Mar 29, 2021 at 03:49:19PM +0200, Ard Biesheuvel wrote:

> > > (+ Pavel)

> > >

> > > On Mon, 29 Mar 2021 at 15:42, Greg Kroah-Hartman

> > > <gregkh@linuxfoundation.org> wrote:

> > > >

> > > > On Mon, Mar 29, 2021 at 03:08:52PM +0200, Ard Biesheuvel wrote:

> > > > > On Mon, 29 Mar 2021 at 12:12, Greg Kroah-Hartman

> > > > > <gregkh@linuxfoundation.org> wrote:

> > > > > >

> > > > > > On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > > > > > > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > > > > > > <gregkh@linuxfoundation.org> wrote:

> > > > > > > >

> > > > > > > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > > > >

> > > > > > > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > > > > > > >

> > > > > > > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > > > > > > used with recently added generic framework.  It drops

> > > > > > > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > > > > > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > > > > > > that mhp_range_allowed() has already been called.

> > > > > > > >

> > > > > > > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > > > > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > > > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > > > > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > > > > > > Cc: Will Deacon <will@kernel.org>

> > > > > > > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > > > > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > > > > > > Cc: Jason Wang <jasowang@redhat.com>

> > > > > > > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > > > > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > > > > > > Cc: Michal Hocko <mhocko@kernel.org>

> > > > > > > > Cc: Oscar Salvador <osalvador@suse.de>

> > > > > > > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > > > > > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > > > > > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > > > > > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > > > > > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > > > > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > > > > > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > > > > > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > > > > > > ---

> > > > > > > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > > > > > > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > > > > > > >

> > > > > > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > > > > > > index 6f0648777d34..92b3be127796 100644

> > > > > > > > --- a/arch/arm64/mm/mmu.c

> > > > > > > > +++ b/arch/arm64/mm/mmu.c

> > > > > > > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > > > > > > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > > > > > > >  }

> > > > > > > >

> > > > > > > > -static bool inside_linear_region(u64 start, u64 size)

> > > > > > > > +struct range arch_get_mappable_range(void)

> > > > > > > >  {

> > > > > > > > +       struct range mhp_range;

> > > > > > > > +

> > > > > > > >         /*

> > > > > > > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > > > > > > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > > > > > > >          * range which can be mapped inside this linear mapping range, must

> > > > > > > >          * also be derived from its end points.

> > > > > > > >          */

> > > > > > > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > > > > > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > > > > > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > > > > > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > > > > > > +       return mhp_range;

> > > > > > > >  }

> > > > > > > >

> > > > > > > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > > > > > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > > > > > > >  {

> > > > > > > >         int ret, flags = 0;

> > > > > > > >

> > > > > > > > -       if (!inside_linear_region(start, size)) {

> > > > > > > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > > > > > > -               return -EINVAL;

> > > > > > > > -       }

> > > > > > > > -

> > > > > > > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > > > > >         if (rodata_full || debug_pagealloc_enabled())

> > > > > > > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> > > > > > >

> > > > > > > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > > > > > > due to below warnings / errors,

> > > > > > >

> > > > > > > > Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > > > >     arm64/mm: define arch_get_mappable_range()

> > > > > > >

> > > > > > >

> > > > > > >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> > > > > > >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > > > > > > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > > > > > > [-Werror=implicit-function-declaration]

> > > > > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > > > >                ^

> > > > > > >   include/linux/build_bug.h:30:63: note: in definition of macro

> > > > > > > 'BUILD_BUG_ON_INVALID'

> > > > > > >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> > > > > > >                                                                  ^

> > > > > > >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> > > > > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > > > >     ^~~~~~~~~

> > > > > > >

> > > > > > > Build link,

> > > > > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > > > > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

> > > > > >

> > > > > > thanks, will go drop this, and the patch that was after it in the

> > > > > > series, from both trees and will push out a -rc2.

> > > > > >

> > > > >

> > > > > Why were these picked up in the first place? I don't see any fixes or

> > > > > cc:stable tags, and the commit log clearly describes that the change

> > > > > is preparatory work for enabling arm64 support into a recently

> > > > > introduced generic framework.

> > > >

> > > > This was needed for a follow-on patch in the series that fixed an issue.

> > > > Specifically it was commit ee7febce0519 ("arm64: mm: correct the inside

> > > > linear map range during hotplug check")

> > > >

> > >

> > > Yeah, but during the discussion of that patch [0], we pointed out that

> > > it needed to be rebased because of these new changes. So trying to

> > > backport this rebased version is obviously not the right approach:

> > > Pavel's original patch would be much more suitable for that.

> > >

> > > Could we have annotated this patch in a better way to make this more obvious?

> >

> > Yes, given that there was no annotation on the patch at all to let us

> > know this :)

> >

> > You can say things like "do not apply to stable trees" or "needs total

> > rework for older kernels" or other fun such things that when we read

> > them, we know to ask for help.  As it is, the patch provided nothing so

> > we guessed and got it wrong...

> 

> I will send the patch for stable trees with the commit id included as requested.


Wonderful, thank you so much.

greg k-h
Anders Roxell March 29, 2021, 2:13 p.m. UTC | #11
On Mon, 29 Mar 2021 at 13:35, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>

> On Mon, Mar 29, 2021 at 01:06:47PM +0200, Anders Roxell wrote:

> > On Mon, 29 Mar 2021 at 12:13, Greg Kroah-Hartman

> > <gregkh@linuxfoundation.org> wrote:

> > >

> > > On Mon, Mar 29, 2021 at 03:05:25PM +0530, Naresh Kamboju wrote:

> > > > On Mon, 29 Mar 2021 at 14:10, Greg Kroah-Hartman

> > > > <gregkh@linuxfoundation.org> wrote:

> > > > >

> > > > > From: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > >

> > > > > [ Upstream commit 03aaf83fba6e5af08b5dd174c72edee9b7d9ed9b ]

> > > > >

> > > > > This overrides arch_get_mappable_range() on arm64 platform which will be

> > > > > used with recently added generic framework.  It drops

> > > > > inside_linear_region() and subsequent check in arch_add_memory() which are

> > > > > no longer required.  It also adds a VM_BUG_ON() check that would ensure

> > > > > that mhp_range_allowed() has already been called.

> > > > >

> > > > > Link: https://lkml.kernel.org/r/1612149902-7867-3-git-send-email-anshuman.khandual@arm.com

> > > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

> > > > > Reviewed-by: David Hildenbrand <david@redhat.com>

> > > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> > > > > Cc: Will Deacon <will@kernel.org>

> > > > > Cc: Ard Biesheuvel <ardb@kernel.org>

> > > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > > Cc: Heiko Carstens <hca@linux.ibm.com>

> > > > > Cc: Jason Wang <jasowang@redhat.com>

> > > > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > > > > Cc: Michal Hocko <mhocko@kernel.org>

> > > > > Cc: Oscar Salvador <osalvador@suse.de>

> > > > > Cc: Pankaj Gupta <pankaj.gupta@cloud.ionos.com>

> > > > > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> > > > > Cc: teawater <teawaterz@linux.alibaba.com>

> > > > > Cc: Vasily Gorbik <gor@linux.ibm.com>

> > > > > Cc: Wei Yang <richard.weiyang@linux.alibaba.com>

> > > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> > > > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> > > > > Signed-off-by: Sasha Levin <sashal@kernel.org>

> > > > > ---

> > > > >  arch/arm64/mm/mmu.c | 15 +++++++--------

> > > > >  1 file changed, 7 insertions(+), 8 deletions(-)

> > > > >

> > > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> > > > > index 6f0648777d34..92b3be127796 100644

> > > > > --- a/arch/arm64/mm/mmu.c

> > > > > +++ b/arch/arm64/mm/mmu.c

> > > > > @@ -1443,16 +1443,19 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)

> > > > >         free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);

> > > > >  }

> > > > >

> > > > > -static bool inside_linear_region(u64 start, u64 size)

> > > > > +struct range arch_get_mappable_range(void)

> > > > >  {

> > > > > +       struct range mhp_range;

> > > > > +

> > > > >         /*

> > > > >          * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]

> > > > >          * accommodating both its ends but excluding PAGE_END. Max physical

> > > > >          * range which can be mapped inside this linear mapping range, must

> > > > >          * also be derived from its end points.

> > > > >          */

> > > > > -       return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&

> > > > > -              (start + size - 1) <= __pa(PAGE_END - 1);

> > > > > +       mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));

> > > > > +       mhp_range.end =  __pa(PAGE_END - 1);

> > > > > +       return mhp_range;

> > > > >  }

> > > > >

> > > > >  int arch_add_memory(int nid, u64 start, u64 size,

> > > > > @@ -1460,11 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,

> > > > >  {

> > > > >         int ret, flags = 0;

> > > > >

> > > > > -       if (!inside_linear_region(start, size)) {

> > > > > -               pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

> > > > > -               return -EINVAL;

> > > > > -       }

> > > > > -

> > > > > +       VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > > >         if (rodata_full || debug_pagealloc_enabled())

> > > > >                 flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;

> > > >

> > > > The stable rc 5.10 and 5.11 builds failed for arm64 architecture

> > > > due to below warnings / errors,

> > > >

> > > > > Anshuman Khandual <anshuman.khandual@arm.com>

> > > > >     arm64/mm: define arch_get_mappable_range()

> > > >

> > > >

> > > >   arch/arm64/mm/mmu.c: In function 'arch_add_memory':

> > > >   arch/arm64/mm/mmu.c:1483:13: error: implicit declaration of function

> > > > 'mhp_range_allowed'; did you mean 'cpu_map_prog_allowed'?

> > > > [-Werror=implicit-function-declaration]

> > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > >                ^

> > > >   include/linux/build_bug.h:30:63: note: in definition of macro

> > > > 'BUILD_BUG_ON_INVALID'

> > > >    #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

> > > >                                                                  ^

> > > >   arch/arm64/mm/mmu.c:1483:2: note: in expansion of macro 'VM_BUG_ON'

> > > >     VM_BUG_ON(!mhp_range_allowed(start, size, true));

> > > >     ^~~~~~~~~

> > > >

> > > > Build link,

> > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.11/DISTRO=lkft,MACHINE=juno,label=docker-buster-lkft/41/consoleText

> > > > https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.10/DISTRO=lkft,MACHINE=dragonboard-410c,label=docker-buster-lkft/120/consoleFull

> > >

> > > thanks, will go drop this, and the patch that was after it in the

> > > series, from both trees and will push out a -rc2.

> > >

> > > Note, I used tuxbuild before doing this release, and it does not show

> > > this error in the arm64 defconfigs. What config did you use to trigger

> > > this?

> >

> > We have a build with CONFIG_MEMORY_HOTPLUG=y enabled too.

> >

> > This is a way to reproduce it locally:

> > tuxmake --runtime podman --target-arch arm64 --toolchain gcc --kconfig

> > defconfig --kconfig-add CONFIG_MEMORY_HOTPLUG=y

>

> Ah, that wasn't expected, but makes sense, thanks.

>

> Does 'allmodconfig' also trigger that?


Yes that would trigger it. I tried this:
tuxmake --runtime docker --target-arch arm64 --toolchain gcc --kconfig
allmodconfig --build-dir=$(pwd)/obj/test-allmod-arm64

>  Maybe I'll go add that to my

> build tests for arm64...


It will take a few minutes to build an allmodconfig kernel on tuxsuite.

Cheers,
Anders
diff mbox series

Patch

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6f0648777d34..92b3be127796 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1443,16 +1443,19 @@  static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size)
 	free_empty_tables(start, end, PAGE_OFFSET, PAGE_END);
 }
 
-static bool inside_linear_region(u64 start, u64 size)
+struct range arch_get_mappable_range(void)
 {
+	struct range mhp_range;
+
 	/*
 	 * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)]
 	 * accommodating both its ends but excluding PAGE_END. Max physical
 	 * range which can be mapped inside this linear mapping range, must
 	 * also be derived from its end points.
 	 */
-	return start >= __pa(_PAGE_OFFSET(vabits_actual)) &&
-	       (start + size - 1) <= __pa(PAGE_END - 1);
+	mhp_range.start = __pa(_PAGE_OFFSET(vabits_actual));
+	mhp_range.end =  __pa(PAGE_END - 1);
+	return mhp_range;
 }
 
 int arch_add_memory(int nid, u64 start, u64 size,
@@ -1460,11 +1463,7 @@  int arch_add_memory(int nid, u64 start, u64 size,
 {
 	int ret, flags = 0;
 
-	if (!inside_linear_region(start, size)) {
-		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);
-		return -EINVAL;
-	}
-
+	VM_BUG_ON(!mhp_range_allowed(start, size, true));
 	if (rodata_full || debug_pagealloc_enabled())
 		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;