Re: [PATCH v3 4/4] riscv: Improve flush_tlb_kernel_range()

[Alexandre Ghiti <alexghiti@rivosinc.com>]

Hi Prabhakar,

On Wed, Sep 6, 2023 at 3:55 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Alexandre,
>
> On Wed, Sep 6, 2023 at 1:43 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >
> > On Wed, Sep 6, 2023 at 2:24 PM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > >
> > > Hi Alexandre,
> > >
> > > On Wed, Sep 6, 2023 at 1:18 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > >
> > > > On Wed, Sep 6, 2023 at 2:09 PM Lad, Prabhakar
> > > > <prabhakar.csengg@gmail.com> wrote:
> > > > >
> > > > > Hi Alexandre,
> > > > >
> > > > > On Wed, Sep 6, 2023 at 1:01 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > > >
> > > > > > Hi Prabhakar,
> > > > > >
> > > > > > On Wed, Sep 6, 2023 at 1:49 PM Lad, Prabhakar
> > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Alexandre,
> > > > > > >
> > > > > > > On Tue, Aug 1, 2023 at 9:58 AM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > > > > >
> > > > > > > > This function used to simply flush the whole tlb of all harts, be more
> > > > > > > > subtile and try to only flush the range.
> > > > > > > >
> > > > > > > > The problem is that we can only use PAGE_SIZE as stride since we don't know
> > > > > > > > the size of the underlying mapping and then this function will be improved
> > > > > > > > only if the size of the region to flush is < threshold * PAGE_SIZE.
> > > > > > > >
> > > > > > > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > > > > > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > > > > > > > ---
> > > > > > > >  arch/riscv/include/asm/tlbflush.h | 11 +++++-----
> > > > > > > >  arch/riscv/mm/tlbflush.c          | 34 +++++++++++++++++++++++--------
> > > > > > > >  2 files changed, 31 insertions(+), 14 deletions(-)
> > > > > > > >
> > > > > > > After applying this patch, I am seeing module load issues on RZ/Five
> > > > > > > (complete log [0]). I am testing defconfig + [1] (rz/five related
> > > > > > > configs).
> > > > > > >
> > > > > > > Any pointers on what could be an issue here?
> > > > > >
> > > > > > Can you give me the exact version of the kernel you use? The trap
> > > > > > addresses are vmalloc addresses, and a fix for those landed very late
> > > > > > in the release cycle.
> > > > > >
> > > > > I am using next-20230906, Ive pushed a branch [1] for you to have a look.
> > > > >
> > > > > [0] https://github.com/prabhakarlad/linux/tree/rzfive-debug
> > > >
> > > > Great, thanks, I had to get rid of this possibility :)
> > > >
> > > > As-is, I have no idea, can you try to "bisect" the problem? I mean
> > > > which patch in the series leads to those traps?
> > > >
> > > Oops sorry for not mentioning earlier, this is the offending patch
> > > which leads to the issues seen on rz/five.
> >
> > Ok, so at least I found the following problem, but I don't see how
> > that could fix your issue: can you give a try anyway? I keep looking
> > into this, thanks
> >
> > diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> > index df2a0838c3a1..b5692bc6c76a 100644
> > --- a/arch/riscv/mm/tlbflush.c
> > +++ b/arch/riscv/mm/tlbflush.c
> > @@ -239,7 +239,7 @@ void flush_tlb_range(struct vm_area_struct *vma,
> > unsigned long start,
> >
> >  void flush_tlb_kernel_range(unsigned long start, unsigned long end)
> >  {
> > -       __flush_tlb_range(NULL, start, end, PAGE_SIZE);
> > +       __flush_tlb_range(NULL, start, end - start, PAGE_SIZE);
> >  }
> >
> I am able to reproduce the issue with the above change too.

I can't reproduce the problem on my Unmatched or Qemu, so it is not
easy to debug. But I took another look at your traces and something is
weird to me. In the following trace (and there is another one), the
trap is taken at 0xffffffff015ca034, which is the beginning of
rz_ssi_probe(): that's a page fault, so no translation was found (or
an invalid one is cached).

[   16.586527] Unable to handle kernel paging request at virtual
address ffffffff015ca034
[   16.594750] Oops [#3]
...
[   16.622000] epc : rz_ssi_probe+0x0/0x52a [snd_soc_rz_ssi]
...
[   16.708697] status: 0000000200000120 badaddr: ffffffff015ca034
cause: 000000000000000c
[   16.716580] [<ffffffff015ca034>] rz_ssi_probe+0x0/0x52a
[snd_soc_rz_ssi]
...

But then here we are able to read the code at this same address:
[   16.821620] Code: 0109 6597 0000 8593 5f65 7097 7f34 80e7 7aa0 b7a9
(7139) f822

So that looks like a "transient" error. Do you know if you uarch
caches invalid TLB entries? If you don't know, I have just written
some piece of code to determine if it does, let me know.

Do those errors always happen?

>
> Cheers,
> Prabhakar