| Message ID | 20231220153205.11072-1-ivan.klokov@syntacore.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] target/riscv: pmp: Ignore writes when RW=01 and MML=0 | expand |
On 12/20/23 12:32, Ivan Klokov wrote: > This patch changes behavior on writing RW=01 to pmpcfg with MML=0. > RWX filed is form of collective WARL with the combination of > pmpcfg.RW=01 remains reserved for future standard use. > > According to definition of WARL writing the CSR has no other side > effect. But current implementation change architectural state and > change system behavior. After writing we will get unreadable-unwriteble > region regardless on the previous state. > > On the other side WARL said that we should read legal value and nothing > says about what we should write. Current behavior change system state > regardless of whether we read this register or not. > > Fixes: ac66f2f0 ("target/riscv: pmp: Ignore writes when RW=01") > > Signed-off-by: Ivan Klokov <ivan.klokov@syntacore.com> > --- Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > target/riscv/pmp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 162e88a90a..c0b814699e 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -126,7 +126,7 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val) > /* If !mseccfg.MML then ignore writes with encoding RW=01 */ > if ((val & PMP_WRITE) && !(val & PMP_READ) && > !MSECCFG_MML_ISSET(env)) { > - val &= ~(PMP_WRITE | PMP_READ); > + return false; > } > env->pmp_state.pmp[pmp_index].cfg_reg = val; > pmp_update_rule_addr(env, pmp_index);
On Thu, Dec 21, 2023 at 1:32 AM Ivan Klokov <ivan.klokov@syntacore.com> wrote: > > This patch changes behavior on writing RW=01 to pmpcfg with MML=0. > RWX filed is form of collective WARL with the combination of > pmpcfg.RW=01 remains reserved for future standard use. > > According to definition of WARL writing the CSR has no other side > effect. But current implementation change architectural state and > change system behavior. After writing we will get unreadable-unwriteble > region regardless on the previous state. > > On the other side WARL said that we should read legal value and nothing > says about what we should write. Current behavior change system state > regardless of whether we read this register or not. > > Fixes: ac66f2f0 ("target/riscv: pmp: Ignore writes when RW=01") > > Signed-off-by: Ivan Klokov <ivan.klokov@syntacore.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/pmp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 162e88a90a..c0b814699e 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -126,7 +126,7 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val) > /* If !mseccfg.MML then ignore writes with encoding RW=01 */ > if ((val & PMP_WRITE) && !(val & PMP_READ) && > !MSECCFG_MML_ISSET(env)) { > - val &= ~(PMP_WRITE | PMP_READ); > + return false; > } > env->pmp_state.pmp[pmp_index].cfg_reg = val; > pmp_update_rule_addr(env, pmp_index); > -- > 2.34.1 > >
On Thu, Dec 21, 2023 at 1:32 AM Ivan Klokov <ivan.klokov@syntacore.com> wrote: > > This patch changes behavior on writing RW=01 to pmpcfg with MML=0. > RWX filed is form of collective WARL with the combination of > pmpcfg.RW=01 remains reserved for future standard use. > > According to definition of WARL writing the CSR has no other side > effect. But current implementation change architectural state and > change system behavior. After writing we will get unreadable-unwriteble > region regardless on the previous state. > > On the other side WARL said that we should read legal value and nothing > says about what we should write. Current behavior change system state > regardless of whether we read this register or not. > > Fixes: ac66f2f0 ("target/riscv: pmp: Ignore writes when RW=01") > > Signed-off-by: Ivan Klokov <ivan.klokov@syntacore.com> Thanks! Applied to riscv-to-apply.next Alistair > --- > target/riscv/pmp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 162e88a90a..c0b814699e 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -126,7 +126,7 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val) > /* If !mseccfg.MML then ignore writes with encoding RW=01 */ > if ((val & PMP_WRITE) && !(val & PMP_READ) && > !MSECCFG_MML_ISSET(env)) { > - val &= ~(PMP_WRITE | PMP_READ); > + return false; > } > env->pmp_state.pmp[pmp_index].cfg_reg = val; > pmp_update_rule_addr(env, pmp_index); > -- > 2.34.1 > >
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c index 162e88a90a..c0b814699e 100644 --- a/target/riscv/pmp.c +++ b/target/riscv/pmp.c @@ -126,7 +126,7 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val) /* If !mseccfg.MML then ignore writes with encoding RW=01 */ if ((val & PMP_WRITE) && !(val & PMP_READ) && !MSECCFG_MML_ISSET(env)) { - val &= ~(PMP_WRITE | PMP_READ); + return false; } env->pmp_state.pmp[pmp_index].cfg_reg = val; pmp_update_rule_addr(env, pmp_index);
This patch changes behavior on writing RW=01 to pmpcfg with MML=0. RWX filed is form of collective WARL with the combination of pmpcfg.RW=01 remains reserved for future standard use. According to definition of WARL writing the CSR has no other side effect. But current implementation change architectural state and change system behavior. After writing we will get unreadable-unwriteble region regardless on the previous state. On the other side WARL said that we should read legal value and nothing says about what we should write. Current behavior change system state regardless of whether we read this register or not. Fixes: ac66f2f0 ("target/riscv: pmp: Ignore writes when RW=01") Signed-off-by: Ivan Klokov <ivan.klokov@syntacore.com> --- target/riscv/pmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)