accel/tcg/cputlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
The fix is to clear TLB_INVALID_MASK bit in tlb_addr, as it happens in other places e.g. load_helper().
Signed-off-by: Dmitriy Solovev <d.solovev@yadro.com>
Signed-off-by: Mikhail Tyutin <m.tyutin@yadro.com>
---
accel/tcg/cputlb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index ba44501a7c..900dfc1079 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1735,7 +1735,7 @@ bool tlb_plugin_lookup(CPUState *cpu, vaddr addr, int mmu_idx,
uintptr_t index = tlb_index(env, mmu_idx, addr);
uint64_t tlb_addr = is_store ? tlb_addr_write(tlbe) : tlbe->addr_read;
- if (likely(tlb_hit(tlb_addr, addr))) {
+ if (likely(tlb_hit(tlb_addr & ~TLB_INVALID_MASK, addr))) {
/* We must have an iotlb entry for MMIO */
if (tlb_addr & TLB_MMIO) {
CPUTLBEntryFull *full;
--
2.34.1
On 8/2/23 06:08, Mikhail Tyutin wrote: > The fix is to clear TLB_INVALID_MASK bit in tlb_addr, as it happens in other places e.g. > load_helper(). > > Signed-off-by: Dmitriy Solovev <d.solovev@yadro.com> > Signed-off-by: Mikhail Tyutin <m.tyutin@yadro.com> > --- > accel/tcg/cputlb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) The other places in load_helper happen only directly after tlb_fill has succeeded. Here you have no such guarantee. I think perhaps the save_iotlb_data() call should be applied to loads as well, and then tlb_plugin_lookup simplified. r~
> On 8/2/23 06:08, Mikhail Tyutin wrote: > > The fix is to clear TLB_INVALID_MASK bit in tlb_addr, as it happens in other places e.g. > > load_helper(). > > > > Signed-off-by: Dmitriy Solovev <d.solovev@yadro.com> > > Signed-off-by: Mikhail Tyutin <m.tyutin@yadro.com> > > --- > > accel/tcg/cputlb.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > The other places in load_helper happen only directly after tlb_fill has succeeded. Here > you have no such guarantee. > > I think perhaps the save_iotlb_data() call should be applied to loads as well, and then > tlb_plugin_lookup simplified. > Hello Richard, We performed testing on more scenarios and noticed that patch when save_iotlb_data() call is added to io_readx (https://patchew.org/QEMU/20230804110903.19968-1-m.tyutin@yadro.com/). It doesn't work for addresses in OCRAM region. Those accessed bypass io_writex/io_readx function and therefore don’t invoke save_iotlb_data(). So we observe the wrong value of cpu->saved_iotlb for it. Would not be better to get back to initial v1 approach when we clean TLB_INVALID_MASK flag in tlb_plugin_lookup()? It works well for those regions. (https://patchew.org/QEMU/bf8ae2fd-158a-57b6-6270-2e56b6506421@yadro.com)
On 8/9/23 06:17, Mikhail Tyutin wrote: > Would not be better to get back to initial v1 approach when we clean TLB_INVALID_MASK flag in > tlb_plugin_lookup()? It works well for those regions. You're just as likely to get invalid data. r~
© 2016 - 2024 Red Hat, Inc.