Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 30 Jun 2020 14:15:19 +0300
From:   Mike Rapoport <rppt@linux.ibm.com>
To:     David Rientjes <rientjes@google.com>
Cc:     Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        kbuild-all@lists.01.org, linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Michal Simek <monstr@monstr.eu>
Subject: Re: ERROR: "min_low_pfn" undefined!
Message-ID: <20200630111519.GA1951986@linux.ibm.com>
References: <202006300524.y9r1ZSHI%lkp@intel.com>
 <alpine.DEB.2.22.394.2006291911220.1118534@chino.kir.corp.google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.22.394.2006291911220.1118534@chino.kir.corp.google.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

(addde Michal)

On Mon, Jun 29, 2020 at 07:13:30PM -0700, David Rientjes wrote:
> On Tue, 30 Jun 2020, kernel test robot wrote:
> 
> > Hi Alexander,
> > 
> > FYI, the error/warning still remains.
> > 
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   7c30b859a947535f2213277e827d7ac7dcff9c84
> > commit: f220df66f67684246ae1bf4a4e479efc7c2f325a intel_th: msu-sink: An example msu buffer "sink"
> > date:   11 months ago
> > config: microblaze-randconfig-c023-20200629 (attached as .config)
> > compiler: microblaze-linux-gcc (GCC) 9.3.0
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    ERROR: "min_low_pfn" [drivers/rpmsg/virtio_rpmsg_bus.ko] undefined!
> > >> ERROR: "min_low_pfn" [drivers/hwtracing/intel_th/intel_th_msu_sink.ko] undefined!
> >    ERROR: "min_low_pfn" [drivers/hwtracing/intel_th/intel_th_msu.ko] undefined!
> >    ERROR: "min_low_pfn" [drivers/mmc/core/mmc_core.ko] undefined!
> >    ERROR: "min_low_pfn" [drivers/md/dm-crypt.ko] undefined!
> >    ERROR: "min_low_pfn" [drivers/net/wireless/ath/ath6kl/ath6kl_sdio.ko] undefined!
> >    ERROR: "min_low_pfn" [crypto/tcrypt.ko] undefined!
> >    ERROR: "min_low_pfn" [crypto/asymmetric_keys/asym_tpm.ko] undefined!
> > 
> 
> Looks like we have precedence for min_low_pfn failures and we can't blame 
> this poor commit :)

This commit only added a new occurence of this error :)
It seems that the problem is caused by nommu defintion of pfn_valid() in
microblaze:

define pfn_valid(pfn)       (((pfn) >= min_low_pfn) && \
			       ((pfn) <= (min_low_pfn + max_mapnr)))

but I could not reproduce the error with nommu_defconfig. 

> I wonder why we don't simply do EXPORT_SYMBOL() in mm/memblock.c, 
> non-microblaze architectures do this themselves because it doesn't get 
> generically exported:

> arch/ia64/kernel/ia64_ksyms.c:EXPORT_SYMBOL(min_low_pfn);       /* defined by bootmem.c, but not exported by generic code */
> arch/sh/kernel/sh_ksyms_32.c:EXPORT_SYMBOL(min_low_pfn);
 
I think we can do better than that. The below patch is build-tested
only.

From 86d21c2c2e6a859400a3319dc21ba9b35d126539 Mon Sep 17 00:00:00 2001
From: Mike Rapoport <rppt@linux.ibm.com>
Date: Tue, 30 Jun 2020 13:37:53 +0300
Subject: [PATCH] microblaze: cleanup memory start aliases

There are several variables that have the same meaning and value, but they
are used with different configuration options:

- memory_start - "main memory where the kernel is"
- __page_offset - an alias for memory_start disguised as PAGE_OFFSET fot
  no-MMU case
- min_low_pfn - a legacy from i386 memory layout weirdness, an alias for
  (memory_start >> PAGE_SHIFT)

Since there are "different" variables that point to the start of the
memory, MMU and no-MMU variants of pfn_valid() and ARCH_PFN_OFFSET used
memory_start and __page_offset with min_low_pfn respectively.

Use memory_start for both MMU and no-MMU versions of pfn_valid() and
ARCH_PFN_OFFSET and get rid of __page_offset entirely.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 arch/microblaze/include/asm/page.h | 18 ++++--------------
 arch/microblaze/mm/init.c          |  1 -
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/arch/microblaze/include/asm/page.h b/arch/microblaze/include/asm/page.h
index b13463d39b38..d664c437b377 100644
--- a/arch/microblaze/include/asm/page.h
+++ b/arch/microblaze/include/asm/page.h
@@ -50,8 +50,7 @@
  * using MMU this corresponds to the first free page in physical memory (aligned
  * on a page boundary).
  */
-extern unsigned int __page_offset;
-#define PAGE_OFFSET __page_offset
+#define PAGE_OFFSET memory_start
 
 #else /* CONFIG_MMU */
 
@@ -124,10 +123,6 @@ typedef struct { p4d_t		pge[1]; }	pgd_t;
  *
  */
 
-extern unsigned long max_low_pfn;
-extern unsigned long min_low_pfn;
-extern unsigned long max_pfn;
-
 extern unsigned long memory_start;
 extern unsigned long memory_size;
 extern unsigned long lowmem_size;
@@ -156,14 +151,9 @@ extern int page_is_ram(unsigned long pfn);
 #  define phys_to_page(paddr)	(pfn_to_page(phys_to_pfn(paddr)))
 #  endif /* CONFIG_MMU */
 
-#  ifndef CONFIG_MMU
-#  define pfn_valid(pfn)	(((pfn) >= min_low_pfn) && \
-				((pfn) <= (min_low_pfn + max_mapnr)))
-#  define ARCH_PFN_OFFSET	(PAGE_OFFSET >> PAGE_SHIFT)
-#  else /* CONFIG_MMU */
-#  define ARCH_PFN_OFFSET	(memory_start >> PAGE_SHIFT)
-#  define pfn_valid(pfn)	((pfn) < (max_mapnr + ARCH_PFN_OFFSET))
-#  endif /* CONFIG_MMU */
+#define ARCH_PFN_OFFSET	(memory_start >> PAGE_SHIFT)
+#define pfn_valid(pfn)	(((pfn) >= ARCH_PFN_OFFSET) && \
+			 ((pfn) < (ARCH_PFN_OFFSET + max_mapnr)))
 
 # endif /* __ASSEMBLY__ */
 
diff --git a/arch/microblaze/mm/init.c b/arch/microblaze/mm/init.c
index 521b59ba716c..0ddc5fcc55c2 100644
--- a/arch/microblaze/mm/init.c
+++ b/arch/microblaze/mm/init.c
@@ -117,7 +117,6 @@ void __init setup_memory(void)
 		if ((memory_start <= (u32)_text) &&
 			((u32)_text <= (memory_start + lowmem_size - 1))) {
 			memory_size = lowmem_size;
-			PAGE_OFFSET = memory_start;
 			pr_info("%s: Main mem: 0x%x, size 0x%08x\n",
 				__func__, (u32) memory_start,
 					(u32) memory_size);
-- 
2.26.2

-- 
Sincerely yours,
Mike.