On Wed, Mar 2, 2016 at 6:41 PM, Ralf Baechle <ralf@xxxxxxxxxxxxxx> wrote:
> On Sat, Feb 27, 2016 at 06:07:36PM +0800, Huacai Chen wrote:
>
>> Loongson-2 has a 4 entry itlb which is a subset of jtlb, Loongson-3 has
>> a 4 entry itlb and a 4 entry dtlb which are subsets of jtlb. We should
>> write diag register to invalidate itlb/dtlb when flushing jtlb because
>> itlb/dtlb are not totally transparent to software.
>>
>> For Loongson-3A R2 (and newer), we should invalidate ITLB, DTLB, VTLB
>> and FTLB before we enable/disable FTLB.
>>
>> Signed-off-by: Huacai Chen <chenhc@xxxxxxxxxx>
>> ---
>> arch/mips/kernel/cpu-probe.c | 2 ++
>> arch/mips/mm/tlb-r4k.c | 27 +++++++++++++++------------
>> 2 files changed, 17 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>> index 2a2ae86..ef605e2 100644
>> --- a/arch/mips/kernel/cpu-probe.c
>> +++ b/arch/mips/kernel/cpu-probe.c
>> @@ -562,6 +562,8 @@ static int set_ftlb_enable(struct cpuinfo_mips *c, int enable)
>> << MIPS_CONF7_FTLBP_SHIFT));
>> break;
>> case CPU_LOONGSON3:
>> + /* Flush ITLB, DTLB, VTLB and FTLB */
>> + write_c0_diag(1<<2 | 1<<3 | 1<<12 | 1<<13);
>
> Too many magic numbers. Could you use defines for the magic numbers you're
> writing to these registers?
OK, I know, but if I define macros, put them in tlb.h or tlbflush.h or
loongson.h in mach-loongson64?
>
>> /* Loongson-3 cores use Config6 to enable the FTLB */
>> config = read_c0_config6();
>> if (enable)
>> diff --git a/arch/mips/mm/tlb-r4k.c b/arch/mips/mm/tlb-r4k.c
>> index c17d762..7593529 100644
>> --- a/arch/mips/mm/tlb-r4k.c
>> +++ b/arch/mips/mm/tlb-r4k.c
>> @@ -28,25 +28,28 @@
>> extern void build_tlb_refill_handler(void);
>>
>> /*
>> - * LOONGSON2/3 has a 4 entry itlb which is a subset of dtlb,
>> - * unfortunately, itlb is not totally transparent to software.
>> + * LOONGSON-2 has a 4 entry itlb which is a subset of jtlb, LOONGSON-3 has
>> + * a 4 entry itlb and a 4 entry dtlb which are subsets of jtlb. Unfortunately,
>> + * itlb/dtlb are not totally transparent to software.
>> */
>> -static inline void flush_itlb(void)
>> +static inline void flush_spec_tlb(void)
>> {
>> switch (current_cpu_type()) {
>> case CPU_LOONGSON2:
>> + write_c0_diag(0x4);
>
> Same here.
>
>> + break;
>> case CPU_LOONGSON3:
>> - write_c0_diag(4);
>> + write_c0_diag(0xc);
>
> And here.
>
> Also, why did the magic number change from 4 to 0xc?
0x4 means flush itlb only, 0xc means flush itlb and dtlb.
>
>> break;
>> default:
>> break;
>> }
>> }
>>
>> -static inline void flush_itlb_vm(struct vm_area_struct *vma)
>> +static inline void flush_spec_tlb_vm(struct vm_area_struct *vma)
>> {
>> if (vma->vm_flags & VM_EXEC)
>> - flush_itlb();
>> + flush_spec_tlb();
>
> Hm.. "spec tlb" is not very descriptive in my opinion. How about
> renameing this function to flush_micro_tlb().
OK, I will change the name.
>
> Ralf
>
