[Top][All Lists]

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

Re: [PATCH 0/2] RISC-V: Correctly generate store/amo faults

From: Richard Henderson
Subject: Re: [PATCH 0/2] RISC-V: Correctly generate store/amo faults
Date: Wed, 2 Feb 2022 11:37:10 +1100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0

On 2/1/22 15:40, Alistair Francis wrote:
Alistair, you're only changing the reporting of MMIO faults for which read 
permission is
missing.  Importantly, the actual permission check is done elsewhere, and you 
changing that to perform a write access check.  Also, you very much need to 
handle normal

I'm a little confused with this part.

Looking at tcg_gen_atomic_cmpxchg_i64() for example we either:
  1. call tcg_gen_qemu_ld_i64() then tcg_gen_qemu_st_i64()
  2. call table_cmpxchg[] which eventually calls atomic_mmu_lookup()
  3. call tcg_gen_atomic_cmpxchg_i32() which is pretty much the same as
the above two

That means in both cases we end up performing a load or tlb_fill(..,
MMU_DATA_LOAD, ..) operation as well as a store operation.


So we are already performing a write permission check...

... but we're doing so *after* the load.

Which means that for a completely unmapped page (as opposed to a read-only page) we will generate a read fault, which generates RISCV_EXCP_LOAD_ACCESS_FAULT and *not* RISCV_EXCP_STORE_AMO_ACCESS_FAULT.

So we need to check for write permission first, before performing the load.

Can't we just do the check in the slow path? By the time we get to the
fast path shouldn't we already have permissions?

No, the fast path performs the permissions check on one bit [rwx] depending on which tlb comparator it loads.

As in add a new INDEX_op_qemu_ld_write_perm_i32/i64, make edits to
atomic_mmu_lookup() and all of the plumbing for those?

That's one possibility, sure.


reply via email to

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