[PATCH -next] powerpc/book3s64: fix link error with CONFIG_PPC_RADIX_MMU=n

Fix link error when CONFIG_PPC_RADIX_MMU is disabled:
powerpc64-linux-gnu-ld: arch/powerpc/platforms/pseries/lpar.o:(.toc+0x0): undefined reference to `mmu_pid_bits'

Reported-by: Hulk Robot <redacted>
Signed-off-by: Yang Yingliang <redacted>
---
 arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++
 arch/powerpc/platforms/pseries/lpar.c  | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c
index 0ba30b8b935b..a8e292cd88f0 100644
--- a/arch/powerpc/mm/book3s64/mmu_context.c
+++ b/arch/powerpc/mm/book3s64/mmu_context.c

@@ -152,6 +152,7 @@ void hash__setup_new_exec(void)
 
 static int radix__init_new_context(struct mm_struct *mm)
 {
+#ifdef CONFIG_PPC_RADIX_MMU
 	unsigned long rts_field;
 	int index, max_id;
 

@@ -177,6 +178,9 @@ static int radix__init_new_context(struct mm_struct *mm)
 	mm->context.hash_context = NULL;
 
 	return index;
+#else
+	return -ENOTSUPP;
+#endif
 }
 
 int init_new_context(struct task_struct *tsk, struct mm_struct *mm)

diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index baf24eacd268..e454e218dbba 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c

@@ -1726,10 +1726,12 @@ void __init hpte_init_pseries(void)
 
 void radix_init_pseries(void)
 {
+#ifdef CONFIG_PPC_RADIX_MMU
 	pr_info("Using radix MMU under hypervisor\n");
 
 	pseries_lpar_register_process_table(__pa(process_tb),
 						0, PRTB_SIZE_SHIFT - 12);
+#endif
 }
 
 #ifdef CONFIG_PPC_SMLPAR

-- 
2.25.1

Le 05/09/2020 à 13:25, Yang Yingliang a écrit :

Fix link error when CONFIG_PPC_RADIX_MMU is disabled:
powerpc64-linux-gnu-ld: arch/powerpc/platforms/pseries/lpar.o:(.toc+0x0): undefined reference to `mmu_pid_bits'

Reported-by: Hulk Robot <redacted>
Signed-off-by: Yang Yingliang <redacted>
---
  arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++

In your commit log, you are just mentionning 
arch/powerpc/platforms/pseries/lpar.o, which is right.

You shouldn't need to modify arch/powerpc/mm/book3s64/mmu_context.c at 
all, see below.

This shouldn't be required. radix__init_new_context() is only called 
when radix_enabled() returns true.
As it is a static function, when it is not called it gets optimised 
away, so you will never get an undefined reference to `mmu_pid_bits` there.

This function is only called from 
/arch/powerpc/mm/book3s64/radix_pgtable.c which is only built when 
CONFIG_PPC_RADIX_MMU is selected.

So the entire function should be encloded in the #ifdef.

  	pr_info("Using radix MMU under hypervisor\n");
  
  	pseries_lpar_register_process_table(__pa(process_tb),
  						0, PRTB_SIZE_SHIFT - 12);
+#endif
  }
  
  #ifdef CONFIG_PPC_SMLPAR

Christophe

On 2020/9/6 14:50, Christophe Leroy wrote:

Le 05/09/2020 à 13:25, Yang Yingliang a écrit :

quoted

Fix link error when CONFIG_PPC_RADIX_MMU is disabled:
powerpc64-linux-gnu-ld: 
arch/powerpc/platforms/pseries/lpar.o:(.toc+0x0): undefined reference 
to `mmu_pid_bits'

Reported-by: Hulk Robot <redacted>
Signed-off-by: Yang Yingliang <redacted>
---
  arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++

In your commit log, you are just mentionning 
arch/powerpc/platforms/pseries/lpar.o, which is right.

You shouldn't need to modify arch/powerpc/mm/book3s64/mmu_context.c at 
all, see below.

quoted

  arch/powerpc/platforms/pseries/lpar.c  | 2 ++
  2 files changed, 6 insertions(+)

diff --git a/arch/powerpc/mm/book3s64/mmu_context.c 

b/arch/powerpc/mm/book3s64/mmu_context.c
index 0ba30b8b935b..a8e292cd88f0 100644

--- a/arch/powerpc/mm/book3s64/mmu_context.c
+++ b/arch/powerpc/mm/book3s64/mmu_context.c

@@ -152,6 +152,7 @@ void hash__setup_new_exec(void)

    static int radix__init_new_context(struct mm_struct *mm)
  {
+#ifdef CONFIG_PPC_RADIX_MMU

This shouldn't be required. radix__init_new_context() is only called 
when radix_enabled() returns true.
As it is a static function, when it is not called it gets optimised 
away, so you will never get an undefined reference to `mmu_pid_bits` 
there.

powerpc64-linux-gnu-ld: 
arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x0): undefined reference 
to `mmu_pid_bits'
powerpc64-linux-gnu-ld: 
arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x8): undefined reference 
to `mmu_base_pid'


mmu_context.c is always compiled, it uses mmu_pid_bits and mmu_base_pid.

quoted

      unsigned long rts_field;
      int index, max_id;
  @@ -177,6 +178,9 @@ static int radix__init_new_context(struct 
mm_struct *mm)
      mm->context.hash_context = NULL;
        return index;
+#else
+    return -ENOTSUPP;
+#endif
  }
    int init_new_context(struct task_struct *tsk, struct mm_struct *mm)

diff --git a/arch/powerpc/platforms/pseries/lpar.c 

b/arch/powerpc/platforms/pseries/lpar.c
index baf24eacd268..e454e218dbba 100644

--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c

@@ -1726,10 +1726,12 @@ void __init hpte_init_pseries(void)

    void radix_init_pseries(void)
  {
+#ifdef CONFIG_PPC_RADIX_MMU

This function is only called from 
/arch/powerpc/mm/book3s64/radix_pgtable.c which is only built when 
CONFIG_PPC_RADIX_MMU is selected.

So the entire function should be encloded in the #ifdef.

OK, I will send a v2 later.

quoted

      pr_info("Using radix MMU under hypervisor\n");
        pseries_lpar_register_process_table(__pa(process_tb),
                          0, PRTB_SIZE_SHIFT - 12);
+#endif
  }
    #ifdef CONFIG_PPC_SMLPAR

Christophe
.

Le 07/09/2020 à 03:51, Yang Yingliang a écrit :

On 2020/9/6 14:50, Christophe Leroy wrote:

quoted

Le 05/09/2020 à 13:25, Yang Yingliang a écrit :

quoted

Fix link error when CONFIG_PPC_RADIX_MMU is disabled:
powerpc64-linux-gnu-ld: 
arch/powerpc/platforms/pseries/lpar.o:(.toc+0x0): undefined reference 
to `mmu_pid_bits'

