Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma

[phantom]

I agree with you partly, my test case is actually from linux kernel, I notice 
the strange sfence.vma before write satp during write our teaching kernel.
I think, the strange code is used to bypass the qemu bug that Idan patched.
Because in hardware, if the stap is empty, sfence.vma will do nothing.
And that's why nobody report it.
Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
So, the kernel author reodered write stap and sfence.vma to make sfence.vma 
place in the same BB with write satp, instead of the following write stvec.
(If don't reorder, sfence.vma will place in the same BB with write stvec,
that will crash kernel, see my origin analysis).

However, in hardware, since tlb is empty, put the first sfence.vma before or 
after write satp is not really matters.
In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
write stap to invalid qemu's translation cache.

Jinyan


> -----Original Messages-----
> From: "Idan Horowitz" <idan.horowitz@gmail.com>
> Sent Time: 2022-03-30 15:35:19 (Wednesday)
> To: "Atish Patra" <atishp@atishpatra.org>
> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair 
> Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" 
> <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
> 
> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > I tested on v5.17 built from defconfig for rv64.
> >
> > Here is the kernel code executing sfence.vma
> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
> >
> 
> I believe this is a kernel bug and not a QEMU one. They perform a
> write to the SATP with the same ASID as the one used before (0) and
> then expect it to be used, without performing an sfence.vma following
> it.
> This was exposed by my change, as previously the write to the satp was
> performed in the same TB block as the sfence.vma *before it*, which
> meant the TLB was not filled in between the previous sfence and the
> write to SATP following it.
> I was able to reproduce the issue with the Fedora Rawhide image in the
> wiki, and I was able to resolve it by artificially forcing a TLB flush
> following all writes to SATP.
> I think the correct course of action here is to:
> 1. Report the issue to the linux kernel mailing list and/or contribute
> a patch that adds said missing sfence.vma following the SATP write.
> (Atish: Are you able to test if adding an sfence.vma in your kernel
> build fixes the issue?)
> 2. Restore the patch
> 
> >
> > --
> > Regards,
> > Atish
> 
> Idan Horowitz