Re: [Qemu-riscv] TLB refresh issue

[Quentin Mundell]

Upon further inspection, I finally found an issue with my sapt/fence/asid logic.

Thank you Jonathan for pointing me to https://github.com/qemu/qemu/commit/1e0d985fa9136a563168a3da66f3d17820404ee2 !

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Thursday, June 20, 2019 11:32 AM, Quentin Mundell <address@hidden> wrote:

Sorry, there was a typo in the commit hash, here is the good one: https://github.com/qemu/qemu/commit/c7b951718815694284501ed01fec7acb8654db7b#diff-f5f017ee174677b23830531cab545e78L531

I have double checked my vma fences and I think the logic behind is correct (I only flush when needed with regard to ASIDs).

As a test, I have added a full `sfence.vma` barrier after every `satp` write: I still have the same random crashes.

QM.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Wednesday, June 19, 2019 7:30 PM, Jonathan Behrens <address@hidden> wrote:

I wasn't able to find the commit you bisected to, but is your code executing a `sfence.vma` instruction after every instance that it changes satp? Until recently this wasn't necessary with QEMU because QEMU flushed the TLB more aggressively than required by the spec, but the behavior was chayou are referringnged in a recent commit.

Jonathan

On Wed, Jun 19, 2019 at 11:46 AM Quentin Mundell via Qemu-riscv <address@hidden> wrote:

Hello,

I have updated my QEMU version (from 3.x to today's master).

I now get all sort of weird and not easily reproducible exceptions triggered in my risc-v guest, happening from both user and supervisor mode.

The command I use is roughly `qemu-system-riscv64 -machine sifive_u -m 1024 -kernel mybinary.elf` (sorry, I cannot share the elf).

I have bisected this issue back to commit c7b95171881569284501ed01fec7acb8654db7b.

Some TLB flushes were removed in the following places:

target/riscv/cpu_helper.c: `csr_write_helper(env, s, CSR_MSTATUS);` -> `env->mstatus = s;` (twice)

target/riscv/op_helper.c: `csr_write_helper(env, s, CSR_MSTATUS);` -> `env->mstatus = s;` (twice)

These are the places where the privileged mode is switched.

If I add them back (`tlb_flush(env_cpu(env));`), everything is fine again.

However, in the `riscv_cpu_set_mode` function, I see the following comment:

> tlb_flush is unnecessary as mode is contained in mmu_idx

It would suggest adding back TLB flushes is not the way to go...

Does someone with better knowledge of the codebase can tell me how to proceed here?

- Should I submit a patch adding back the TLB flushes?

- Is it an issue somewhere else?

Regards

QM