[Qemu-devel] exec.c:invalidate_and_set_dirty() only checks whether first page in its range is dirty...

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

I'm trying to track down a bug in ARM TCG where we:
 * boot a guest
 * run 'shutdown -r now' to trigger a reboot
 * on reboot, crash when running userspace because the contents
   of physical RAM have changed but the translated code from
   before the shutdown was never invalidated

This is with a virtio-mmio block device as the disk.

Debugging indicates that when the post-reboot guest reloads
binaries from disk into ram we fail to invalidate the cached
translations. For the specific case I looked at, we have a
translation of code at ramaddr_t 0x806e000. The disk load
pulls 0x16000 bytes of data off disk to address 0x806a000.
Virtio correctly calls address_space_unmap(), which is supposed
to be what marks the ram range as dirty. It in turn calls
invalidate_and_set_clean(). However invalidate_and_set_clean()
just does this:

    if (cpu_physical_memory_is_clean(addr)) {
        /* invalidate code */
        tb_invalidate_phys_page_range(addr, addr + length, 0);
        /* set dirty bit */
        cpu_physical_memory_set_dirty_range_nocode(addr, length);
    }

So if the first page in the range (here 0x806a000) happens
to be dirty then we won't do anything, even if later pages
in the range do need to be invalidated. Also, we'll call
tb_invalidate_phys_page_range() with a start/end which may
be in different physical pages, which is forbidden by that
function's API.

I guess invalidate_and_set_clean() really needs to be
fixed to loop through each page in the range; does anybody
know how this is supposed to work (or why nobody's noticed
this bug before :-)) ?

thanks
-- PMM