Re: [PATCH 0/6] target/riscv: Fix PMP related problem

[Weiwei Li]

On 2023/4/18 11:07, LIU Zhiwei wrote:
> On 2023/4/13 17:01, Weiwei Li wrote:
> > This patchset tries to fix the PMP bypass problem issue 
> > https://gitlab.com/qemu-project/qemu/-/issues/1542
>
> Please add your analysis of this issue here.
>
> By the way, I think this problem is introduced by
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg939331.html

It seems have no relationship with this commit.

I think there are several problems for this issue:

1. TLB will not be cached only when the access address have matched PMP 
entry.  So the other address access  may hit the TLB(if first access of 
the page didn't hit the PMP entry)

and bypass the pmp check. This is fixed by patch 1.

2. Writing to pmpaddr  didn't trigger tlb flush. This is fixed by patch 3.

3. The tb isn't flushed when PMP permission changes, so It also may hit 
the tb and bypass the changed PMP check for instruction fetch. This is 
fixed by patch 5.

4. We set the tlb_size to 1 to make the TLB_INVALID_MASK set. However 
this flag will be cleared after fill_tlb, and this will make the host 
address be cached, and let the following instruction fetch in the same 
tb bypass the PMP check. This is fixed by patch 6.

Regards,

Weiwei Li

> I have commented on how to correct this patch. But by accident, it has 
> been merged.
>
> Zhiwei
>
> > The port is available here:
> > https://github.com/plctlab/plct-qemu/tree/plct-pmp-fix
> >
> > Weiwei Li (6):
> >    target/riscv: Update pmp_get_tlb_size()
> >    target/riscv: Move pmp_get_tlb_size apart from
> >      get_physical_address_pmp
> >    target/riscv: flush tlb when pmpaddr is updated
> >    target/riscv: Flush TLB only when pmpcfg/pmpaddr really changes
> >    target/riscv: flush tb when PMP entry changes
> >    accel/tcg: Remain TLB_INVALID_MASK in the address when TLB is
> >      re-filled
> >
> >   accel/tcg/cputlb.c        |  7 -----
> >   target/riscv/cpu_helper.c | 19 ++++---------
> >   target/riscv/pmp.c        | 60 ++++++++++++++++++++++++++-------------
> >   target/riscv/pmp.h        |  3 +-
> >   4 files changed, 47 insertions(+), 42 deletions(-)