Re: [PATCH v3 5/9] hvf: fix memory dirty-tracking

[Peter Maydell]

On Wed, 2 Mar 2022 at 13:04, Yan-Jie Wang <ubzeme@gmail.com> wrote:
>
> Dirty-tracking in HVF is not properly implemented.
>
> On Intel Macs, Ubuntu ISO boot menu does not show properly.
>
> On Apple Silicon, using bochs-display may cause the guest crashes because
> the guest may uses load/store instructions on framebuffer which causes
> vmexits and the exception register does not contain enough information
> (ESR_EL2.ISV = 0) for QEMU to emulate the memory operation.
>
> The strategy to log the dirty pages is to write-protect the memory regions
> that are being dirty-tracked.
>
> When the guest is trapped to the host because of memory write, check whether
> the address being written is being dirty-tracked.
>
> If it is being dirty-tracked, restore the write permission of the page and
> mark the accessed page dirty, and resume the guest without increasing
> program counter, and then the same instruction will be execute again.
>
> This patch fixes the problem and make the dirty-tracking work properly.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1827005
> Signed-off-by: Yan-Jie Wang <ubzeme@gmail.com>
> ---
>  accel/hvf/hvf-mem.c      | 62 ++++++++++++++++++++++++++++++++++++----
>  include/sysemu/hvf_int.h | 14 +--------
>  target/arm/hvf/hvf.c     |  5 ++++
>  target/i386/hvf/hvf.c    | 25 ++++------------
>  4 files changed, 68 insertions(+), 38 deletions(-)
>


> +/*
> + * The function is called when the guest is accessing memory causing vmexit.
> + * Check whether the guest can access the memory directly and
> + * also mark the accessed page being written dirty
> + * if the page is being dirty-tracked.
> + *
> + * Return true if the access is within the mapped region,
> + * otherwise return false.
> + */
> +bool hvf_access_memory(hwaddr address, bool write)
> +{
> +    HVFSlot *slot;
> +    hv_return_t ret;
> +    hwaddr start, size;
> +
> +    slot = hvf_find_overlap_slot(address, 1);

What happens if the guest does an unaligned 4 byte access
such that byte 1 is in one slot and bytes 2-4 are in a
different slot, or not covered by a slot at all ?

> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 4d4ddab348..398ad50a29 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -1202,6 +1202,11 @@ int hvf_vcpu_exec(CPUState *cpu)
>              break;
>          }
>
> +        if (iswrite &&
> +            hvf_access_memory(hvf_exit->exception.physical_address, 1)) {

hvf_access_memory() can return true even if it has not changed the
protection flags on the memory, in which case we'll go into an
infinite loop of taking the fault again.

> +            break;
> +        }
> +
>          assert(isv);
>
>          if (iswrite)

thanks
-- PMM