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

[Richard Henderson]

On 2/4/22 18:36, Alistair Francis wrote:
> > So we need to check for write permission first, before performing the load.
>
> Isn't that what this series does though, albeit for IO accesses only

No.

> Using probe_write() solves part of this problem. If we have RAM at the
> address but no permissions to access it, then probe_write() will
> generate a store/AMO fault. But it won't help if nothing is mapped at
> that address.
>
> Let's say you are performing an atomic operation at an unmapped
> address 0x00, in M mode (so no MMU). probe_write() will eventually
> call riscv_cpu_tlb_fill() and get_physical_address(). On a system
> without an MMU and no PMP enforcement we get full read/write/execute
> permissions from riscv_cpu_tlb_fill(). So probe_write() succeeds.

True.

But there it's not a permission problem, per se.  What are you supposed to get here on 
riscv?  On some other cpus you don't get a "normal" segv, but a machine check.  I suppose 
you still want to see "store" rather than "load" in reporting that...


> > > 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.
>
> If you have permissions then that's fine. I thought we went via the
> slow path if the permission check fails?

We do.  But you haven't changed any permissions checks, so you don't really know what 
you're getting -- you may not arrive at the slow path at all.


r~