Hi Michael,
On 03/09/2018 05:12 AM, Michael Clark wrote:
> From reading other code that accesses memory regions directly,
> it appears that the rcu_read_lock needs to be held. Note: the
> original code for accessing RAM directly was added because
> there is no other way to use atomic_cmpxchg on guest physical
> address space.
>
> Cc: Sagar Karandikar <sag...@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbast...@mail.uni-paderborn.de>
> CC: Stefan O'Rear <sore...@gmail.com>
> Signed-off-by: Michael Clark <m...@sifive.com>
> Signed-off-by: Palmer Dabbelt <pal...@sifive.com>
> ---
> target/riscv/helper.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/riscv/helper.c b/target/riscv/helper.c
> index 02cbcea..228933c 100644
> --- a/target/riscv/helper.c
> +++ b/target/riscv/helper.c
> @@ -209,6 +209,7 @@ restart:
> as the PTE is no longer valid */
> MemoryRegion *mr;
> hwaddr l = sizeof(target_ulong), addr1;
> + rcu_read_lock();
> mr = address_space_translate(cs->as, pte_addr,
> &addr1, &l, false);
> if (memory_access_is_direct(mr, true)) {
> @@ -222,16 +223,19 @@ restart:
> target_ulong old_pte =
> atomic_cmpxchg(pte_pa, pte, updated_pte);
> if (old_pte != pte) {
> + rcu_read_unlock();
> goto restart;
> } else {
> pte = updated_pte;
> }
> #endif
> } else {
> + rcu_read_unlock();
> /* misconfigured PTE in ROM (AD bits are not preset) or
> * PTE is in IO space and can't be updated atomically */
> return TRANSLATE_FAIL;
> }
> + rcu_read_unlock();
Can you refactor to have a unique pair of lock/unlock?
This would be less bug-prone.
Thanks,
Phil.
> }
>
> /* for superpage mappings, make a fake leaf PTE for the TLB's
>
