[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-riscv] TLB refresh issue

From: Quentin Mundell
Subject: Re: [Qemu-riscv] TLB refresh issue
Date: Thu, 20 Jun 2019 09:32:49 +0000

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.


‐‐‐‐‐‐‐ 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.


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

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?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]