Reported-by: Hulk Robot <redacted>
Signed-off-by: Yang Yingliang <redacted>
---
  arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++

In your commit log, you are just mentionning 
arch/powerpc/platforms/pseries/lpar.o, which is right.

You shouldn't need to modify arch/powerpc/mm/book3s64/mmu_context.c at 
all, see below.

quoted

  arch/powerpc/platforms/pseries/lpar.c  | 2 ++
  2 files changed, 6 insertions(+)

diff --git a/arch/powerpc/mm/book3s64/mmu_context.c 

b/arch/powerpc/mm/book3s64/mmu_context.c
index 0ba30b8b935b..a8e292cd88f0 100644

--- a/arch/powerpc/mm/book3s64/mmu_context.c
+++ b/arch/powerpc/mm/book3s64/mmu_context.c

@@ -152,6 +152,7 @@ void hash__setup_new_exec(void)

    static int radix__init_new_context(struct mm_struct *mm)
  {
+#ifdef CONFIG_PPC_RADIX_MMU

This shouldn't be required. radix__init_new_context() is only called 
when radix_enabled() returns true.
As it is a static function, when it is not called it gets optimised 
away, so you will never get an undefined reference to `mmu_pid_bits` 
there.

powerpc64-linux-gnu-ld: 
arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x0): undefined reference 
to `mmu_pid_bits'
powerpc64-linux-gnu-ld: 
arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x8): undefined reference 
to `mmu_base_pid'


mmu_context.c is always compiled, it uses mmu_pid_bits and mmu_base_pid.

Yes, mmu_context.c is always compiled, but as I explained, 
radix__init_new_context() is defined as 'static' so it is optimised out 
when radix_enabled() returns false because there is no caller in that case.

I just made the test with ppc64_defconfig + CONFIG_PPC_RADIX_MMU=n (GCC 8.1)

The only failure I got was on lpar.c, which I fixed by enclosing the 
entire radix_init_pseries() in an #ifdef.

Once this is fixed, the build is OK, without any modification to 
mmu_context.c

powerpc64-linux-objdump -x arch/powerpc/mm/book3s64/mmu_context.o shows 
only the following objects in the .toc:

RELOCATION RECORDS FOR [.toc]:
OFFSET           TYPE              VALUE
0000000000000000 R_PPC64_ADDR64    kmalloc_caches
0000000000000008 R_PPC64_ADDR64    vmemmap
0000000000000010 R_PPC64_ADDR64    __pmd_frag_nr
0000000000000018 R_PPC64_ADDR64    __pmd_frag_size_shift

mmu_pid_bits and mmu_base_pid are not part of the undefined objetcs:

0000000000000000         *UND*	0000000000000000 vmemmap
0000000000000000         *UND*	0000000000000000 .mm_iommu_init
0000000000000000         *UND*	0000000000000000 __pmd_frag_nr
0000000000000000         *UND*	0000000000000000 .ida_alloc_range
0000000000000000         *UND*	0000000000000000 .slb_setup_new_exec
0000000000000000         *UND*	0000000000000000 mmu_feature_keys
0000000000000000         *UND*	0000000000000000 .memset
0000000000000000         *UND*	0000000000000000 .memcpy
0000000000000000         *UND*	0000000000000000 .slice_init_new_context_exec
0000000000000000         *UND*	0000000000000000 ._mcount
0000000000000000         *UND*	0000000000000000 .__free_pages
0000000000000000         *UND*	0000000000000000 __pmd_frag_size_shift
0000000000000000         *UND*	0000000000000000 .slice_setup_new_exec
0000000000000000         *UND*	0000000000000000 .ida_free
0000000000000000         *UND*	0000000000000000 .pte_frag_destroy
0000000000000000         *UND*	0000000000000000 .kfree
0000000000000000         *UND*	0000000000000000 .pkey_mm_init
0000000000000000         *UND*	0000000000000000 .kmem_cache_alloc_trace
0000000000000000         *UND*	0000000000000000 .__warn_printk
0000000000000000         *UND*	0000000000000000 _mcount
0000000000000000         *UND*	0000000000000000 kmalloc_caches

Christophe