I recently bought a Mac with M1 Pro chip, and use QEMU to setup a Linux
virtual machine.  QEMU crashed when I started a VM with HVF accelerator
enabled and with the device, bochs-display, added.

After digging into the source code, I found that dirty-tracking in HVF
did not work properly, which made QEMU crashed. Therefore I made this
series of patches to fix the problem.

Followings are the summary of the changes that these patches make:
 1. Move HVF memory-related functions and codes into a new file
    hvf-mem.c
 2. Simplify the logics of adding and removing memory regions in HVF
    memory listener
 3. Fix HVF dirty-tracking logics for both Intel and Apple Silicon Macs
 4. Use GTree and dynamically-allocated structures to store HVF memory
    slots instead of fixed-size arrays. This makes memory slots more
    scalable. It is inspired by the recent changes in Linux kernel
    (v5.17) that use red-black trees instead of arrays to store
    in-kernel KVM memory slots.
 5. Add a lock to protect the data structures of HVF memory slots

Patches have been tested on Apple Silicon Macs and Intel Macs.

Yan-Jie Wang (7):
  hvf: move memory related functions from hvf-accel-ops.c to hvf-mem.c
  hvf: simplify data structures and codes of memory related functions
  hvf: use correct data types for addresses in memory related functions
  hvf: rename struct hvf_slot to HVFSlot
  hvf: fix memory dirty-tracking
  hvf: add a lock for memory related functions
  hvf: use GTree to store memory slots instead of fixed-size array

 accel/hvf/hvf-accel-ops.c | 221 +-------------------------
 accel/hvf/hvf-mem.c       | 318 ++++++++++++++++++++++++++++++++++++++
 accel/hvf/meson.build     |   1 +
 include/sysemu/hvf_int.h  |  18 +--
 target/arm/hvf/hvf.c      |   5 +
 target/i386/hvf/hvf.c     |  25 +--
 6 files changed, 334 insertions(+), 254 deletions(-)
 create mode 100644 accel/hvf/hvf-mem.c

-- 
2.34.1

On Mon, 28 Feb 2022 at 14:07, ~ubzeme <ubzeme@git.sr.ht> wrote:
>
> I recently bought a Mac with M1 Pro chip, and use QEMU to setup a Linux
> virtual machine.  QEMU crashed when I started a VM with HVF accelerator
> enabled and with the device, bochs-display, added.
>
> After digging into the source code, I found that dirty-tracking in HVF
> did not work properly, which made QEMU crashed. Therefore I made this
> series of patches to fix the problem.

How does this series compare with Alex's patch to enable
hvf dirty tracking for target/arm/hvf ?
https://patchew.org/QEMU/20220203142320.33022-1-agraf@csgraf.de/

thanks
-- PMM

For the dirty-tracking part in my patch series, the major difference 
between this patch and Alex's patch is that the dirty-tracking logic in 
my patch will only mark the page being written dirty instead of marking 
the whole memory slot dirty, and will only restore the write permission 
to the page being written instead of the whole memory slot.

When memory regions overlap, "region_add" in memory listeners may be 
called with structure MemoryRegionSection containing non-zero 
offset_within_region.  This makes the start address of memory slots 
(member "start" in struct hvf_slot) not the same as the start address of 
the memory region. However, the dirty-tracking logics in both 
target/i386/hvf and Alex's patch assume that they are the same.

Currently, there is a bug in the dirty-tracking logic in 
target/i386/hvf. I modified codes in target/i386/hvf to fix the problem.

On the x86 platform, Ubuntu ISO boot menu did not show properly.

The link of the bug is https://bugs.launchpad.net/qemu/+bug/1827005

The modified codes use the new function, "hvf_access_memory",
introduced in this patch series to handle dirty-tracking.

Following is the dirty-tracking logic in original codes in
target/i386/hvf.

     if (write && slot) {
         if (slot->flags & HVF_SLOT_LOG) {
             memory_region_set_dirty(slot->region, gpa - slot->start, 1);
             hv_vm_protect((hv_gpaddr_t)slot->start, (size_t)slot->size,
                           HV_MEMORY_READ | HV_MEMORY_WRITE);
         }
     }

The problem of the above code is:

'memory_region_set_dirty' sets only the page that is being written 
dirty, but hv_vm_protect makes the whole memory slot writable.

Any write to the memory slot excluding the previous written page
and before the next call to "log_sync" in memory listener
will not be correctly tracked.


On Mon, Feb 28, 2022 at 10:11 PM Peter Maydell 
<peter.maydell@linaro.org> wrote:
>
> On Mon, 28 Feb 2022 at 14:07, ~ubzeme <ubzeme@git.sr.ht> wrote:
> >
> > I recently bought a Mac with M1 Pro chip, and use QEMU to setup a Linux
> > virtual machine.  QEMU crashed when I started a VM with HVF accelerator
> > enabled and with the device, bochs-display, added.
> >
> > After digging into the source code, I found that dirty-tracking in HVF
> > did not work properly, which made QEMU crashed. Therefore I made this
> > series of patches to fix the problem.
>
> How does this series compare with Alex's patch to enable
> hvf dirty tracking for target/arm/hvf ?
> https://patchew.org/QEMU/20220203142320.33022-1-agraf@csgraf.de/
>
> thanks
> -- PMM

Sorry, I made a mistake.
The last line in the function, hvf_find_free_slot, introduced in this
commit "hvf: simplify data structures and codes of memory related
functions" should be "return NULL;"

static hvf_slot *hvf_find_free_slot(void)
{
    hvf_slot *slot;
    int x;
    for (x = 0; x < HVF_NUM_SLOTS; x++) {
        slot = &memslots[x];
        if (!slot->size) {
            return slot;
        }
    }

    return NULL;   // <---- This line is changed
}

I will submit a new version of the patch series after I go home.