On Thu, 03/26 18:38, Paolo Bonzini wrote:
> Most of the time, not all bitmaps have to be marked as dirty;
> do not do anything if the interesting ones are already dirty.
> Previously, any clean bitmap would have cause all the bitmaps to be
> marked dirty.
>
> In fact, unless running TCG most of the time bitmap operations
> are not done at all because memory_region_is_logging returns
> zero. In this case, skip cpu_physical_memory_range_includes_clean
> as well.
>
> With this patch, cpu_physical_memory_set_dirty_range is called
> unconditionally, so there need not be anymore a separate call to
> xen_modified_memory.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> exec.c | 18 +++++++++---------
> include/exec/ram_addr.h | 19 +++++++++++++------
> 2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 94b7644..83c58ff 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2236,16 +2236,16 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong
> addr,
> static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
> hwaddr length)
> {
> - if (cpu_physical_memory_range_includes_clean(addr, length)) {
> - uint8_t dirty_log_mask = memory_region_is_logging(mr);
> - if (dirty_log_mask & (1 << DIRTY_MEMORY_CODE)) {
> - tb_invalidate_phys_range(addr, addr + length, 0);
> - dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
> - }
> - cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask);
> - } else {
> - xen_modified_memory(addr, length);
> + uint8_t dirty_log_mask = memory_region_is_logging(mr);
> + if (dirty_log_mask &&
> + !cpu_physical_memory_range_includes_clean(addr, length,
> dirty_log_mask)) {
> + dirty_log_mask = 0;
> + }
> + if (dirty_log_mask & (1 << DIRTY_MEMORY_CODE)) {
> + tb_invalidate_phys_range(addr, addr + length, 0);
> + dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
> }
> + cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask);
> }
>
> static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 6c3b74e..b408408 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -90,13 +90,20 @@ static inline bool
> cpu_physical_memory_is_clean(ram_addr_t addr)
> }
>
> static inline bool cpu_physical_memory_range_includes_clean(ram_addr_t start,
> - ram_addr_t
> length)
> + ram_addr_t
> length,
> + uint8_t mask)
> {
> - bool vga = cpu_physical_memory_get_clean(start, length,
> DIRTY_MEMORY_VGA);
> - bool code = cpu_physical_memory_get_clean(start, length,
> DIRTY_MEMORY_CODE);
> - bool migration =
> - cpu_physical_memory_get_clean(start, length, DIRTY_MEMORY_MIGRATION);
> - return vga || code || migration;
> + bool clean = false;
> + if (mask & (1 << DIRTY_MEMORY_VGA)) {
> + clean = cpu_physical_memory_get_clean(start, length,
> DIRTY_MEMORY_VGA);
> + }
> + if (!clean && (mask & (1 << DIRTY_MEMORY_CODE))) {
> + clean = cpu_physical_memory_get_clean(start, length,
> DIRTY_MEMORY_CODE);
> + }
> + if (!clean && (mask & (1 << DIRTY_MEMORY_MIGRATION))) {
> + clean = cpu_physical_memory_get_clean(start, length,
> DIRTY_MEMORY_MIGRATION);
> + }
> + return clean;
> }
Out of curiosity, is it valid that a mask bit is cleared but the corresponding
dirty bit is set?
>
> static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
> --
> 2.3.3
>
>
>
