Message ID | 20200905112548.3265530-1-yangyingliang@huawei.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | [-next] powerpc/book3s64: fix link error with CONFIG_PPC_RADIX_MMU=n | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (35f066fda170dde0a31f1447547a5d30b83c3920) |
snowpatch_ozlabs/build-ppc64le | warning | Upstream build failed, couldn't test patch |
snowpatch_ozlabs/build-ppc64be | warning | Upstream build failed, couldn't test patch |
snowpatch_ozlabs/build-ppc64e | warning | Upstream build failed, couldn't test patch |
snowpatch_ozlabs/build-pmac32 | warning | Upstream build failed, couldn't test patch |
snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 2 warnings, 0 checks, 28 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
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 <hulkci@huawei.com> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > 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. > 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. > 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. > 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 : >> 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 <hulkci@huawei.com> >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> 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. > >> 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. > >> 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. > >> 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: >> >> >> 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 <hulkci@huawei.com> >>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >>> --- >>> 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. >> >>> 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
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
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 <hulkci@huawei.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++ arch/powerpc/platforms/pseries/lpar.c | 2 ++ 2 files changed, 6 insertions(+)