On Mon, Aug 24, 2020 at 03:13:20PM +0800, Huacai Chen wrote:
the fix in __update_tlb is same as [1], which is to check if the CP0
index returned by tlbp is in FTLB range, if so, then invalidate this
entry and rewrite it with Huge Page by tlbwr again, otherwise tlbwr
when not hit or tlbwi when hit in VTLB range.
The previous patch, aka patch 2, just reveal the problem *explicitly*. Just
with Patch 3 but without Patch 2, the __update_tlb would write a Huge Page
mapping into VTLB without _PAGE_VALID set successfully, which still need [1]
to cover it in the second TLb Invalid exception, and make Patch 3 looks
like DOES NOT WORK
Anyway，Patch 3 can not fix it perfectly, without Patch 2
> Hi, Pei,
>
> On Sat, Aug 22, 2020 at 12:27 PM 黄沛 <huangpei@xxxxxxxxxxx> wrote:
> >
> >
> >
> > 原始消息
> > 发件人： Huacai Chen
> > 已发送： 2020年8月21日星期五 18:37
> > 收件人： Huang Pei
> > 抄送： Thomas Bogendoerfer; Paul Ambrose; Li Xuefeng; Yang Tiezhu; Gao Juxin; Fuxin Zhang; open list:MIPS
> > 主题： Re: [PATCH 3/3] Revert "MIPS: Flush wrong invalid FTLB entry for huge page"
> >
> > Got it, it is "too late", anything else?
> >
> > I will re-send it next week
> How to fix it in __update_tlb? the previous patch?
>
> Huacai
> >
> >
> > On Fri, Aug 21, 2020 at 3:24 PM Huang Pei <huangpei@xxxxxxxxxxx> wrote:
> > >
> > > This reverts commit 0115f6cbf26663c86496bc56eeea293f85b77897.
> > >
> > > The fix in 0115f6cbf26663c86496bc56eeea293f85b77897 is two late, since
> > Do you means "too late"?
> >
> > > __update_tlb hit the same problem first. So let __update_tlb fix it
> > >
> > > Signed-off-by: Huang Pei <huangpei@xxxxxxxxxxx>
> > > ---
> > > arch/mips/mm/tlb-r4k.c | 15 ++++++++++++++-
> > > arch/mips/mm/tlbex.c | 25 ++++---------------------
> > > 2 files changed, 18 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/arch/mips/mm/tlb-r4k.c b/arch/mips/mm/tlb-r4k.c
> > > index 38e2894d5fa3..cb8afa326b2c 100644
> > > --- a/arch/mips/mm/tlb-r4k.c
> > > +++ b/arch/mips/mm/tlb-r4k.c
> > > @@ -328,6 +328,7 @@ void __update_tlb(struct vm_area_struct * vma, unsigned long address, pte_t pte)
> > > /* this could be a huge page */
> > > if (pmd_huge(*pmdp)) {
> > > unsigned long lo;
> > > + unsigned long entryhi;
> > > write_c0_pagemask(PM_HUGE_MASK);
> > > ptep = (pte_t *)pmdp;
> > > lo = pte_to_entrylo(pte_val(*ptep));
> > > @@ -335,7 +336,19 @@ void __update_tlb(struct vm_area_struct * vma, unsigned long address, pte_t pte)
> > > write_c0_entrylo1(lo + (HPAGE_SIZE >> 7));
> > >
> > > mtc0_tlbw_hazard();
> > > - if (idx < 0)
> > > + if (idx >= current_cpu_data.tlbsizevtlb) {
> > > + /* hit in FTLB.
> > > + * Invalid it then tlbwr, since FTLB hold only base page*/
> > > + entryhi = read_c0_entryhi();
> > > + write_c0_entryhi(MIPS_ENTRYHI_EHINV);
> > > + tlb_write_indexed();
> > > + tlbw_use_hazard();
> > > + write_c0_entryhi(entryhi);
> > > +
> > > + }
> > > +
> > > +
> > > + if (idx < 0 || idx >= current_cpu_data.tlbsizevtlb)
> > > tlb_write_random();
> > > else
> > > tlb_write_indexed();
> > > diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> > > index 14f8ba93367f..9c4cd08c00d3 100644
> > > --- a/arch/mips/mm/tlbex.c
> > > +++ b/arch/mips/mm/tlbex.c
> > > @@ -762,8 +762,7 @@ static void build_huge_update_entries(u32 **p, unsigned int pte,
> > > static void build_huge_handler_tail(u32 **p, struct uasm_reloc **r,
> > > struct uasm_label **l,
> > > unsigned int pte,
> > > - unsigned int ptr,
> > > - unsigned int flush)
> > > + unsigned int ptr)
> > > {
> > > #ifdef CONFIG_SMP
> > > UASM_i_SC(p, pte, 0, ptr);
> > > @@ -772,22 +771,6 @@ static void build_huge_handler_tail(u32 **p, struct uasm_reloc **r,
> > > #else
> > > UASM_i_SW(p, pte, 0, ptr);
> > > #endif
> > > - if (cpu_has_ftlb && flush) {
> > > - BUG_ON(!cpu_has_tlbinv);
> > > -
> > > - UASM_i_MFC0(p, ptr, C0_ENTRYHI);
> > > - uasm_i_ori(p, ptr, ptr, MIPS_ENTRYHI_EHINV);
> > > - UASM_i_MTC0(p, ptr, C0_ENTRYHI);
> > > - build_tlb_write_entry(p, l, r, tlb_indexed);
> > > -
> > > - uasm_i_xori(p, ptr, ptr, MIPS_ENTRYHI_EHINV);
> > > - UASM_i_MTC0(p, ptr, C0_ENTRYHI);
> > > - build_huge_update_entries(p, pte, ptr);
> > > - build_huge_tlb_write_entry(p, l, r, pte, tlb_random, 0);
> > > -
> > > - return;
> > > - }
> > > -
> > > build_huge_update_entries(p, pte, ptr);
> > > build_huge_tlb_write_entry(p, l, r, pte, tlb_indexed, 0);
> > > }
> > > @@ -2278,7 +2261,7 @@ static void build_r4000_tlb_load_handler(void)
> > > uasm_l_tlbl_goaround2(&l, p);
> > > }
> > > uasm_i_ori(&p, wr.r1, wr.r1, (_PAGE_ACCESSED | _PAGE_VALID));
> > > - build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2, 1);
> > > + build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2);
> > > #endif
> > >
> > > uasm_l_nopage_tlbl(&l, p);
> > > @@ -2334,7 +2317,7 @@ static void build_r4000_tlb_store_handler(void)
> > > build_tlb_probe_entry(&p);
> > > uasm_i_ori(&p, wr.r1, wr.r1,
> > > _PAGE_ACCESSED | _PAGE_MODIFIED | _PAGE_VALID | _PAGE_DIRTY);
> > > - build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2, 1);
> > > + build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2);
> > > #endif
> > >
> > > uasm_l_nopage_tlbs(&l, p);
> > > @@ -2391,7 +2374,7 @@ static void build_r4000_tlb_modify_handler(void)
> > > build_tlb_probe_entry(&p);
> > > uasm_i_ori(&p, wr.r1, wr.r1,
> > > _PAGE_ACCESSED | _PAGE_MODIFIED | _PAGE_VALID | _PAGE_DIRTY);
> > > - build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2, 0);
> > > + build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2);
> > > #endif
> > >
> > > uasm_l_nopage_tlbm(&l, p);
> > > --
> > > 2.17.1
> > >
