| Message ID | 20230413090122.65228-1-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| Headers | show |
| Series | target/riscv: Fix PMP related problem | expand |
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 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(-) >
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(-) >>
On 2023/4/18 11:36, Weiwei Li wrote: > > 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. TLB will be filled only when PMP check and PTW check pass. > So the other address access may hit the TLB(if first access of the > page didn't hit the PMP entry) This page will not be filled to TLB if the first access of the page didn't pass the PMP check. > > and bypass the pmp check. This is fixed by patch 1. Never it should be. Zhiwei > > 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(-) >>>
On 2023/4/18 12:47, LIU Zhiwei wrote: > > On 2023/4/18 11:36, Weiwei Li wrote: >> >> 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. > TLB will be filled only when PMP check and PTW check pass. >> So the other address access may hit the TLB(if first access of the >> page didn't hit the PMP entry) > This page will not be filled to TLB if the first access of the page > didn't pass the PMP check. I have given an example for this bypass in the replied email of patch 1. Regards, Weiwei Li >> >> and bypass the pmp check. This is fixed by patch 1. > > Never it should be. > > Zhiwei > >> >> 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(-) >>>>
This patchset tries to fix the PMP bypass problem issue https://gitlab.com/qemu-project/qemu/-/issues/1542 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(-)