On Tue, 3 Sep 2019 at 17:09, Richard Henderson <richard.hender...@linaro.org> wrote: > > We had two different mechanisms to force a recheck of the tlb. > > Before TLB_RECHECK was introduced, we had a PAGE_WRITE_INV bit > that would immediate set TLB_INVALID_MASK, which automatically > means that a second check of the tlb entry fails. > > We can use the same mechanism to handle small pages. > Conserve TLB_* bits by removing TLB_RECHECK. > > Reviewed-by: David Hildenbrand <da...@redhat.com> > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > ---
> @@ -1265,27 +1269,6 @@ load_helper(CPUArchState *env, target_ulong addr, > TCGMemOpIdx oi, > if ((addr & (size - 1)) != 0) { > goto do_unaligned_access; > } > - > - if (tlb_addr & TLB_RECHECK) { > - /* > - * This is a TLB_RECHECK access, where the MMU protection > - * covers a smaller range than a target page, and we must > - * repeat the MMU check here. This tlb_fill() call might > - * longjump out if this access should cause a guest exception. > - */ > - tlb_fill(env_cpu(env), addr, size, > - access_type, mmu_idx, retaddr); > - index = tlb_index(env, mmu_idx, addr); > - entry = tlb_entry(env, mmu_idx, addr); > - > - tlb_addr = code_read ? entry->addr_code : entry->addr_read; > - tlb_addr &= ~TLB_RECHECK; > - if (!(tlb_addr & ~TARGET_PAGE_MASK)) { > - /* RAM access */ > - goto do_aligned_access; > - } > - } > - > return io_readx(env, &env_tlb(env)->d[mmu_idx].iotlb[index], > mmu_idx, addr, retaddr, access_type, op); > } In the old version of this code, we do the "tlb fill if TLB_RECHECK is set", and then we say "now we've done the refill have we actually got RAM", and we avoid calling io_readx() if that is the case. This is necessary because io_readx() will misbehave if you try to call it on RAM (notably if what we have is notdirty-mem then we need to do the read-from-actual-host-ram because the IO ops backing notdirty-mem are intended for writes only). With this patch applied, we seem to have lost the handling for if the tlb_fill in a TLB_RECHECK case gives us back some real RAM. (Similarly for store_helper().) I think this is what's causing Mark Cave-Ayland's Solaris test case to fail. More generally, I don't really understand why this merging is correct -- "TLB needs a recheck" is not the same thing as "TLB is invalid" and I don't think we can merge the two bits. thanks -- PMM