Re: [Qemu-devel] [PULL 23/32] tcg: Support MMU protection regions smaller than TARGET_PAGE_SIZE

[Peter Maydell <peter.maydell@linaro.org>]

On 28 June 2018 at 23:26, Laurent Vivier <laurent@vivier.eu> wrote:
> ./m68k-softmmu/qemu-system-m68k -M q800 \
>     -serial none -serial mon:stdio \
>     -kernel vmlinux-4.15.0-2-m68k \
>     -nographic

Thanks for the test case. I'm still investigating, but there
are a couple of things happening here.

First, there's a bug in get_page_addr_code()'s "is this a
TLB miss?" condition which was introduced in commit 71b9a45330fe22:

    if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
                 (addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK)))) {

takes a (not necessarily page aligned) address, and masks out
everything but the page-aligned top half (good) and the
TLB_INVALID bit (not good, because that could be either 0 or 1
depending on the address). This means sometimes we'll incorrectly
decide we got a miss in the TLB and do an unnecessary refill.

The second thing that's going on here is that the m68k target
code writes TLB entries for the same address with different
prot bits without doing a flush in between:

tlb_set_page_with_attrs: vaddr=0029b000 paddr=0x000000000029b000 prot=3 idx=0
tlb_set_page_with_attrs: vaddr=0029b000 paddr=0x000000000029b000 prot=7 idx=0

The tlb_set_page_with_attrs() code isn't expecting this, so
we end up with two TLB entries for the same address, one in
the main TLB and one in the victim cache TLB. The bug above
means that we get this sequence of events:
 * fill main TLB entry with prot=3 entry
 * later, fill main TLB with prot=7 entry, and evict prot=3
   entry to victim cache
 * hit on the prot=7 entry in the main TLB
 * refill condition incorrectly fails, but we hit in the victim cache
 * so we pull the prot=3 entry from victim to main TLB
 * prot=3 means "addr_code == -1", so the check of the TLB_RECHECK
   bit succeeds
 * in the TLB_RECHECK code we do a tlb_fill()
 * that fills in the main TLB with a prot=7 entry again, bouncing
   the prot=3 entry back out to the victim cache
 * prot=7 means the addr_code is correct, so we find ourselves in
   the "TLB_RECHECK but this is RAM" abort code path

I'm not sure whether it's supposed to be the responsibility
of the target code or the common accel/tcg code to ensure
that we don't have multiple TLB entries for the same address.

thanks
-- PMM