| Message ID | 20210227061944.266415-2-huangpei@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MIPS: clean up CONFIG_MIPS_PGD_C0_CONTEXT handling | expand |
On Sat, 27 Feb 2021, Huang Pei wrote: > index 2000bb2b0220..517509ad8596 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -2142,6 +2142,7 @@ config CPU_SUPPORTS_HUGEPAGES > depends on !(32BIT && (ARCH_PHYS_ADDR_T_64BIT || EVA)) > config MIPS_PGD_C0_CONTEXT > bool > + depends on 64BIT > default y if 64BIT && (CPU_MIPSR2 || CPU_MIPSR6) && !CPU_XLP I guess you want: default y if (CPU_MIPSR2 || CPU_MIPSR6) && !CPU_XLP at the same time too. Otherwise you have cruft left behind. > diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c > index a7521b8f7658..5bb9724578f7 100644 > --- a/arch/mips/mm/tlbex.c > +++ b/arch/mips/mm/tlbex.c > @@ -1106,6 +1106,7 @@ struct mips_huge_tlb_info { > bool need_reload_pte; > }; > > +#ifdef CONFIG_64BIT > static struct mips_huge_tlb_info > build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, > struct uasm_reloc **r, unsigned int tmp, Does it actually build without a warning for !CONFIG_64BIT given the reference below? > @@ -1164,8 +1165,8 @@ build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, > > if (pgd_reg == -1) { > vmalloc_branch_delay_filled = 1; > - /* 1 0 1 0 1 << 6 xkphys cached */ > - uasm_i_ori(p, ptr, ptr, 0x540); > + /* insert bit[63:59] of CAC_BASE into bit[11:6] of ptr */ > + uasm_i_ori(p, ptr, ptr, (CAC_BASE >> 53)); Instead I'd paper the issue over by casting the constant to `s64'. Or better yet fixed it properly by defining CAC_BASE, etc. as `unsigned long long' long rather than `unsigned long' to stop all this nonsense (e.g. PHYS_TO_XKPHYS already casts the result to `s64'). Thomas, WDYT? Maciej
Hi, On Mon, Mar 01, 2021 at 12:00:28AM +0100, Maciej W. Rozycki wrote: > On Sat, 27 Feb 2021, Huang Pei wrote: > > > index 2000bb2b0220..517509ad8596 100644 > > --- a/arch/mips/Kconfig > > +++ b/arch/mips/Kconfig > > @@ -2142,6 +2142,7 @@ config CPU_SUPPORTS_HUGEPAGES > > depends on !(32BIT && (ARCH_PHYS_ADDR_T_64BIT || EVA)) > > config MIPS_PGD_C0_CONTEXT > > bool > > + depends on 64BIT > > default y if 64BIT && (CPU_MIPSR2 || CPU_MIPSR6) && !CPU_XLP > > I guess you want: > > default y if (CPU_MIPSR2 || CPU_MIPSR6) && !CPU_XLP > > at the same time too. Otherwise you have cruft left behind. > Yes, it is much better > > diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c > > index a7521b8f7658..5bb9724578f7 100644 > > --- a/arch/mips/mm/tlbex.c > > +++ b/arch/mips/mm/tlbex.c > > @@ -1106,6 +1106,7 @@ struct mips_huge_tlb_info { > > bool need_reload_pte; > > }; > > > > +#ifdef CONFIG_64BIT > > static struct mips_huge_tlb_info > > build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, > > struct uasm_reloc **r, unsigned int tmp, > > Does it actually build without a warning for !CONFIG_64BIT given the > reference below? No, my bad, my first reaction when seeing "IS_ENABLED(CONFIG_64BIT)" is "#ifdef CONFIG_64BIT" > > > @@ -1164,8 +1165,8 @@ build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, > > > > if (pgd_reg == -1) { > > vmalloc_branch_delay_filled = 1; > > - /* 1 0 1 0 1 << 6 xkphys cached */ > > - uasm_i_ori(p, ptr, ptr, 0x540); > > + /* insert bit[63:59] of CAC_BASE into bit[11:6] of ptr */ > > + uasm_i_ori(p, ptr, ptr, (CAC_BASE >> 53)); > > Instead I'd paper the issue over by casting the constant to `s64'. > > Or better yet fixed it properly by defining CAC_BASE, etc. as `unsigned > long long' long rather than `unsigned long' to stop all this nonsense > (e.g. PHYS_TO_XKPHYS already casts the result to `s64'). Thomas, WDYT? Sorry, I do not get it , on MIPS32, how can CAC_BASE be unsigned long long ? If this did not work, how about one step back, just explicit comment wihtout "(CAC_BASE << 53)" ? > > Maciej
On Thu, 4 Mar 2021, Huang Pei wrote: > > > @@ -1164,8 +1165,8 @@ build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, > > > > > > if (pgd_reg == -1) { > > > vmalloc_branch_delay_filled = 1; > > > - /* 1 0 1 0 1 << 6 xkphys cached */ > > > - uasm_i_ori(p, ptr, ptr, 0x540); > > > + /* insert bit[63:59] of CAC_BASE into bit[11:6] of ptr */ > > > + uasm_i_ori(p, ptr, ptr, (CAC_BASE >> 53)); > > > > Instead I'd paper the issue over by casting the constant to `s64'. > > > > Or better yet fixed it properly by defining CAC_BASE, etc. as `unsigned > > long long' long rather than `unsigned long' to stop all this nonsense > > (e.g. PHYS_TO_XKPHYS already casts the result to `s64'). Thomas, WDYT? > Sorry, I do not get it , on MIPS32, how can CAC_BASE be unsigned long > long ? By using the `ULL' suffix with constants. It won't change code produced, because they are unsigned anyway and the compiler will truncate them with no change to the actual value to fit in narrower data types as needed, but it will silence the warnings. Maciej
Hi, On Thu, Mar 04, 2021 at 02:40:54AM +0100, Maciej W. Rozycki wrote: > On Thu, 4 Mar 2021, Huang Pei wrote: > > > > > @@ -1164,8 +1165,8 @@ build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, > > > > > > > > if (pgd_reg == -1) { > > > > vmalloc_branch_delay_filled = 1; > > > > - /* 1 0 1 0 1 << 6 xkphys cached */ > > > > - uasm_i_ori(p, ptr, ptr, 0x540); > > > > + /* insert bit[63:59] of CAC_BASE into bit[11:6] of ptr */ > > > > + uasm_i_ori(p, ptr, ptr, (CAC_BASE >> 53)); > > > > > > Instead I'd paper the issue over by casting the constant to `s64'. > > > > > > Or better yet fixed it properly by defining CAC_BASE, etc. as `unsigned > > > long long' long rather than `unsigned long' to stop all this nonsense > > > (e.g. PHYS_TO_XKPHYS already casts the result to `s64'). Thomas, WDYT? > > Sorry, I do not get it , on MIPS32, how can CAC_BASE be unsigned long > > long ? > > By using the `ULL' suffix with constants. It won't change code produced, > because they are unsigned anyway and the compiler will truncate them with > no change to the actual value to fit in narrower data types as needed, but > it will silence the warnings. > > Maciej On Linux 5.11 with this patch and **ONLY** attaching ULL to CAC_BASE in arch/mips/include/asm/mach-generic/space.h for CONFIG_32BIT, cross gcc 7.5 in Ubuntu 18.04, loongon1c_defconfig .......... make[1]: Entering directory '/home/hp/projects/Linux/temp/out_stable' GEN Makefile CALL /home/hp/projects/Linux/temp/linux-stable/scripts/atomic/check-atomics.sh CALL /home/hp/projects/Linux/temp/linux-stable/scripts/checksyscalls.sh CC arch/mips/mm/cache.o CC arch/mips/kernel/branch.o CC arch/mips/mm/context.o CC arch/mips/loongson32/common/time.o CC arch/mips/loongson32/ls1c/board.o CHK include/generated/compile.h CC arch/mips/vdso/vgettimeofday.o HOSTCC arch/mips/vdso/genvdso CC kernel/sched/cputime.o In file included from /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/mmiowb.h:5:0, from /home/hp/projects/Linux/temp/linux-stable/include/linux/spinlock.h:61, from /home/hp/projects/Linux/temp/linux-stable/include/linux/wait.h:9, from /home/hp/projects/Linux/temp/linux-stable/include/linux/wait_bit.h:8, from /home/hp/projects/Linux/temp/linux-stable/include/linux/fs.h:6, from /home/hp/projects/Linux/temp/linux-stable/arch/mips/mm/cache.c:9: /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/io.h: In function ‘phys_to_virt’: /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/io.h:122:9: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] return (void *)(address + PAGE_OFFSET - PHYS_OFFSET); ^ In file included from /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/mmiowb.h:5:0, from /home/hp/projects/Linux/temp/linux-stable/include/linux/spinlock.h:61, from /home/hp/projects/Linux/temp/linux-stable/include/linux/wait.h:9, from /home/hp/projects/Linux/temp/linux-stable/include/linux/pid.h:6, from /home/hp/projects/Linux/temp/linux-stable/include/linux/sched.h:14, from /home/hp/projects/Linux/temp/linux-stable/include/linux/sched/signal.h:7, from /home/hp/projects/Linux/temp/linux-stable/arch/mips/kernel/branch.c:10: /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/io.h: In function ‘phys_to_virt’: /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/io.h:122:9: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] return (void *)(address + PAGE_OFFSET - PHYS_OFFSET); ......... Only change CAC_BASE Does NOT work Huang Pei
On Fri, 5 Mar 2021, Huang Pei wrote: > /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/io.h: In function ‘phys_to_virt’: > /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/io.h:122:9: error: cast to pointer > from integer of different size [-Werror=int-to-pointer-cast] > return (void *)(address + PAGE_OFFSET - PHYS_OFFSET); > > > ......... > > Only change CAC_BASE Does NOT work Thank you for checking. Right. I don't know why it fails for `phys_to_virt' where `address' is of the `unsigned long' type, but there are other places where the macros themselves are cast to `void *'. We may want to rework that stuff, but not necessarily on this occasion. Use an explicit cast of the macro to `s64' here then, as my other suggestion was. Anything is better than hardcoded magic numbers. Maciej
Hi, >On Fri, 5 Mar 2021, Huang Pei wrote: > >> /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/io.h: In function ‘phys_to_virt’: >> /home/hp/projects/Linux/temp/linux-stable/arch/mips/include/asm/io.h:122:9: error: cast to pointer >> from integer of different size [-Werror=int-to-pointer-cast] >> return (void *)(address + PAGE_OFFSET - PHYS_OFFSET); >> >> >> ......... >> >> Only change CAC_BASE Does NOT work > > Thank you for checking. > > Right. I don't know why it fails for `phys_to_virt' where `address' is >of the `unsigned long' type, but there are other places where the macros >themselves are cast to `void *'. We may want to rework that stuff, but >not necessarily on this occasion. > > Use an explicit cast of the macro to `s64' here then, as my other >suggestion was. Anything is better than hardcoded magic numbers. > > Maciej cast into s64 works. I will resend v3 soon. Another issue, please take a look at GCC bug 99217, there is a partial fix, but I am not sure that if it is advised mips16 should also be covered and how? I found the ftrace on MIPS is not safe on SMP, since when disabling tracing, the callsite of _mcount need two writes to transform jal _mcount addiu sp, sp, -offset into nop nop no matter in what order these two writes are seen, it is wrecked in these two case jal _mcount nop or nop addiu sp, sp, -offset so, I add a new ftrace implementation based on -fpatchable-function-entry=3, which is blocked by GCC bug 99217 see https://lore.kernel.org/linux-mips/20210305101933.9799-1-huangpei@loongson.cn/ At last, is it possible to add fix 99217 and 93242 into gcc 8.5? ----------------- Huang Pei
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 2000bb2b0220..517509ad8596 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -2142,6 +2142,7 @@ config CPU_SUPPORTS_HUGEPAGES depends on !(32BIT && (ARCH_PHYS_ADDR_T_64BIT || EVA)) config MIPS_PGD_C0_CONTEXT bool + depends on 64BIT default y if 64BIT && (CPU_MIPSR2 || CPU_MIPSR6) && !CPU_XLP # diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c index a7521b8f7658..5bb9724578f7 100644 --- a/arch/mips/mm/tlbex.c +++ b/arch/mips/mm/tlbex.c @@ -848,8 +848,8 @@ void build_get_pmde64(u32 **p, struct uasm_label **l, struct uasm_reloc **r, /* Clear lower 23 bits of context. */ uasm_i_dins(p, ptr, 0, 0, 23); - /* 1 0 1 0 1 << 6 xkphys cached */ - uasm_i_ori(p, ptr, ptr, 0x540); + /* insert bit[63:59] of CAC_BASE into bit[11:6] of ptr */ + uasm_i_ori(p, ptr, ptr, (CAC_BASE >> 53)); uasm_i_drotr(p, ptr, ptr, 11); #elif defined(CONFIG_SMP) UASM_i_CPUID_MFC0(p, ptr, SMP_CPUID_REG); @@ -1106,6 +1106,7 @@ struct mips_huge_tlb_info { bool need_reload_pte; }; +#ifdef CONFIG_64BIT static struct mips_huge_tlb_info build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, struct uasm_reloc **r, unsigned int tmp, @@ -1164,8 +1165,8 @@ build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, if (pgd_reg == -1) { vmalloc_branch_delay_filled = 1; - /* 1 0 1 0 1 << 6 xkphys cached */ - uasm_i_ori(p, ptr, ptr, 0x540); + /* insert bit[63:59] of CAC_BASE into bit[11:6] of ptr */ + uasm_i_ori(p, ptr, ptr, (CAC_BASE >> 53)); uasm_i_drotr(p, ptr, ptr, 11); } @@ -1292,7 +1293,7 @@ build_fast_tlb_refill_handler (u32 **p, struct uasm_label **l, return rv; } - +#endif /* * For a 64-bit kernel, we are using the 64-bit XTLB refill exception * because EXL == 0. If we wrap, we can also use the 32 instruction
CP0 Context has enough room for wraping pgd into its 41-bit PTEBase field. +. For XPHYS, the trick is that pgd is 4kB aligned, and the PABITS <= 48, only save 48 - 12 + 5(for bit[63:59]) = 41 bits, aka. : bit[63:59] | 0000 0000 000 | bit[47:12] | 0000 0000 0000 +. for CKSEG0, only save 29 - 12 = 17 bits +. use CAC_BASE for accessing bit[63:59] of pgd +. let CONFIG_MIPS_PGD_C0_CONTEXT depend on 64bit, and protect build_fast_mips_refill_handler from 32bit building Signed-off-by: Huang Pei <huangpei@loongson.cn> --- arch/mips/Kconfig | 1 + arch/mips/mm/tlbex.c | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-)