[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, 26 Jan 2022 11:09:15 +1100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0

On 1/24/22 4:17 PM, LIU Zhiwei wrote:

On 2022/1/24 上午8:59, Alistair Francis wrote:
From: Alistair Francis <alistair.francis@wdc.com>

This series adds a MO_ op to specify that a load instruction should
produce a store fault. This is used on RISC-V to produce a store/amo
fault when an atomic access fails.

Hi Alistair,

As Richard said,  we  can address this issue in two ways, probe_read(I think probe_write is typo)

It is not a typo: we want to verify that the memory is writable before we perform the load. This will raise a write fault on a no-access page before a read fault would be generated by the load. This may still generate the wrong fault for a write-only page. (Is such a page permission encoding possible with RISCV? Not all cpus support that, since at first blush it seems to be mostly useless. But some do, and a generic tcg feature should be designed with those in mind.)

In my opinion use MO_op in io_readx may be not right because the issue is not only with IO access. And MO_ op in io_readx is too later because the exception has been created when tlb_fill.

You are correct that changing only io_readx is insufficient.  Very much so.

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 aren't changing that to perform a write access check. Also, you very much need to handle normal memory not just MMIO. Which will require changes across all tcg/arch/, as well as in all of the memory access helpers in accel/tcg/.

We may not want to add this check along the normal hot path of a normal load, but create separate helpers for "load with write-permission-check". And we should answer the question of whether it should really be "load with read-write-permission-check", which will make the changes to tcg/arch/ harder.


reply via email to

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