[RFC PATCH-for-9.0 0/9] hw/xen: Have ARM targets use common xen_memory_listener

[RFC PATCH-for-9.0 0/9] hw/xen: Have ARM targets use common xen_memory_listener

[Philippe Mathieu-Daudé]

Hi,

While looking at Xen target-specific code, I noticed some
generic code used by x86 which is not implemented for ARM.

Maybe ARM machines don't need it, I don't know. But I
wanted to see if I can get this common code target agnostic
and build it once, possibly bringing smth useful to ARM.

The changes don't break CI testing and Avocado local tests.
If this xen_memory_listener feature isn't required for ARM,
I'll send follow up patch that keep this series with common
xen_memory_listener but with a runtime flag to disable.

Patches do the usual "change target-specific API to a
target-agnostic one", in particular using "exec/target_page.h"
at runtime. Then non-x86 code is extracted, then merged to
the generic xen/xen-hvm-common.c.

Thoughts?

Regards,

Phil.

Based-on: <20231114143816.71079-1-philmd@linaro.org>

Philippe Mathieu-Daudé (9):
  hw/xen/hvm: Inline TARGET_PAGE_ALIGN() macro
  hw/xen/hvm: Propagate page_mask to a pair of functions
  hw/xen/hvm: Get target page size at runtime
  hw/xen/hvm: Expose xen_memory_listener declaration
  hw/xen/hvm: Expose xen_read_physmap() prototype
  hw/xen/hvm: Initialize xen_physmap QLIST in xen_read_physmap()
  hw/xen/hvm: Extract common code to xen-hvm-common.c
  hw/xen/hvm: Merge xen-hvm-common.c files
  hw/xen/hvm: Inline xen_arch_set_memory()

 include/hw/xen/xen-hvm-common.h |   5 +-
 hw/arm/xen_arm.c                |  24 --
 hw/i386/xen/xen-hvm.c           | 441 +----------------------------
 hw/xen/xen-hvm-common.c         | 474 +++++++++++++++++++++++++++++++-
 stubs/xen-hw-stub.c             |   4 -
 5 files changed, 468 insertions(+), 480 deletions(-)

-- 
2.41.0

[PATCH-for-9.0 1/9] hw/xen/hvm: Inline TARGET_PAGE_ALIGN() macro

[Philippe Mathieu-Daudé]

Use TARGET_PAGE_SIZE to calculate TARGET_PAGE_ALIGN.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/xen/xen-hvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f1c30d1384..8aa6a1ec3b 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -678,7 +678,7 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
     trace_xen_client_set_memory(start_addr, size, log_dirty);
 
     start_addr &= TARGET_PAGE_MASK;
-    size = TARGET_PAGE_ALIGN(size);
+    size = ROUND_UP(size, TARGET_PAGE_SIZE);
 
     if (add) {
         if (!memory_region_is_rom(section->mr)) {
-- 
2.41.0

Re: [PATCH-for-9.0 1/9] hw/xen/hvm: Inline TARGET_PAGE_ALIGN() macro

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>Use TARGET_PAGE_SIZE to calculate TARGET_PAGE_ALIGN.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/xen/xen-hvm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
>index f1c30d1384..8aa6a1ec3b 100644
>--- a/hw/i386/xen/xen-hvm.c
>+++ b/hw/i386/xen/xen-hvm.c
>@@ -678,7 +678,7 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>     trace_xen_client_set_memory(start_addr, size, log_dirty);
> 
>     start_addr &= TARGET_PAGE_MASK;
>-    size = TARGET_PAGE_ALIGN(size);
>+    size = ROUND_UP(size, TARGET_PAGE_SIZE);
> 
>     if (add) {
>         if (!memory_region_is_rom(section->mr)) {
>-- 
>2.41.0
>
>

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

[PATCH-for-9.0 2/9] hw/xen/hvm: Propagate page_mask to a pair of functions

[Philippe Mathieu-Daudé]

We are going to replace TARGET_PAGE_MASK by a
runtime variable. In order to reduce code duplication,
propagate TARGET_PAGE_MASK to get_physmapping() and
xen_phys_offset_to_gaddr().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/xen/xen-hvm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 8aa6a1ec3b..3b10425986 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -174,11 +174,12 @@ static void xen_ram_init(PCMachineState *pcms,
     }
 }
 
-static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size)
+static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
+                                   int page_mask)
 {
     XenPhysmap *physmap = NULL;
 
-    start_addr &= TARGET_PAGE_MASK;
+    start_addr &= page_mask;
 
     QLIST_FOREACH(physmap, &xen_physmap, list) {
         if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
@@ -188,9 +189,10 @@ static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size)
     return NULL;
 }
 
-static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size)
+static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
+                                       int page_mask)
 {
-    hwaddr addr = phys_offset & TARGET_PAGE_MASK;
+    hwaddr addr = phys_offset & page_mask;
     XenPhysmap *physmap = NULL;
 
     QLIST_FOREACH(physmap, &xen_physmap, list) {
@@ -252,7 +254,7 @@ static int xen_add_to_physmap(XenIOState *state,
     hwaddr phys_offset = memory_region_get_ram_addr(mr);
     const char *mr_name;
 
-    if (get_physmapping(start_addr, size)) {
+    if (get_physmapping(start_addr, size, TARGET_PAGE_MASK)) {
         return 0;
     }
     if (size <= 0) {
@@ -325,7 +327,7 @@ static int xen_remove_from_physmap(XenIOState *state,
     XenPhysmap *physmap = NULL;
     hwaddr phys_offset = 0;
 
-    physmap = get_physmapping(start_addr, size);
+    physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
     if (physmap == NULL) {
         return -1;
     }
@@ -373,7 +375,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
     int rc, i, j;
     const XenPhysmap *physmap = NULL;
 
-    physmap = get_physmapping(start_addr, size);
+    physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
     if (physmap == NULL) {
         /* not handled */
         return;
@@ -633,7 +635,7 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
         int rc;
         ram_addr_t start_pfn, nb_pages;
 
-        start = xen_phys_offset_to_gaddr(start, length);
+        start = xen_phys_offset_to_gaddr(start, length, TARGET_PAGE_MASK);
 
         if (length == 0) {
             length = TARGET_PAGE_SIZE;
-- 
2.41.0

Re: [PATCH-for-9.0 2/9] hw/xen/hvm: Propagate page_mask to a pair of functions

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>We are going to replace TARGET_PAGE_MASK by a
>runtime variable. In order to reduce code duplication,
>propagate TARGET_PAGE_MASK to get_physmapping() and
>xen_phys_offset_to_gaddr().
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/xen/xen-hvm.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
>diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
>index 8aa6a1ec3b..3b10425986 100644
>--- a/hw/i386/xen/xen-hvm.c
>+++ b/hw/i386/xen/xen-hvm.c
>@@ -174,11 +174,12 @@ static void xen_ram_init(PCMachineState *pcms,
>     }
> }
> 
>-static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size)
>+static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
>+                                   int page_mask)
> {
>     XenPhysmap *physmap = NULL;
> 
>-    start_addr &= TARGET_PAGE_MASK;
>+    start_addr &= page_mask;
> 
>     QLIST_FOREACH(physmap, &xen_physmap, list) {
>         if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
>@@ -188,9 +189,10 @@ static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size)
>     return NULL;
> }
> 
>-static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size)
>+static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
>+                                       int page_mask)
> {
>-    hwaddr addr = phys_offset & TARGET_PAGE_MASK;
>+    hwaddr addr = phys_offset & page_mask;
>     XenPhysmap *physmap = NULL;
> 
>     QLIST_FOREACH(physmap, &xen_physmap, list) {
>@@ -252,7 +254,7 @@ static int xen_add_to_physmap(XenIOState *state,
>     hwaddr phys_offset = memory_region_get_ram_addr(mr);
>     const char *mr_name;
> 
>-    if (get_physmapping(start_addr, size)) {
>+    if (get_physmapping(start_addr, size, TARGET_PAGE_MASK)) {
>         return 0;
>     }
>     if (size <= 0) {
>@@ -325,7 +327,7 @@ static int xen_remove_from_physmap(XenIOState *state,
>     XenPhysmap *physmap = NULL;
>     hwaddr phys_offset = 0;
> 
>-    physmap = get_physmapping(start_addr, size);
>+    physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
>     if (physmap == NULL) {
>         return -1;
>     }
>@@ -373,7 +375,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>     int rc, i, j;
>     const XenPhysmap *physmap = NULL;
> 
>-    physmap = get_physmapping(start_addr, size);
>+    physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
>     if (physmap == NULL) {
>         /* not handled */
>         return;
>@@ -633,7 +635,7 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
>         int rc;
>         ram_addr_t start_pfn, nb_pages;
> 
>-        start = xen_phys_offset_to_gaddr(start, length);
>+        start = xen_phys_offset_to_gaddr(start, length, TARGET_PAGE_MASK);
> 
>         if (length == 0) {
>             length = TARGET_PAGE_SIZE;
>-- 
>2.41.0
>
>


Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

[PATCH-for-9.0 3/9] hw/xen/hvm: Get target page size at runtime

[Philippe Mathieu-Daudé]

In order to build this file once for all targets, replace:

  TARGET_PAGE_BITS -> qemu_target_page_bits()
  TARGET_PAGE_SIZE -> qemu_target_page_size()
  TARGET_PAGE_MASK -> -qemu_target_page_size()

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/xen/xen-hvm.c | 62 +++++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 3b10425986..b64204ea94 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -22,6 +22,7 @@
 
 #include "hw/xen/xen-hvm-common.h"
 #include <xen/hvm/e820.h>
+#include "exec/target_page.h"
 #include "cpu.h"
 
 static MemoryRegion ram_640k, ram_lo, ram_hi;
@@ -247,6 +248,9 @@ static int xen_add_to_physmap(XenIOState *state,
                               MemoryRegion *mr,
                               hwaddr offset_within_region)
 {
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
     unsigned long nr_pages;
     int rc = 0;
     XenPhysmap *physmap = NULL;
@@ -254,7 +258,7 @@ static int xen_add_to_physmap(XenIOState *state,
     hwaddr phys_offset = memory_region_get_ram_addr(mr);
     const char *mr_name;
 
-    if (get_physmapping(start_addr, size, TARGET_PAGE_MASK)) {
+    if (get_physmapping(start_addr, size, page_mask)) {
         return 0;
     }
     if (size <= 0) {
@@ -294,9 +298,9 @@ go_physmap:
         return 0;
     }
 
-    pfn = phys_offset >> TARGET_PAGE_BITS;
-    start_gpfn = start_addr >> TARGET_PAGE_BITS;
-    nr_pages = size >> TARGET_PAGE_BITS;
+    pfn = phys_offset >> target_page_bits;
+    start_gpfn = start_addr >> target_page_bits;
+    nr_pages = size >> target_page_bits;
     rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
                                         start_gpfn);
     if (rc) {
@@ -310,8 +314,8 @@ go_physmap:
     }
 
     rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
-                                   start_addr >> TARGET_PAGE_BITS,
-                                   (start_addr + size - 1) >> TARGET_PAGE_BITS,
+                                   start_addr >> target_page_bits,
+                                   (start_addr + size - 1) >> target_page_bits,
                                    XEN_DOMCTL_MEM_CACHEATTR_WB);
     if (rc) {
         error_report("pin_memory_cacheattr failed: %s", strerror(errno));
@@ -323,11 +327,14 @@ static int xen_remove_from_physmap(XenIOState *state,
                                    hwaddr start_addr,
                                    ram_addr_t size)
 {
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
     int rc = 0;
     XenPhysmap *physmap = NULL;
     hwaddr phys_offset = 0;
 
-    physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
+    physmap = get_physmapping(start_addr, size, page_mask);
     if (physmap == NULL) {
         return -1;
     }
@@ -338,9 +345,9 @@ static int xen_remove_from_physmap(XenIOState *state,
     DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
             "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
 
-    size >>= TARGET_PAGE_BITS;
-    start_addr >>= TARGET_PAGE_BITS;
-    phys_offset >>= TARGET_PAGE_BITS;
+    size >>= target_page_bits;
+    start_addr >>= target_page_bits;
+    phys_offset >>= target_page_bits;
     rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
                                         phys_offset);
     if (rc) {
@@ -369,13 +376,16 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
                                   hwaddr start_addr,
                                   ram_addr_t size)
 {
-    hwaddr npages = size >> TARGET_PAGE_BITS;
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    hwaddr npages = size >> target_page_bits;
     const int width = sizeof(unsigned long) * 8;
     size_t bitmap_size = DIV_ROUND_UP(npages, width);
     int rc, i, j;
     const XenPhysmap *physmap = NULL;
 
-    physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
+    physmap = get_physmapping(start_addr, size, page_mask);
     if (physmap == NULL) {
         /* not handled */
         return;
@@ -389,7 +399,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
         return;
     }
 
-    rc = xen_track_dirty_vram(xen_domid, start_addr >> TARGET_PAGE_BITS,
+    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
                               npages, dirty_bitmap);
     if (rc < 0) {
 #ifndef ENODATA
@@ -410,8 +420,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
             j = ctzl(map);
             map &= ~(1ul << j);
             memory_region_set_dirty(framebuffer,
-                                    (i * width + j) * TARGET_PAGE_SIZE,
-                                    TARGET_PAGE_SIZE);
+                                    (i * width + j) * page_size, page_size);
         };
     }
 }
@@ -631,17 +640,21 @@ void xen_register_framebuffer(MemoryRegion *mr)
 
 void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
 {
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+
     if (unlikely(xen_in_migration)) {
         int rc;
         ram_addr_t start_pfn, nb_pages;
 
-        start = xen_phys_offset_to_gaddr(start, length, TARGET_PAGE_MASK);
+        start = xen_phys_offset_to_gaddr(start, length, page_mask);
 
         if (length == 0) {
-            length = TARGET_PAGE_SIZE;
+            length = page_size;
         }
-        start_pfn = start >> TARGET_PAGE_BITS;
-        nb_pages = ((start + length + TARGET_PAGE_SIZE - 1) >> TARGET_PAGE_BITS)
+        start_pfn = start >> target_page_bits;
+        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
             - start_pfn;
         rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
         if (rc) {
@@ -664,6 +677,9 @@ void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
 void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
                          bool add)
 {
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
     hwaddr start_addr = section->offset_within_address_space;
     ram_addr_t size = int128_get64(section->size);
     bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
@@ -679,8 +695,8 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
 
     trace_xen_client_set_memory(start_addr, size, log_dirty);
 
-    start_addr &= TARGET_PAGE_MASK;
-    size = ROUND_UP(size, TARGET_PAGE_SIZE);
+    start_addr &= page_mask;
+    size = ROUND_UP(size, page_size);
 
     if (add) {
         if (!memory_region_is_rom(section->mr)) {
@@ -689,8 +705,8 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
         } else {
             mem_type = HVMMEM_ram_ro;
             if (xen_set_mem_type(xen_domid, mem_type,
-                                 start_addr >> TARGET_PAGE_BITS,
-                                 size >> TARGET_PAGE_BITS)) {
+                                 start_addr >> target_page_bits,
+                                 size >> target_page_bits)) {
                 DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
                         start_addr);
             }
-- 
2.41.0

Re: [PATCH-for-9.0 3/9] hw/xen/hvm: Get target page size at runtime

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>In order to build this file once for all targets, replace:
>
>  TARGET_PAGE_BITS -> qemu_target_page_bits()
>  TARGET_PAGE_SIZE -> qemu_target_page_size()
>  TARGET_PAGE_MASK -> -qemu_target_page_size()
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/xen/xen-hvm.c | 62 +++++++++++++++++++++++++++----------------
> 1 file changed, 39 insertions(+), 23 deletions(-)
>
>diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
>index 3b10425986..b64204ea94 100644
>--- a/hw/i386/xen/xen-hvm.c
>+++ b/hw/i386/xen/xen-hvm.c
>@@ -22,6 +22,7 @@
> 
> #include "hw/xen/xen-hvm-common.h"
> #include <xen/hvm/e820.h>
>+#include "exec/target_page.h"
> #include "cpu.h"
> 
> static MemoryRegion ram_640k, ram_lo, ram_hi;
>@@ -247,6 +248,9 @@ static int xen_add_to_physmap(XenIOState *state,
>                               MemoryRegion *mr,
>                               hwaddr offset_within_region)
> {
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>     unsigned long nr_pages;
>     int rc = 0;
>     XenPhysmap *physmap = NULL;
>@@ -254,7 +258,7 @@ static int xen_add_to_physmap(XenIOState *state,
>     hwaddr phys_offset = memory_region_get_ram_addr(mr);
>     const char *mr_name;
> 
>-    if (get_physmapping(start_addr, size, TARGET_PAGE_MASK)) {
>+    if (get_physmapping(start_addr, size, page_mask)) {
>         return 0;
>     }
>     if (size <= 0) {
>@@ -294,9 +298,9 @@ go_physmap:
>         return 0;
>     }
> 
>-    pfn = phys_offset >> TARGET_PAGE_BITS;
>-    start_gpfn = start_addr >> TARGET_PAGE_BITS;
>-    nr_pages = size >> TARGET_PAGE_BITS;
>+    pfn = phys_offset >> target_page_bits;
>+    start_gpfn = start_addr >> target_page_bits;
>+    nr_pages = size >> target_page_bits;
>     rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
>                                         start_gpfn);
>     if (rc) {
>@@ -310,8 +314,8 @@ go_physmap:
>     }
> 
>     rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
>-                                   start_addr >> TARGET_PAGE_BITS,
>-                                   (start_addr + size - 1) >> TARGET_PAGE_BITS,
>+                                   start_addr >> target_page_bits,
>+                                   (start_addr + size - 1) >> target_page_bits,
>                                    XEN_DOMCTL_MEM_CACHEATTR_WB);
>     if (rc) {
>         error_report("pin_memory_cacheattr failed: %s", strerror(errno));
>@@ -323,11 +327,14 @@ static int xen_remove_from_physmap(XenIOState *state,
>                                    hwaddr start_addr,
>                                    ram_addr_t size)
> {
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>     int rc = 0;
>     XenPhysmap *physmap = NULL;
>     hwaddr phys_offset = 0;
> 
>-    physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
>+    physmap = get_physmapping(start_addr, size, page_mask);
>     if (physmap == NULL) {
>         return -1;
>     }
>@@ -338,9 +345,9 @@ static int xen_remove_from_physmap(XenIOState *state,
>     DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
>             "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
> 
>-    size >>= TARGET_PAGE_BITS;
>-    start_addr >>= TARGET_PAGE_BITS;
>-    phys_offset >>= TARGET_PAGE_BITS;
>+    size >>= target_page_bits;
>+    start_addr >>= target_page_bits;
>+    phys_offset >>= target_page_bits;
>     rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
>                                         phys_offset);
>     if (rc) {
>@@ -369,13 +376,16 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>                                   hwaddr start_addr,
>                                   ram_addr_t size)
> {
>-    hwaddr npages = size >> TARGET_PAGE_BITS;
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    hwaddr npages = size >> target_page_bits;
>     const int width = sizeof(unsigned long) * 8;
>     size_t bitmap_size = DIV_ROUND_UP(npages, width);
>     int rc, i, j;
>     const XenPhysmap *physmap = NULL;
> 
>-    physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
>+    physmap = get_physmapping(start_addr, size, page_mask);
>     if (physmap == NULL) {
>         /* not handled */
>         return;
>@@ -389,7 +399,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>         return;
>     }
> 
>-    rc = xen_track_dirty_vram(xen_domid, start_addr >> TARGET_PAGE_BITS,
>+    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
>                               npages, dirty_bitmap);
>     if (rc < 0) {
> #ifndef ENODATA
>@@ -410,8 +420,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>             j = ctzl(map);
>             map &= ~(1ul << j);
>             memory_region_set_dirty(framebuffer,
>-                                    (i * width + j) * TARGET_PAGE_SIZE,
>-                                    TARGET_PAGE_SIZE);
>+                                    (i * width + j) * page_size, page_size);
>         };
>     }
> }
>@@ -631,17 +640,21 @@ void xen_register_framebuffer(MemoryRegion *mr)
> 
> void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
> {
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+
>     if (unlikely(xen_in_migration)) {
>         int rc;
>         ram_addr_t start_pfn, nb_pages;
> 
>-        start = xen_phys_offset_to_gaddr(start, length, TARGET_PAGE_MASK);
>+        start = xen_phys_offset_to_gaddr(start, length, page_mask);
> 
>         if (length == 0) {
>-            length = TARGET_PAGE_SIZE;
>+            length = page_size;
>         }
>-        start_pfn = start >> TARGET_PAGE_BITS;
>-        nb_pages = ((start + length + TARGET_PAGE_SIZE - 1) >> TARGET_PAGE_BITS)
>+        start_pfn = start >> target_page_bits;
>+        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
>             - start_pfn;
>         rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
>         if (rc) {
>@@ -664,6 +677,9 @@ void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
> void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>                          bool add)
> {
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>     hwaddr start_addr = section->offset_within_address_space;
>     ram_addr_t size = int128_get64(section->size);
>     bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
>@@ -679,8 +695,8 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
> 
>     trace_xen_client_set_memory(start_addr, size, log_dirty);
> 
>-    start_addr &= TARGET_PAGE_MASK;
>-    size = ROUND_UP(size, TARGET_PAGE_SIZE);
>+    start_addr &= page_mask;
>+    size = ROUND_UP(size, page_size);
> 
>     if (add) {
>         if (!memory_region_is_rom(section->mr)) {
>@@ -689,8 +705,8 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>         } else {
>             mem_type = HVMMEM_ram_ro;
>             if (xen_set_mem_type(xen_domid, mem_type,
>-                                 start_addr >> TARGET_PAGE_BITS,
>-                                 size >> TARGET_PAGE_BITS)) {
>+                                 start_addr >> target_page_bits,
>+                                 size >> target_page_bits)) {
>                 DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
>                         start_addr);
>             }
>-- 
>2.41.0
>
>


Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

[PATCH-for-9.0 4/9] hw/xen/hvm: Expose xen_memory_listener declaration

[Philippe Mathieu-Daudé]

There can only be a single xen_memory_listener definition
in a qemu-system binary.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/xen/xen-hvm-common.h | 1 +
 hw/arm/xen_arm.c                | 2 +-
 hw/i386/xen/xen-hvm.c           | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 83ed16f425..0fed15ed04 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -18,6 +18,7 @@
 extern MemoryRegion xen_memory;
 extern MemoryListener xen_io_listener;
 extern DeviceListener xen_device_listener;
+extern const MemoryListener xen_memory_listener;
 
 //#define DEBUG_XEN_HVM
 
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index b478d74ea0..39dcd74d07 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -38,7 +38,7 @@
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
 
-static const MemoryListener xen_memory_listener = {
+const MemoryListener xen_memory_listener = {
     .region_add = xen_region_add,
     .region_del = xen_region_del,
     .log_start = NULL,
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index b64204ea94..a65a96f0de 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -469,7 +469,7 @@ static void xen_log_global_stop(MemoryListener *listener)
     xen_in_migration = false;
 }
 
-static const MemoryListener xen_memory_listener = {
+const MemoryListener xen_memory_listener = {
     .name = "xen-memory",
     .region_add = xen_region_add,
     .region_del = xen_region_del,
-- 
2.41.0

Re: [PATCH-for-9.0 4/9] hw/xen/hvm: Expose xen_memory_listener declaration

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>There can only be a single xen_memory_listener definition
>in a qemu-system binary.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> include/hw/xen/xen-hvm-common.h | 1 +
> hw/arm/xen_arm.c                | 2 +-
> hw/i386/xen/xen-hvm.c           | 2 +-
> 3 files changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
>index 83ed16f425..0fed15ed04 100644
>--- a/include/hw/xen/xen-hvm-common.h
>+++ b/include/hw/xen/xen-hvm-common.h
>@@ -18,6 +18,7 @@
> extern MemoryRegion xen_memory;
> extern MemoryListener xen_io_listener;
> extern DeviceListener xen_device_listener;
>+extern const MemoryListener xen_memory_listener;
> 
> //#define DEBUG_XEN_HVM
> 
>diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
>index b478d74ea0..39dcd74d07 100644
>--- a/hw/arm/xen_arm.c
>+++ b/hw/arm/xen_arm.c
>@@ -38,7 +38,7 @@
> #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
> OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
> 
>-static const MemoryListener xen_memory_listener = {
>+const MemoryListener xen_memory_listener = {
>     .region_add = xen_region_add,
>     .region_del = xen_region_del,
>     .log_start = NULL,
>diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
>index b64204ea94..a65a96f0de 100644
>--- a/hw/i386/xen/xen-hvm.c
>+++ b/hw/i386/xen/xen-hvm.c
>@@ -469,7 +469,7 @@ static void xen_log_global_stop(MemoryListener *listener)
>     xen_in_migration = false;
> }
> 
>-static const MemoryListener xen_memory_listener = {
>+const MemoryListener xen_memory_listener = {
>     .name = "xen-memory",
>     .region_add = xen_region_add,
>     .region_del = xen_region_del,
>-- 
>2.41.0
>
>


Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

[PATCH-for-9.0 5/9] hw/xen/hvm: Expose xen_read_physmap() prototype

[Philippe Mathieu-Daudé]

In a pair of commit we are going to call xen_read_physmap()
out of hw/i386/xen/xen-hvm.c.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/xen/xen-hvm-common.h | 1 +
 hw/i386/xen/xen-hvm.c           | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 0fed15ed04..536712dc83 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -97,6 +97,7 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
 
 void cpu_ioreq_pio(ioreq_t *req);
 
+void xen_read_physmap(XenIOState *state);
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
 void xen_arch_set_memory(XenIOState *state,
                          MemoryRegionSection *section,
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index a65a96f0de..789779d02c 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -525,7 +525,7 @@ static void handle_vmport_ioreq(XenIOState *state, ioreq_t *req)
 }
 
 #ifdef XEN_COMPAT_PHYSMAP
-static void xen_read_physmap(XenIOState *state)
+void xen_read_physmap(XenIOState *state)
 {
     XenPhysmap *physmap = NULL;
     unsigned int len, num, i;
@@ -573,7 +573,7 @@ static void xen_read_physmap(XenIOState *state)
     free(entries);
 }
 #else
-static void xen_read_physmap(XenIOState *state)
+void xen_read_physmap(XenIOState *state)
 {
 }
 #endif
-- 
2.41.0

Re: [PATCH-for-9.0 5/9] hw/xen/hvm: Expose xen_read_physmap() prototype

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>In a pair of commit we are going to call xen_read_physmap()
>out of hw/i386/xen/xen-hvm.c.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> include/hw/xen/xen-hvm-common.h | 1 +
> hw/i386/xen/xen-hvm.c           | 4 ++--
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
>index 0fed15ed04..536712dc83 100644
>--- a/include/hw/xen/xen-hvm-common.h
>+++ b/include/hw/xen/xen-hvm-common.h
>@@ -97,6 +97,7 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
> 
> void cpu_ioreq_pio(ioreq_t *req);
> 
>+void xen_read_physmap(XenIOState *state);
> void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
> void xen_arch_set_memory(XenIOState *state,
>                          MemoryRegionSection *section,
>diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
>index a65a96f0de..789779d02c 100644
>--- a/hw/i386/xen/xen-hvm.c
>+++ b/hw/i386/xen/xen-hvm.c
>@@ -525,7 +525,7 @@ static void handle_vmport_ioreq(XenIOState *state, ioreq_t *req)
> }
> 
> #ifdef XEN_COMPAT_PHYSMAP
>-static void xen_read_physmap(XenIOState *state)
>+void xen_read_physmap(XenIOState *state)
> {
>     XenPhysmap *physmap = NULL;
>     unsigned int len, num, i;
>@@ -573,7 +573,7 @@ static void xen_read_physmap(XenIOState *state)
>     free(entries);
> }
> #else
>-static void xen_read_physmap(XenIOState *state)
>+void xen_read_physmap(XenIOState *state)
> {
> }
> #endif
>-- 
>2.41.0
>
>

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

[RFC PATCH-for-9.0 6/9] hw/xen/hvm: Initialize xen_physmap QLIST in xen_read_physmap()

[Philippe Mathieu-Daudé]

xen_read_physmap() is the first function requiring
xen_physmap QLIST being initialized. Move the init
call there.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/xen/xen-hvm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 789779d02c..3b9c31c1c8 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -532,6 +532,8 @@ void xen_read_physmap(XenIOState *state)
     char path[80], *value = NULL;
     char **entries = NULL;
 
+    QLIST_INIT(&xen_physmap);
+
     snprintf(path, sizeof(path),
             "/local/domain/0/device-model/%d/physmap", xen_domid);
     entries = xs_directory(state->xenstore, 0, path, &num);
@@ -575,6 +577,7 @@ void xen_read_physmap(XenIOState *state)
 #else
 void xen_read_physmap(XenIOState *state)
 {
+    QLIST_INIT(&xen_physmap);
 }
 #endif
 
@@ -595,7 +598,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory)
 
     xen_register_ioreq(state, max_cpus, &xen_memory_listener);
 
-    QLIST_INIT(&xen_physmap);
     xen_read_physmap(state);
 
     suspend.notify = xen_suspend_notifier;
-- 
2.41.0

Re: [RFC PATCH-for-9.0 6/9] hw/xen/hvm: Initialize xen_physmap QLIST in xen_read_physmap()

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>xen_read_physmap() is the first function requiring
>xen_physmap QLIST being initialized. Move the init
>call there.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/xen/xen-hvm.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
>index 789779d02c..3b9c31c1c8 100644
>--- a/hw/i386/xen/xen-hvm.c
>+++ b/hw/i386/xen/xen-hvm.c
>@@ -532,6 +532,8 @@ void xen_read_physmap(XenIOState *state)
>     char path[80], *value = NULL;
>     char **entries = NULL;
> 
>+    QLIST_INIT(&xen_physmap);
>+
>     snprintf(path, sizeof(path),
>             "/local/domain/0/device-model/%d/physmap", xen_domid);
>     entries = xs_directory(state->xenstore, 0, path, &num);
>@@ -575,6 +577,7 @@ void xen_read_physmap(XenIOState *state)
> #else
> void xen_read_physmap(XenIOState *state)
> {
>+    QLIST_INIT(&xen_physmap);
> }
> #endif
> 
>@@ -595,7 +598,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory)
> 
>     xen_register_ioreq(state, max_cpus, &xen_memory_listener);
> 
>-    QLIST_INIT(&xen_physmap);
>     xen_read_physmap(state);
> 
>     suspend.notify = xen_suspend_notifier;
>-- 
>2.41.0
>
>

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

[PATCH-for-9.0 7/9] hw/xen/hvm: Extract common code to xen-hvm-common.c

[Philippe Mathieu-Daudé]

Extract non-x86 specific code out of xen-hvm.c,
to xen-hvm-common.c. For now this new file is
only build for x86 targets.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/xen/xen-hvm-common.c | 473 +++++++++++++++++++++++++++++++++++
 hw/i386/xen/xen-hvm.c        | 459 ---------------------------------
 hw/i386/xen/meson.build      |   1 +
 3 files changed, 474 insertions(+), 459 deletions(-)
 create mode 100644 hw/i386/xen/xen-hvm-common.c

diff --git a/hw/i386/xen/xen-hvm-common.c b/hw/i386/xen/xen-hvm-common.c
new file mode 100644
index 0000000000..e8ef0e0c94
--- /dev/null
+++ b/hw/i386/xen/xen-hvm-common.c
@@ -0,0 +1,473 @@
+/*
+ * Copyright (C) 2010       Citrix Ltd.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/range.h"
+#include "qapi/qapi-commands-migration.h"
+#include "exec/target_page.h"
+#include "hw/xen/xen-hvm-common.h"
+#include "trace.h"
+
+static MemoryRegion *framebuffer;
+static bool xen_in_migration;
+
+static QLIST_HEAD(, XenPhysmap) xen_physmap;
+static const XenPhysmap *log_for_dirtybit;
+/* Buffer used by xen_sync_dirty_bitmap */
+static unsigned long *dirty_bitmap;
+
+static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
+                                   int page_mask)
+{
+    XenPhysmap *physmap = NULL;
+
+    start_addr &= -page_mask;
+
+    QLIST_FOREACH(physmap, &xen_physmap, list) {
+        if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
+            return physmap;
+        }
+    }
+    return NULL;
+}
+
+static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
+                                       int page_mask)
+{
+    hwaddr addr = phys_offset & -page_mask;
+    XenPhysmap *physmap = NULL;
+
+    QLIST_FOREACH(physmap, &xen_physmap, list) {
+        if (range_covers_byte(physmap->phys_offset, physmap->size, addr)) {
+            return physmap->start_addr + (phys_offset - physmap->phys_offset);
+        }
+    }
+
+    return phys_offset;
+}
+
+#ifdef XEN_COMPAT_PHYSMAP
+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
+{
+    char path[80], value[17];
+
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
+            xen_domid, (uint64_t)physmap->phys_offset);
+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+        return -1;
+    }
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
+            xen_domid, (uint64_t)physmap->phys_offset);
+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+        return -1;
+    }
+    if (physmap->name) {
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
+                xen_domid, (uint64_t)physmap->phys_offset);
+        if (!xs_write(state->xenstore, 0, path,
+                      physmap->name, strlen(physmap->name))) {
+            return -1;
+        }
+    }
+    return 0;
+}
+#else
+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
+{
+    return 0;
+}
+#endif
+
+static int xen_add_to_physmap(XenIOState *state,
+                              hwaddr start_addr,
+                              ram_addr_t size,
+                              MemoryRegion *mr,
+                              hwaddr offset_within_region)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    unsigned long nr_pages;
+    int rc = 0;
+    XenPhysmap *physmap = NULL;
+    hwaddr pfn, start_gpfn;
+    hwaddr phys_offset = memory_region_get_ram_addr(mr);
+    const char *mr_name;
+
+    if (get_physmapping(start_addr, size, page_mask)) {
+        return 0;
+    }
+    if (size <= 0) {
+        return -1;
+    }
+
+    /* Xen can only handle a single dirty log region for now and we want
+     * the linear framebuffer to be that region.
+     * Avoid tracking any regions that is not videoram and avoid tracking
+     * the legacy vga region. */
+    if (mr == framebuffer && start_addr > 0xbffff) {
+        goto go_physmap;
+    }
+    return -1;
+
+go_physmap:
+    DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
+            start_addr, start_addr + size);
+
+    mr_name = memory_region_name(mr);
+
+    physmap = g_new(XenPhysmap, 1);
+
+    physmap->start_addr = start_addr;
+    physmap->size = size;
+    physmap->name = mr_name;
+    physmap->phys_offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
+
+    if (runstate_check(RUN_STATE_INMIGRATE)) {
+        /* Now when we have a physmap entry we can replace a dummy mapping with
+         * a real one of guest foreign memory. */
+        uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
+        assert(p && p == memory_region_get_ram_ptr(mr));
+
+        return 0;
+    }
+
+    pfn = phys_offset >> target_page_bits;
+    start_gpfn = start_addr >> target_page_bits;
+    nr_pages = size >> target_page_bits;
+    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
+                                        start_gpfn);
+    if (rc) {
+        int saved_errno = errno;
+
+        error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
+                     " to GFN %"HWADDR_PRIx" failed: %s",
+                     nr_pages, pfn, start_gpfn, strerror(saved_errno));
+        errno = saved_errno;
+        return -1;
+    }
+
+    rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
+                                   start_addr >> target_page_bits,
+                                   (start_addr + size - 1) >> target_page_bits,
+                                   XEN_DOMCTL_MEM_CACHEATTR_WB);
+    if (rc) {
+        error_report("pin_memory_cacheattr failed: %s", strerror(errno));
+    }
+    return xen_save_physmap(state, physmap);
+}
+
+static int xen_remove_from_physmap(XenIOState *state,
+                                   hwaddr start_addr,
+                                   ram_addr_t size)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    int rc = 0;
+    XenPhysmap *physmap = NULL;
+    hwaddr phys_offset = 0;
+
+    physmap = get_physmapping(start_addr, size, page_mask);
+    if (physmap == NULL) {
+        return -1;
+    }
+
+    phys_offset = physmap->phys_offset;
+    size = physmap->size;
+
+    DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
+            "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
+
+    size >>= target_page_bits;
+    start_addr >>= target_page_bits;
+    phys_offset >>= target_page_bits;
+    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
+                                        phys_offset);
+    if (rc) {
+        int saved_errno = errno;
+
+        error_report("relocate_memory "RAM_ADDR_FMT" pages"
+                     " from GFN %"HWADDR_PRIx
+                     " to GFN %"HWADDR_PRIx" failed: %s",
+                     size, start_addr, phys_offset, strerror(saved_errno));
+        errno = saved_errno;
+        return -1;
+    }
+
+    QLIST_REMOVE(physmap, list);
+    if (log_for_dirtybit == physmap) {
+        log_for_dirtybit = NULL;
+        g_free(dirty_bitmap);
+        dirty_bitmap = NULL;
+    }
+    g_free(physmap);
+
+    return 0;
+}
+
+static void xen_sync_dirty_bitmap(XenIOState *state,
+                                  hwaddr start_addr,
+                                  ram_addr_t size)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    hwaddr npages = size >> target_page_bits;
+    const int width = sizeof(unsigned long) * 8;
+    size_t bitmap_size = DIV_ROUND_UP(npages, width);
+    int rc, i, j;
+    const XenPhysmap *physmap = NULL;
+
+    physmap = get_physmapping(start_addr, size, page_mask);
+    if (physmap == NULL) {
+        /* not handled */
+        return;
+    }
+
+    if (log_for_dirtybit == NULL) {
+        log_for_dirtybit = physmap;
+        dirty_bitmap = g_new(unsigned long, bitmap_size);
+    } else if (log_for_dirtybit != physmap) {
+        /* Only one range for dirty bitmap can be tracked. */
+        return;
+    }
+
+    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
+                              npages, dirty_bitmap);
+    if (rc < 0) {
+#ifndef ENODATA
+#define ENODATA  ENOENT
+#endif
+        if (errno == ENODATA) {
+            memory_region_set_dirty(framebuffer, 0, size);
+            DPRINTF("xen: track_dirty_vram failed (0x" HWADDR_FMT_plx
+                    ", 0x" HWADDR_FMT_plx "): %s\n",
+                    start_addr, start_addr + size, strerror(errno));
+        }
+        return;
+    }
+
+    for (i = 0; i < bitmap_size; i++) {
+        unsigned long map = dirty_bitmap[i];
+        while (map != 0) {
+            j = ctzl(map);
+            map &= ~(1ul << j);
+            memory_region_set_dirty(framebuffer,
+                                    (i * width + j) * page_size,
+                                    page_size);
+        };
+    }
+}
+
+static void xen_log_start(MemoryListener *listener,
+                          MemoryRegionSection *section,
+                          int old, int new)
+{
+    XenIOState *state = container_of(listener, XenIOState, memory_listener);
+
+    if (new & ~old & (1 << DIRTY_MEMORY_VGA)) {
+        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
+                              int128_get64(section->size));
+    }
+}
+
+static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
+                         int old, int new)
+{
+    if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
+        log_for_dirtybit = NULL;
+        g_free(dirty_bitmap);
+        dirty_bitmap = NULL;
+        /* Disable dirty bit tracking */
+        xen_track_dirty_vram(xen_domid, 0, 0, NULL);
+    }
+}
+
+static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
+{
+    XenIOState *state = container_of(listener, XenIOState, memory_listener);
+
+    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
+                          int128_get64(section->size));
+}
+
+static void xen_log_global_start(MemoryListener *listener)
+{
+    if (xen_enabled()) {
+        xen_in_migration = true;
+    }
+}
+
+static void xen_log_global_stop(MemoryListener *listener)
+{
+    xen_in_migration = false;
+}
+
+const MemoryListener xen_memory_listener = {
+    .name = "xen-memory",
+    .region_add = xen_region_add,
+    .region_del = xen_region_del,
+    .log_start = xen_log_start,
+    .log_stop = xen_log_stop,
+    .log_sync = xen_log_sync,
+    .log_global_start = xen_log_global_start,
+    .log_global_stop = xen_log_global_stop,
+    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
+};
+
+#ifdef XEN_COMPAT_PHYSMAP
+void xen_read_physmap(XenIOState *state)
+{
+    XenPhysmap *physmap = NULL;
+    unsigned int len, num, i;
+    char path[80], *value = NULL;
+    char **entries = NULL;
+
+    QLIST_INIT(&xen_physmap);
+
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap", xen_domid);
+    entries = xs_directory(state->xenstore, 0, path, &num);
+    if (entries == NULL)
+        return;
+
+    for (i = 0; i < num; i++) {
+        physmap = g_new(XenPhysmap, 1);
+        physmap->phys_offset = strtoull(entries[i], NULL, 16);
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
+                xen_domid, entries[i]);
+        value = xs_read(state->xenstore, 0, path, &len);
+        if (value == NULL) {
+            g_free(physmap);
+            continue;
+        }
+        physmap->start_addr = strtoull(value, NULL, 16);
+        free(value);
+
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%s/size",
+                xen_domid, entries[i]);
+        value = xs_read(state->xenstore, 0, path, &len);
+        if (value == NULL) {
+            g_free(physmap);
+            continue;
+        }
+        physmap->size = strtoull(value, NULL, 16);
+        free(value);
+
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%s/name",
+                xen_domid, entries[i]);
+        physmap->name = xs_read(state->xenstore, 0, path, &len);
+
+        QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
+    }
+    free(entries);
+}
+#else
+void xen_read_physmap(XenIOState *state)
+{
+    QLIST_INIT(&xen_physmap);
+}
+#endif
+
+void xen_register_framebuffer(MemoryRegion *mr)
+{
+    framebuffer = mr;
+}
+
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+
+    if (unlikely(xen_in_migration)) {
+        int rc;
+        ram_addr_t start_pfn, nb_pages;
+
+        start = xen_phys_offset_to_gaddr(start, length, page_mask);
+
+        if (length == 0) {
+            length = page_size;
+        }
+        start_pfn = start >> target_page_bits;
+        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
+            - start_pfn;
+        rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
+        if (rc) {
+            fprintf(stderr,
+                    "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n",
+                    __func__, start, nb_pages, errno, strerror(errno));
+        }
+    }
+}
+
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+    if (enable) {
+        memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
+    } else {
+        memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
+    }
+}
+
+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
+                         bool add)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    hwaddr start_addr = section->offset_within_address_space;
+    ram_addr_t size = int128_get64(section->size);
+    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
+    hvmmem_type_t mem_type;
+
+    if (!memory_region_is_ram(section->mr)) {
+        return;
+    }
+
+    if (log_dirty != add) {
+        return;
+    }
+
+    trace_xen_client_set_memory(start_addr, size, log_dirty);
+
+    start_addr &= page_mask;
+    size = ROUND_UP(size, page_size);
+
+    if (add) {
+        if (!memory_region_is_rom(section->mr)) {
+            xen_add_to_physmap(state, start_addr, size,
+                               section->mr, section->offset_within_region);
+        } else {
+            mem_type = HVMMEM_ram_ro;
+            if (xen_set_mem_type(xen_domid, mem_type,
+                                 start_addr >> target_page_bits,
+                                 size >> target_page_bits)) {
+                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
+                        start_addr);
+            }
+        }
+    } else {
+        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
+            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
+        }
+    }
+}
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 3b9c31c1c8..5657693e1b 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -9,16 +9,11 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/units.h"
-#include "qapi/error.h"
-#include "qapi/qapi-commands-migration.h"
-#include "trace.h"
 
 #include "hw/i386/pc.h"
 #include "hw/irq.h"
 #include "hw/i386/apic-msidef.h"
 #include "hw/xen/xen-x86.h"
-#include "qemu/range.h"
 
 #include "hw/xen/xen-hvm-common.h"
 #include <xen/hvm/e820.h>
@@ -26,8 +21,6 @@
 #include "cpu.h"
 
 static MemoryRegion ram_640k, ram_lo, ram_hi;
-static MemoryRegion *framebuffer;
-static bool xen_in_migration;
 
 /* Compatibility with older version */
 
@@ -56,10 +49,6 @@ typedef struct shared_vmport_iopage shared_vmport_iopage_t;
 
 static shared_vmport_iopage_t *shared_vmport_page;
 
-static QLIST_HEAD(, XenPhysmap) xen_physmap;
-static const XenPhysmap *log_for_dirtybit;
-/* Buffer used by xen_sync_dirty_bitmap */
-static unsigned long *dirty_bitmap;
 static Notifier suspend;
 static Notifier wakeup;
 
@@ -175,312 +164,6 @@ static void xen_ram_init(PCMachineState *pcms,
     }
 }
 
-static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
-                                   int page_mask)
-{
-    XenPhysmap *physmap = NULL;
-
-    start_addr &= page_mask;
-
-    QLIST_FOREACH(physmap, &xen_physmap, list) {
-        if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
-            return physmap;
-        }
-    }
-    return NULL;
-}
-
-static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
-                                       int page_mask)
-{
-    hwaddr addr = phys_offset & page_mask;
-    XenPhysmap *physmap = NULL;
-
-    QLIST_FOREACH(physmap, &xen_physmap, list) {
-        if (range_covers_byte(physmap->phys_offset, physmap->size, addr)) {
-            return physmap->start_addr + (phys_offset - physmap->phys_offset);
-        }
-    }
-
-    return phys_offset;
-}
-
-#ifdef XEN_COMPAT_PHYSMAP
-static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
-{
-    char path[80], value[17];
-
-    snprintf(path, sizeof(path),
-            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
-            xen_domid, (uint64_t)physmap->phys_offset);
-    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
-    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
-        return -1;
-    }
-    snprintf(path, sizeof(path),
-            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
-            xen_domid, (uint64_t)physmap->phys_offset);
-    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
-    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
-        return -1;
-    }
-    if (physmap->name) {
-        snprintf(path, sizeof(path),
-                "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
-                xen_domid, (uint64_t)physmap->phys_offset);
-        if (!xs_write(state->xenstore, 0, path,
-                      physmap->name, strlen(physmap->name))) {
-            return -1;
-        }
-    }
-    return 0;
-}
-#else
-static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
-{
-    return 0;
-}
-#endif
-
-static int xen_add_to_physmap(XenIOState *state,
-                              hwaddr start_addr,
-                              ram_addr_t size,
-                              MemoryRegion *mr,
-                              hwaddr offset_within_region)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    unsigned long nr_pages;
-    int rc = 0;
-    XenPhysmap *physmap = NULL;
-    hwaddr pfn, start_gpfn;
-    hwaddr phys_offset = memory_region_get_ram_addr(mr);
-    const char *mr_name;
-
-    if (get_physmapping(start_addr, size, page_mask)) {
-        return 0;
-    }
-    if (size <= 0) {
-        return -1;
-    }
-
-    /* Xen can only handle a single dirty log region for now and we want
-     * the linear framebuffer to be that region.
-     * Avoid tracking any regions that is not videoram and avoid tracking
-     * the legacy vga region. */
-    if (mr == framebuffer && start_addr > 0xbffff) {
-        goto go_physmap;
-    }
-    return -1;
-
-go_physmap:
-    DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
-            start_addr, start_addr + size);
-
-    mr_name = memory_region_name(mr);
-
-    physmap = g_new(XenPhysmap, 1);
-
-    physmap->start_addr = start_addr;
-    physmap->size = size;
-    physmap->name = mr_name;
-    physmap->phys_offset = phys_offset;
-
-    QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
-
-    if (runstate_check(RUN_STATE_INMIGRATE)) {
-        /* Now when we have a physmap entry we can replace a dummy mapping with
-         * a real one of guest foreign memory. */
-        uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
-        assert(p && p == memory_region_get_ram_ptr(mr));
-
-        return 0;
-    }
-
-    pfn = phys_offset >> target_page_bits;
-    start_gpfn = start_addr >> target_page_bits;
-    nr_pages = size >> target_page_bits;
-    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
-                                        start_gpfn);
-    if (rc) {
-        int saved_errno = errno;
-
-        error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
-                     " to GFN %"HWADDR_PRIx" failed: %s",
-                     nr_pages, pfn, start_gpfn, strerror(saved_errno));
-        errno = saved_errno;
-        return -1;
-    }
-
-    rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
-                                   start_addr >> target_page_bits,
-                                   (start_addr + size - 1) >> target_page_bits,
-                                   XEN_DOMCTL_MEM_CACHEATTR_WB);
-    if (rc) {
-        error_report("pin_memory_cacheattr failed: %s", strerror(errno));
-    }
-    return xen_save_physmap(state, physmap);
-}
-
-static int xen_remove_from_physmap(XenIOState *state,
-                                   hwaddr start_addr,
-                                   ram_addr_t size)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    int rc = 0;
-    XenPhysmap *physmap = NULL;
-    hwaddr phys_offset = 0;
-
-    physmap = get_physmapping(start_addr, size, page_mask);
-    if (physmap == NULL) {
-        return -1;
-    }
-
-    phys_offset = physmap->phys_offset;
-    size = physmap->size;
-
-    DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
-            "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
-
-    size >>= target_page_bits;
-    start_addr >>= target_page_bits;
-    phys_offset >>= target_page_bits;
-    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
-                                        phys_offset);
-    if (rc) {
-        int saved_errno = errno;
-
-        error_report("relocate_memory "RAM_ADDR_FMT" pages"
-                     " from GFN %"HWADDR_PRIx
-                     " to GFN %"HWADDR_PRIx" failed: %s",
-                     size, start_addr, phys_offset, strerror(saved_errno));
-        errno = saved_errno;
-        return -1;
-    }
-
-    QLIST_REMOVE(physmap, list);
-    if (log_for_dirtybit == physmap) {
-        log_for_dirtybit = NULL;
-        g_free(dirty_bitmap);
-        dirty_bitmap = NULL;
-    }
-    g_free(physmap);
-
-    return 0;
-}
-
-static void xen_sync_dirty_bitmap(XenIOState *state,
-                                  hwaddr start_addr,
-                                  ram_addr_t size)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    hwaddr npages = size >> target_page_bits;
-    const int width = sizeof(unsigned long) * 8;
-    size_t bitmap_size = DIV_ROUND_UP(npages, width);
-    int rc, i, j;
-    const XenPhysmap *physmap = NULL;
-
-    physmap = get_physmapping(start_addr, size, page_mask);
-    if (physmap == NULL) {
-        /* not handled */
-        return;
-    }
-
-    if (log_for_dirtybit == NULL) {
-        log_for_dirtybit = physmap;
-        dirty_bitmap = g_new(unsigned long, bitmap_size);
-    } else if (log_for_dirtybit != physmap) {
-        /* Only one range for dirty bitmap can be tracked. */
-        return;
-    }
-
-    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
-                              npages, dirty_bitmap);
-    if (rc < 0) {
-#ifndef ENODATA
-#define ENODATA  ENOENT
-#endif
-        if (errno == ENODATA) {
-            memory_region_set_dirty(framebuffer, 0, size);
-            DPRINTF("xen: track_dirty_vram failed (0x" HWADDR_FMT_plx
-                    ", 0x" HWADDR_FMT_plx "): %s\n",
-                    start_addr, start_addr + size, strerror(errno));
-        }
-        return;
-    }
-
-    for (i = 0; i < bitmap_size; i++) {
-        unsigned long map = dirty_bitmap[i];
-        while (map != 0) {
-            j = ctzl(map);
-            map &= ~(1ul << j);
-            memory_region_set_dirty(framebuffer,
-                                    (i * width + j) * page_size, page_size);
-        };
-    }
-}
-
-static void xen_log_start(MemoryListener *listener,
-                          MemoryRegionSection *section,
-                          int old, int new)
-{
-    XenIOState *state = container_of(listener, XenIOState, memory_listener);
-
-    if (new & ~old & (1 << DIRTY_MEMORY_VGA)) {
-        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
-                              int128_get64(section->size));
-    }
-}
-
-static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
-                         int old, int new)
-{
-    if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
-        log_for_dirtybit = NULL;
-        g_free(dirty_bitmap);
-        dirty_bitmap = NULL;
-        /* Disable dirty bit tracking */
-        xen_track_dirty_vram(xen_domid, 0, 0, NULL);
-    }
-}
-
-static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
-{
-    XenIOState *state = container_of(listener, XenIOState, memory_listener);
-
-    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
-                          int128_get64(section->size));
-}
-
-static void xen_log_global_start(MemoryListener *listener)
-{
-    if (xen_enabled()) {
-        xen_in_migration = true;
-    }
-}
-
-static void xen_log_global_stop(MemoryListener *listener)
-{
-    xen_in_migration = false;
-}
-
-const MemoryListener xen_memory_listener = {
-    .name = "xen-memory",
-    .region_add = xen_region_add,
-    .region_del = xen_region_del,
-    .log_start = xen_log_start,
-    .log_stop = xen_log_stop,
-    .log_sync = xen_log_sync,
-    .log_global_start = xen_log_global_start,
-    .log_global_stop = xen_log_global_stop,
-    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
-};
-
 static void regs_to_cpu(vmware_regs_t *vmport_regs, ioreq_t *req)
 {
     X86CPU *cpu;
@@ -524,63 +207,6 @@ static void handle_vmport_ioreq(XenIOState *state, ioreq_t *req)
     current_cpu = NULL;
 }
 
-#ifdef XEN_COMPAT_PHYSMAP
-void xen_read_physmap(XenIOState *state)
-{
-    XenPhysmap *physmap = NULL;
-    unsigned int len, num, i;
-    char path[80], *value = NULL;
-    char **entries = NULL;
-
-    QLIST_INIT(&xen_physmap);
-
-    snprintf(path, sizeof(path),
-            "/local/domain/0/device-model/%d/physmap", xen_domid);
-    entries = xs_directory(state->xenstore, 0, path, &num);
-    if (entries == NULL)
-        return;
-
-    for (i = 0; i < num; i++) {
-        physmap = g_new(XenPhysmap, 1);
-        physmap->phys_offset = strtoull(entries[i], NULL, 16);
-        snprintf(path, sizeof(path),
-                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
-                xen_domid, entries[i]);
-        value = xs_read(state->xenstore, 0, path, &len);
-        if (value == NULL) {
-            g_free(physmap);
-            continue;
-        }
-        physmap->start_addr = strtoull(value, NULL, 16);
-        free(value);
-
-        snprintf(path, sizeof(path),
-                "/local/domain/0/device-model/%d/physmap/%s/size",
-                xen_domid, entries[i]);
-        value = xs_read(state->xenstore, 0, path, &len);
-        if (value == NULL) {
-            g_free(physmap);
-            continue;
-        }
-        physmap->size = strtoull(value, NULL, 16);
-        free(value);
-
-        snprintf(path, sizeof(path),
-                "/local/domain/0/device-model/%d/physmap/%s/name",
-                xen_domid, entries[i]);
-        physmap->name = xs_read(state->xenstore, 0, path, &len);
-
-        QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
-    }
-    free(entries);
-}
-#else
-void xen_read_physmap(XenIOState *state)
-{
-    QLIST_INIT(&xen_physmap);
-}
-#endif
-
 static void xen_wakeup_notifier(Notifier *notifier, void *data)
 {
     xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
@@ -635,91 +261,6 @@ err:
     exit(1);
 }
 
-void xen_register_framebuffer(MemoryRegion *mr)
-{
-    framebuffer = mr;
-}
-
-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-
-    if (unlikely(xen_in_migration)) {
-        int rc;
-        ram_addr_t start_pfn, nb_pages;
-
-        start = xen_phys_offset_to_gaddr(start, length, page_mask);
-
-        if (length == 0) {
-            length = page_size;
-        }
-        start_pfn = start >> target_page_bits;
-        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
-            - start_pfn;
-        rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
-        if (rc) {
-            fprintf(stderr,
-                    "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n",
-                    __func__, start, nb_pages, errno, strerror(errno));
-        }
-    }
-}
-
-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
-{
-    if (enable) {
-        memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
-    } else {
-        memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
-    }
-}
-
-void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
-                         bool add)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    hwaddr start_addr = section->offset_within_address_space;
-    ram_addr_t size = int128_get64(section->size);
-    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
-    hvmmem_type_t mem_type;
-
-    if (!memory_region_is_ram(section->mr)) {
-        return;
-    }
-
-    if (log_dirty != add) {
-        return;
-    }
-
-    trace_xen_client_set_memory(start_addr, size, log_dirty);
-
-    start_addr &= page_mask;
-    size = ROUND_UP(size, page_size);
-
-    if (add) {
-        if (!memory_region_is_rom(section->mr)) {
-            xen_add_to_physmap(state, start_addr, size,
-                               section->mr, section->offset_within_region);
-        } else {
-            mem_type = HVMMEM_ram_ro;
-            if (xen_set_mem_type(xen_domid, mem_type,
-                                 start_addr >> target_page_bits,
-                                 size >> target_page_bits)) {
-                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
-                        start_addr);
-            }
-        }
-    } else {
-        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
-            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
-        }
-    }
-}
-
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
     switch (req->type) {
diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index 3f0df8bc07..d38759cfe4 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -1,6 +1,7 @@
 i386_ss.add(when: 'CONFIG_XEN', if_true: files(
   'xen_apic.c',
   'xen_pvdevice.c',
+  'xen-hvm-common.c',
 ))
 i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
   'xen-hvm.c',
-- 
2.41.0

Re: [PATCH-for-9.0 7/9] hw/xen/hvm: Extract common code to xen-hvm-common.c

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>Extract non-x86 specific code out of xen-hvm.c,
>to xen-hvm-common.c. For now this new file is
>only build for x86 targets.

s/build/built

>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/xen/xen-hvm-common.c | 473 +++++++++++++++++++++++++++++++++++
> hw/i386/xen/xen-hvm.c        | 459 ---------------------------------
> hw/i386/xen/meson.build      |   1 +
> 3 files changed, 474 insertions(+), 459 deletions(-)
> create mode 100644 hw/i386/xen/xen-hvm-common.c
>
>diff --git a/hw/i386/xen/xen-hvm-common.c b/hw/i386/xen/xen-hvm-common.c
>new file mode 100644
>index 0000000000..e8ef0e0c94
>--- /dev/null
>+++ b/hw/i386/xen/xen-hvm-common.c
>@@ -0,0 +1,473 @@
>+/*
>+ * Copyright (C) 2010       Citrix Ltd.
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2.  See
>+ * the COPYING file in the top-level directory.
>+ *
>+ * Contributions after 2012-01-13 are licensed under the terms of the
>+ * GNU GPL, version 2 or (at your option) any later version.
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "qemu/range.h"
>+#include "qapi/qapi-commands-migration.h"
>+#include "exec/target_page.h"
>+#include "hw/xen/xen-hvm-common.h"
>+#include "trace.h"
>+
>+static MemoryRegion *framebuffer;
>+static bool xen_in_migration;
>+
>+static QLIST_HEAD(, XenPhysmap) xen_physmap;
>+static const XenPhysmap *log_for_dirtybit;
>+/* Buffer used by xen_sync_dirty_bitmap */
>+static unsigned long *dirty_bitmap;
>+
>+static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
>+                                   int page_mask)
>+{
>+    XenPhysmap *physmap = NULL;
>+
>+    start_addr &= -page_mask;
>+
>+    QLIST_FOREACH(physmap, &xen_physmap, list) {
>+        if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
>+            return physmap;
>+        }
>+    }
>+    return NULL;
>+}
>+
>+static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
>+                                       int page_mask)
>+{
>+    hwaddr addr = phys_offset & -page_mask;
>+    XenPhysmap *physmap = NULL;
>+
>+    QLIST_FOREACH(physmap, &xen_physmap, list) {
>+        if (range_covers_byte(physmap->phys_offset, physmap->size, addr)) {
>+            return physmap->start_addr + (phys_offset - physmap->phys_offset);
>+        }
>+    }
>+
>+    return phys_offset;
>+}
>+
>+#ifdef XEN_COMPAT_PHYSMAP
>+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
>+{
>+    char path[80], value[17];
>+
>+    snprintf(path, sizeof(path),
>+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
>+            xen_domid, (uint64_t)physmap->phys_offset);
>+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
>+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>+        return -1;
>+    }
>+    snprintf(path, sizeof(path),
>+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
>+            xen_domid, (uint64_t)physmap->phys_offset);
>+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
>+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>+        return -1;
>+    }
>+    if (physmap->name) {
>+        snprintf(path, sizeof(path),
>+                "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
>+                xen_domid, (uint64_t)physmap->phys_offset);
>+        if (!xs_write(state->xenstore, 0, path,
>+                      physmap->name, strlen(physmap->name))) {
>+            return -1;
>+        }
>+    }
>+    return 0;
>+}
>+#else
>+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
>+{
>+    return 0;
>+}
>+#endif
>+
>+static int xen_add_to_physmap(XenIOState *state,
>+                              hwaddr start_addr,
>+                              ram_addr_t size,
>+                              MemoryRegion *mr,
>+                              hwaddr offset_within_region)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    unsigned long nr_pages;
>+    int rc = 0;
>+    XenPhysmap *physmap = NULL;
>+    hwaddr pfn, start_gpfn;
>+    hwaddr phys_offset = memory_region_get_ram_addr(mr);
>+    const char *mr_name;
>+
>+    if (get_physmapping(start_addr, size, page_mask)) {
>+        return 0;
>+    }
>+    if (size <= 0) {
>+        return -1;
>+    }
>+
>+    /* Xen can only handle a single dirty log region for now and we want
>+     * the linear framebuffer to be that region.
>+     * Avoid tracking any regions that is not videoram and avoid tracking
>+     * the legacy vga region. */
>+    if (mr == framebuffer && start_addr > 0xbffff) {
>+        goto go_physmap;
>+    }
>+    return -1;
>+
>+go_physmap:
>+    DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
>+            start_addr, start_addr + size);
>+
>+    mr_name = memory_region_name(mr);
>+
>+    physmap = g_new(XenPhysmap, 1);
>+
>+    physmap->start_addr = start_addr;
>+    physmap->size = size;
>+    physmap->name = mr_name;
>+    physmap->phys_offset = phys_offset;
>+
>+    QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
>+
>+    if (runstate_check(RUN_STATE_INMIGRATE)) {
>+        /* Now when we have a physmap entry we can replace a dummy mapping with
>+         * a real one of guest foreign memory. */
>+        uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
>+        assert(p && p == memory_region_get_ram_ptr(mr));
>+
>+        return 0;
>+    }
>+
>+    pfn = phys_offset >> target_page_bits;
>+    start_gpfn = start_addr >> target_page_bits;
>+    nr_pages = size >> target_page_bits;
>+    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
>+                                        start_gpfn);
>+    if (rc) {
>+        int saved_errno = errno;
>+
>+        error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
>+                     " to GFN %"HWADDR_PRIx" failed: %s",
>+                     nr_pages, pfn, start_gpfn, strerror(saved_errno));
>+        errno = saved_errno;
>+        return -1;
>+    }
>+
>+    rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
>+                                   start_addr >> target_page_bits,
>+                                   (start_addr + size - 1) >> target_page_bits,
>+                                   XEN_DOMCTL_MEM_CACHEATTR_WB);
>+    if (rc) {
>+        error_report("pin_memory_cacheattr failed: %s", strerror(errno));
>+    }
>+    return xen_save_physmap(state, physmap);
>+}
>+
>+static int xen_remove_from_physmap(XenIOState *state,
>+                                   hwaddr start_addr,
>+                                   ram_addr_t size)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    int rc = 0;
>+    XenPhysmap *physmap = NULL;
>+    hwaddr phys_offset = 0;
>+
>+    physmap = get_physmapping(start_addr, size, page_mask);
>+    if (physmap == NULL) {
>+        return -1;
>+    }
>+
>+    phys_offset = physmap->phys_offset;
>+    size = physmap->size;
>+
>+    DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
>+            "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
>+
>+    size >>= target_page_bits;
>+    start_addr >>= target_page_bits;
>+    phys_offset >>= target_page_bits;
>+    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
>+                                        phys_offset);
>+    if (rc) {
>+        int saved_errno = errno;
>+
>+        error_report("relocate_memory "RAM_ADDR_FMT" pages"
>+                     " from GFN %"HWADDR_PRIx
>+                     " to GFN %"HWADDR_PRIx" failed: %s",
>+                     size, start_addr, phys_offset, strerror(saved_errno));
>+        errno = saved_errno;
>+        return -1;
>+    }
>+
>+    QLIST_REMOVE(physmap, list);
>+    if (log_for_dirtybit == physmap) {
>+        log_for_dirtybit = NULL;
>+        g_free(dirty_bitmap);
>+        dirty_bitmap = NULL;
>+    }
>+    g_free(physmap);
>+
>+    return 0;
>+}
>+
>+static void xen_sync_dirty_bitmap(XenIOState *state,
>+                                  hwaddr start_addr,
>+                                  ram_addr_t size)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    hwaddr npages = size >> target_page_bits;
>+    const int width = sizeof(unsigned long) * 8;
>+    size_t bitmap_size = DIV_ROUND_UP(npages, width);
>+    int rc, i, j;
>+    const XenPhysmap *physmap = NULL;
>+
>+    physmap = get_physmapping(start_addr, size, page_mask);
>+    if (physmap == NULL) {
>+        /* not handled */
>+        return;
>+    }
>+
>+    if (log_for_dirtybit == NULL) {
>+        log_for_dirtybit = physmap;
>+        dirty_bitmap = g_new(unsigned long, bitmap_size);
>+    } else if (log_for_dirtybit != physmap) {
>+        /* Only one range for dirty bitmap can be tracked. */
>+        return;
>+    }
>+
>+    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
>+                              npages, dirty_bitmap);
>+    if (rc < 0) {
>+#ifndef ENODATA
>+#define ENODATA  ENOENT
>+#endif
>+        if (errno == ENODATA) {
>+            memory_region_set_dirty(framebuffer, 0, size);
>+            DPRINTF("xen: track_dirty_vram failed (0x" HWADDR_FMT_plx
>+                    ", 0x" HWADDR_FMT_plx "): %s\n",
>+                    start_addr, start_addr + size, strerror(errno));
>+        }
>+        return;
>+    }
>+
>+    for (i = 0; i < bitmap_size; i++) {
>+        unsigned long map = dirty_bitmap[i];
>+        while (map != 0) {
>+            j = ctzl(map);
>+            map &= ~(1ul << j);
>+            memory_region_set_dirty(framebuffer,
>+                                    (i * width + j) * page_size,
>+                                    page_size);
>+        };
>+    }
>+}
>+
>+static void xen_log_start(MemoryListener *listener,
>+                          MemoryRegionSection *section,
>+                          int old, int new)
>+{
>+    XenIOState *state = container_of(listener, XenIOState, memory_listener);
>+
>+    if (new & ~old & (1 << DIRTY_MEMORY_VGA)) {
>+        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
>+                              int128_get64(section->size));
>+    }
>+}
>+
>+static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
>+                         int old, int new)
>+{
>+    if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
>+        log_for_dirtybit = NULL;
>+        g_free(dirty_bitmap);
>+        dirty_bitmap = NULL;
>+        /* Disable dirty bit tracking */
>+        xen_track_dirty_vram(xen_domid, 0, 0, NULL);
>+    }
>+}
>+
>+static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
>+{
>+    XenIOState *state = container_of(listener, XenIOState, memory_listener);
>+
>+    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
>+                          int128_get64(section->size));
>+}
>+
>+static void xen_log_global_start(MemoryListener *listener)
>+{
>+    if (xen_enabled()) {
>+        xen_in_migration = true;
>+    }
>+}
>+
>+static void xen_log_global_stop(MemoryListener *listener)
>+{
>+    xen_in_migration = false;
>+}
>+
>+const MemoryListener xen_memory_listener = {
>+    .name = "xen-memory",
>+    .region_add = xen_region_add,
>+    .region_del = xen_region_del,
>+    .log_start = xen_log_start,
>+    .log_stop = xen_log_stop,
>+    .log_sync = xen_log_sync,
>+    .log_global_start = xen_log_global_start,
>+    .log_global_stop = xen_log_global_stop,
>+    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
>+};
>+
>+#ifdef XEN_COMPAT_PHYSMAP
>+void xen_read_physmap(XenIOState *state)
>+{
>+    XenPhysmap *physmap = NULL;
>+    unsigned int len, num, i;
>+    char path[80], *value = NULL;
>+    char **entries = NULL;
>+
>+    QLIST_INIT(&xen_physmap);
>+
>+    snprintf(path, sizeof(path),
>+            "/local/domain/0/device-model/%d/physmap", xen_domid);
>+    entries = xs_directory(state->xenstore, 0, path, &num);
>+    if (entries == NULL)
>+        return;
>+
>+    for (i = 0; i < num; i++) {
>+        physmap = g_new(XenPhysmap, 1);
>+        physmap->phys_offset = strtoull(entries[i], NULL, 16);
>+        snprintf(path, sizeof(path),
>+                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
>+                xen_domid, entries[i]);
>+        value = xs_read(state->xenstore, 0, path, &len);
>+        if (value == NULL) {
>+            g_free(physmap);
>+            continue;
>+        }
>+        physmap->start_addr = strtoull(value, NULL, 16);
>+        free(value);
>+
>+        snprintf(path, sizeof(path),
>+                "/local/domain/0/device-model/%d/physmap/%s/size",
>+                xen_domid, entries[i]);
>+        value = xs_read(state->xenstore, 0, path, &len);
>+        if (value == NULL) {
>+            g_free(physmap);
>+            continue;
>+        }
>+        physmap->size = strtoull(value, NULL, 16);
>+        free(value);
>+
>+        snprintf(path, sizeof(path),
>+                "/local/domain/0/device-model/%d/physmap/%s/name",
>+                xen_domid, entries[i]);
>+        physmap->name = xs_read(state->xenstore, 0, path, &len);
>+
>+        QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
>+    }
>+    free(entries);
>+}
>+#else
>+void xen_read_physmap(XenIOState *state)
>+{
>+    QLIST_INIT(&xen_physmap);
>+}
>+#endif
>+
>+void xen_register_framebuffer(MemoryRegion *mr)
>+{
>+    framebuffer = mr;
>+}
>+
>+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+
>+    if (unlikely(xen_in_migration)) {
>+        int rc;
>+        ram_addr_t start_pfn, nb_pages;
>+
>+        start = xen_phys_offset_to_gaddr(start, length, page_mask);
>+
>+        if (length == 0) {
>+            length = page_size;
>+        }
>+        start_pfn = start >> target_page_bits;
>+        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
>+            - start_pfn;
>+        rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
>+        if (rc) {
>+            fprintf(stderr,
>+                    "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n",
>+                    __func__, start, nb_pages, errno, strerror(errno));
>+        }
>+    }
>+}
>+
>+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
>+{
>+    if (enable) {
>+        memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
>+    } else {
>+        memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
>+    }
>+}
>+
>+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>+                         bool add)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    hwaddr start_addr = section->offset_within_address_space;
>+    ram_addr_t size = int128_get64(section->size);
>+    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
>+    hvmmem_type_t mem_type;
>+
>+    if (!memory_region_is_ram(section->mr)) {
>+        return;
>+    }
>+
>+    if (log_dirty != add) {
>+        return;
>+    }
>+
>+    trace_xen_client_set_memory(start_addr, size, log_dirty);
>+
>+    start_addr &= page_mask;
>+    size = ROUND_UP(size, page_size);
>+
>+    if (add) {
>+        if (!memory_region_is_rom(section->mr)) {
>+            xen_add_to_physmap(state, start_addr, size,
>+                               section->mr, section->offset_within_region);
>+        } else {
>+            mem_type = HVMMEM_ram_ro;
>+            if (xen_set_mem_type(xen_domid, mem_type,
>+                                 start_addr >> target_page_bits,
>+                                 size >> target_page_bits)) {
>+                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
>+                        start_addr);
>+            }
>+        }
>+    } else {
>+        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
>+            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
>+        }
>+    }
>+}
>diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
>index 3b9c31c1c8..5657693e1b 100644
>--- a/hw/i386/xen/xen-hvm.c
>+++ b/hw/i386/xen/xen-hvm.c
>@@ -9,16 +9,11 @@
>  */
> 
> #include "qemu/osdep.h"
>-#include "qemu/units.h"
>-#include "qapi/error.h"
>-#include "qapi/qapi-commands-migration.h"
>-#include "trace.h"
> 
> #include "hw/i386/pc.h"
> #include "hw/irq.h"
> #include "hw/i386/apic-msidef.h"
> #include "hw/xen/xen-x86.h"
>-#include "qemu/range.h"
> 
> #include "hw/xen/xen-hvm-common.h"
> #include <xen/hvm/e820.h>
>@@ -26,8 +21,6 @@
> #include "cpu.h"
> 
> static MemoryRegion ram_640k, ram_lo, ram_hi;
>-static MemoryRegion *framebuffer;
>-static bool xen_in_migration;
> 
> /* Compatibility with older version */
> 
>@@ -56,10 +49,6 @@ typedef struct shared_vmport_iopage shared_vmport_iopage_t;
> 
> static shared_vmport_iopage_t *shared_vmport_page;
> 
>-static QLIST_HEAD(, XenPhysmap) xen_physmap;
>-static const XenPhysmap *log_for_dirtybit;
>-/* Buffer used by xen_sync_dirty_bitmap */
>-static unsigned long *dirty_bitmap;
> static Notifier suspend;
> static Notifier wakeup;
> 
>@@ -175,312 +164,6 @@ static void xen_ram_init(PCMachineState *pcms,
>     }
> }
> 
>-static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
>-                                   int page_mask)
>-{
>-    XenPhysmap *physmap = NULL;
>-
>-    start_addr &= page_mask;
>-
>-    QLIST_FOREACH(physmap, &xen_physmap, list) {
>-        if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
>-            return physmap;
>-        }
>-    }
>-    return NULL;
>-}
>-
>-static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
>-                                       int page_mask)
>-{
>-    hwaddr addr = phys_offset & page_mask;
>-    XenPhysmap *physmap = NULL;
>-
>-    QLIST_FOREACH(physmap, &xen_physmap, list) {
>-        if (range_covers_byte(physmap->phys_offset, physmap->size, addr)) {
>-            return physmap->start_addr + (phys_offset - physmap->phys_offset);
>-        }
>-    }
>-
>-    return phys_offset;
>-}
>-
>-#ifdef XEN_COMPAT_PHYSMAP
>-static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
>-{
>-    char path[80], value[17];
>-
>-    snprintf(path, sizeof(path),
>-            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
>-            xen_domid, (uint64_t)physmap->phys_offset);
>-    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
>-    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>-        return -1;
>-    }
>-    snprintf(path, sizeof(path),
>-            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
>-            xen_domid, (uint64_t)physmap->phys_offset);
>-    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
>-    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>-        return -1;
>-    }
>-    if (physmap->name) {
>-        snprintf(path, sizeof(path),
>-                "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
>-                xen_domid, (uint64_t)physmap->phys_offset);
>-        if (!xs_write(state->xenstore, 0, path,
>-                      physmap->name, strlen(physmap->name))) {
>-            return -1;
>-        }
>-    }
>-    return 0;
>-}
>-#else
>-static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
>-{
>-    return 0;
>-}
>-#endif
>-
>-static int xen_add_to_physmap(XenIOState *state,
>-                              hwaddr start_addr,
>-                              ram_addr_t size,
>-                              MemoryRegion *mr,
>-                              hwaddr offset_within_region)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    unsigned long nr_pages;
>-    int rc = 0;
>-    XenPhysmap *physmap = NULL;
>-    hwaddr pfn, start_gpfn;
>-    hwaddr phys_offset = memory_region_get_ram_addr(mr);
>-    const char *mr_name;
>-
>-    if (get_physmapping(start_addr, size, page_mask)) {
>-        return 0;
>-    }
>-    if (size <= 0) {
>-        return -1;
>-    }
>-
>-    /* Xen can only handle a single dirty log region for now and we want
>-     * the linear framebuffer to be that region.
>-     * Avoid tracking any regions that is not videoram and avoid tracking
>-     * the legacy vga region. */
>-    if (mr == framebuffer && start_addr > 0xbffff) {
>-        goto go_physmap;
>-    }
>-    return -1;
>-
>-go_physmap:
>-    DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
>-            start_addr, start_addr + size);
>-
>-    mr_name = memory_region_name(mr);
>-
>-    physmap = g_new(XenPhysmap, 1);
>-
>-    physmap->start_addr = start_addr;
>-    physmap->size = size;
>-    physmap->name = mr_name;
>-    physmap->phys_offset = phys_offset;
>-
>-    QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
>-
>-    if (runstate_check(RUN_STATE_INMIGRATE)) {
>-        /* Now when we have a physmap entry we can replace a dummy mapping with
>-         * a real one of guest foreign memory. */
>-        uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
>-        assert(p && p == memory_region_get_ram_ptr(mr));
>-
>-        return 0;
>-    }
>-
>-    pfn = phys_offset >> target_page_bits;
>-    start_gpfn = start_addr >> target_page_bits;
>-    nr_pages = size >> target_page_bits;
>-    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
>-                                        start_gpfn);
>-    if (rc) {
>-        int saved_errno = errno;
>-
>-        error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
>-                     " to GFN %"HWADDR_PRIx" failed: %s",
>-                     nr_pages, pfn, start_gpfn, strerror(saved_errno));
>-        errno = saved_errno;
>-        return -1;
>-    }
>-
>-    rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
>-                                   start_addr >> target_page_bits,
>-                                   (start_addr + size - 1) >> target_page_bits,
>-                                   XEN_DOMCTL_MEM_CACHEATTR_WB);
>-    if (rc) {
>-        error_report("pin_memory_cacheattr failed: %s", strerror(errno));
>-    }
>-    return xen_save_physmap(state, physmap);
>-}
>-
>-static int xen_remove_from_physmap(XenIOState *state,
>-                                   hwaddr start_addr,
>-                                   ram_addr_t size)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    int rc = 0;
>-    XenPhysmap *physmap = NULL;
>-    hwaddr phys_offset = 0;
>-
>-    physmap = get_physmapping(start_addr, size, page_mask);
>-    if (physmap == NULL) {
>-        return -1;
>-    }
>-
>-    phys_offset = physmap->phys_offset;
>-    size = physmap->size;
>-
>-    DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
>-            "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
>-
>-    size >>= target_page_bits;
>-    start_addr >>= target_page_bits;
>-    phys_offset >>= target_page_bits;
>-    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
>-                                        phys_offset);
>-    if (rc) {
>-        int saved_errno = errno;
>-
>-        error_report("relocate_memory "RAM_ADDR_FMT" pages"
>-                     " from GFN %"HWADDR_PRIx
>-                     " to GFN %"HWADDR_PRIx" failed: %s",
>-                     size, start_addr, phys_offset, strerror(saved_errno));
>-        errno = saved_errno;
>-        return -1;
>-    }
>-
>-    QLIST_REMOVE(physmap, list);
>-    if (log_for_dirtybit == physmap) {
>-        log_for_dirtybit = NULL;
>-        g_free(dirty_bitmap);
>-        dirty_bitmap = NULL;
>-    }
>-    g_free(physmap);
>-
>-    return 0;
>-}
>-
>-static void xen_sync_dirty_bitmap(XenIOState *state,
>-                                  hwaddr start_addr,
>-                                  ram_addr_t size)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    hwaddr npages = size >> target_page_bits;
>-    const int width = sizeof(unsigned long) * 8;
>-    size_t bitmap_size = DIV_ROUND_UP(npages, width);
>-    int rc, i, j;
>-    const XenPhysmap *physmap = NULL;
>-
>-    physmap = get_physmapping(start_addr, size, page_mask);
>-    if (physmap == NULL) {
>-        /* not handled */
>-        return;
>-    }
>-
>-    if (log_for_dirtybit == NULL) {
>-        log_for_dirtybit = physmap;
>-        dirty_bitmap = g_new(unsigned long, bitmap_size);
>-    } else if (log_for_dirtybit != physmap) {
>-        /* Only one range for dirty bitmap can be tracked. */
>-        return;
>-    }
>-
>-    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
>-                              npages, dirty_bitmap);
>-    if (rc < 0) {
>-#ifndef ENODATA
>-#define ENODATA  ENOENT
>-#endif
>-        if (errno == ENODATA) {
>-            memory_region_set_dirty(framebuffer, 0, size);
>-            DPRINTF("xen: track_dirty_vram failed (0x" HWADDR_FMT_plx
>-                    ", 0x" HWADDR_FMT_plx "): %s\n",
>-                    start_addr, start_addr + size, strerror(errno));
>-        }
>-        return;
>-    }
>-
>-    for (i = 0; i < bitmap_size; i++) {
>-        unsigned long map = dirty_bitmap[i];
>-        while (map != 0) {
>-            j = ctzl(map);
>-            map &= ~(1ul << j);
>-            memory_region_set_dirty(framebuffer,
>-                                    (i * width + j) * page_size, page_size);
>-        };
>-    }
>-}
>-
>-static void xen_log_start(MemoryListener *listener,
>-                          MemoryRegionSection *section,
>-                          int old, int new)
>-{
>-    XenIOState *state = container_of(listener, XenIOState, memory_listener);
>-
>-    if (new & ~old & (1 << DIRTY_MEMORY_VGA)) {
>-        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
>-                              int128_get64(section->size));
>-    }
>-}
>-
>-static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
>-                         int old, int new)
>-{
>-    if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
>-        log_for_dirtybit = NULL;
>-        g_free(dirty_bitmap);
>-        dirty_bitmap = NULL;
>-        /* Disable dirty bit tracking */
>-        xen_track_dirty_vram(xen_domid, 0, 0, NULL);
>-    }
>-}
>-
>-static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
>-{
>-    XenIOState *state = container_of(listener, XenIOState, memory_listener);
>-
>-    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
>-                          int128_get64(section->size));
>-}
>-
>-static void xen_log_global_start(MemoryListener *listener)
>-{
>-    if (xen_enabled()) {
>-        xen_in_migration = true;
>-    }
>-}
>-
>-static void xen_log_global_stop(MemoryListener *listener)
>-{
>-    xen_in_migration = false;
>-}
>-
>-const MemoryListener xen_memory_listener = {
>-    .name = "xen-memory",
>-    .region_add = xen_region_add,
>-    .region_del = xen_region_del,
>-    .log_start = xen_log_start,
>-    .log_stop = xen_log_stop,
>-    .log_sync = xen_log_sync,
>-    .log_global_start = xen_log_global_start,
>-    .log_global_stop = xen_log_global_stop,
>-    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
>-};
>-
> static void regs_to_cpu(vmware_regs_t *vmport_regs, ioreq_t *req)
> {
>     X86CPU *cpu;
>@@ -524,63 +207,6 @@ static void handle_vmport_ioreq(XenIOState *state, ioreq_t *req)
>     current_cpu = NULL;
> }
> 
>-#ifdef XEN_COMPAT_PHYSMAP
>-void xen_read_physmap(XenIOState *state)
>-{
>-    XenPhysmap *physmap = NULL;
>-    unsigned int len, num, i;
>-    char path[80], *value = NULL;
>-    char **entries = NULL;
>-
>-    QLIST_INIT(&xen_physmap);
>-
>-    snprintf(path, sizeof(path),
>-            "/local/domain/0/device-model/%d/physmap", xen_domid);
>-    entries = xs_directory(state->xenstore, 0, path, &num);
>-    if (entries == NULL)
>-        return;
>-
>-    for (i = 0; i < num; i++) {
>-        physmap = g_new(XenPhysmap, 1);
>-        physmap->phys_offset = strtoull(entries[i], NULL, 16);
>-        snprintf(path, sizeof(path),
>-                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
>-                xen_domid, entries[i]);
>-        value = xs_read(state->xenstore, 0, path, &len);
>-        if (value == NULL) {
>-            g_free(physmap);
>-            continue;
>-        }
>-        physmap->start_addr = strtoull(value, NULL, 16);
>-        free(value);
>-
>-        snprintf(path, sizeof(path),
>-                "/local/domain/0/device-model/%d/physmap/%s/size",
>-                xen_domid, entries[i]);
>-        value = xs_read(state->xenstore, 0, path, &len);
>-        if (value == NULL) {
>-            g_free(physmap);
>-            continue;
>-        }
>-        physmap->size = strtoull(value, NULL, 16);
>-        free(value);
>-
>-        snprintf(path, sizeof(path),
>-                "/local/domain/0/device-model/%d/physmap/%s/name",
>-                xen_domid, entries[i]);
>-        physmap->name = xs_read(state->xenstore, 0, path, &len);
>-
>-        QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
>-    }
>-    free(entries);
>-}
>-#else
>-void xen_read_physmap(XenIOState *state)
>-{
>-    QLIST_INIT(&xen_physmap);
>-}
>-#endif
>-
> static void xen_wakeup_notifier(Notifier *notifier, void *data)
> {
>     xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
>@@ -635,91 +261,6 @@ err:
>     exit(1);
> }
> 
>-void xen_register_framebuffer(MemoryRegion *mr)
>-{
>-    framebuffer = mr;
>-}
>-
>-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-
>-    if (unlikely(xen_in_migration)) {
>-        int rc;
>-        ram_addr_t start_pfn, nb_pages;
>-
>-        start = xen_phys_offset_to_gaddr(start, length, page_mask);
>-
>-        if (length == 0) {
>-            length = page_size;
>-        }
>-        start_pfn = start >> target_page_bits;
>-        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
>-            - start_pfn;
>-        rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
>-        if (rc) {
>-            fprintf(stderr,
>-                    "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n",
>-                    __func__, start, nb_pages, errno, strerror(errno));
>-        }
>-    }
>-}
>-
>-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
>-{
>-    if (enable) {
>-        memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
>-    } else {
>-        memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
>-    }
>-}
>-
>-void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>-                         bool add)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    hwaddr start_addr = section->offset_within_address_space;
>-    ram_addr_t size = int128_get64(section->size);
>-    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
>-    hvmmem_type_t mem_type;
>-
>-    if (!memory_region_is_ram(section->mr)) {
>-        return;
>-    }
>-
>-    if (log_dirty != add) {
>-        return;
>-    }
>-
>-    trace_xen_client_set_memory(start_addr, size, log_dirty);
>-
>-    start_addr &= page_mask;
>-    size = ROUND_UP(size, page_size);
>-
>-    if (add) {
>-        if (!memory_region_is_rom(section->mr)) {
>-            xen_add_to_physmap(state, start_addr, size,
>-                               section->mr, section->offset_within_region);
>-        } else {
>-            mem_type = HVMMEM_ram_ro;
>-            if (xen_set_mem_type(xen_domid, mem_type,
>-                                 start_addr >> target_page_bits,
>-                                 size >> target_page_bits)) {
>-                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
>-                        start_addr);
>-            }
>-        }
>-    } else {
>-        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
>-            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
>-        }
>-    }
>-}
>-
> void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
> {
>     switch (req->type) {
>diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
>index 3f0df8bc07..d38759cfe4 100644
>--- a/hw/i386/xen/meson.build
>+++ b/hw/i386/xen/meson.build
>@@ -1,6 +1,7 @@
> i386_ss.add(when: 'CONFIG_XEN', if_true: files(
>   'xen_apic.c',
>   'xen_pvdevice.c',
>+  'xen-hvm-common.c',
> ))
> i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
>   'xen-hvm.c',
>-- 
>2.41.0
>
>


Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

[RFC PATCH-for-9.0 8/9] hw/xen/hvm: Merge xen-hvm-common.c files

[Philippe Mathieu-Daudé]

hw/i386/xen/xen-hvm-common.c content is target agnostic,
and should be common to all targets. Merge both files.
Remove the now unnecessary xen_register_framebuffer() stub.

ARM targets now inherit the common xen_memory_listener.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/xen_arm.c             |  24 --
 hw/i386/xen/xen-hvm-common.c | 473 -----------------------------------
 hw/xen/xen-hvm-common.c      | 458 +++++++++++++++++++++++++++++++++
 stubs/xen-hw-stub.c          |   4 -
 hw/i386/xen/meson.build      |   1 -
 5 files changed, 458 insertions(+), 502 deletions(-)
 delete mode 100644 hw/i386/xen/xen-hvm-common.c

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 39dcd74d07..0ead84c9e1 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -38,17 +38,6 @@
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
 
-const MemoryListener xen_memory_listener = {
-    .region_add = xen_region_add,
-    .region_del = xen_region_del,
-    .log_start = NULL,
-    .log_stop = NULL,
-    .log_sync = NULL,
-    .log_global_start = NULL,
-    .log_global_stop = NULL,
-    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
-};
-
 struct XenArmState {
     /*< private >*/
     MachineState parent;
@@ -136,19 +125,6 @@ void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
     return;
 }
 
-void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
-                         bool add)
-{
-}
-
-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
-{
-}
-
-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
-{
-}
-
 #ifdef CONFIG_TPM
 static void xen_enable_tpm(XenArmState *xam)
 {
diff --git a/hw/i386/xen/xen-hvm-common.c b/hw/i386/xen/xen-hvm-common.c
deleted file mode 100644
index e8ef0e0c94..0000000000
--- a/hw/i386/xen/xen-hvm-common.c
+++ /dev/null
@@ -1,473 +0,0 @@
-/*
- * Copyright (C) 2010       Citrix Ltd.
- *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
- *
- * Contributions after 2012-01-13 are licensed under the terms of the
- * GNU GPL, version 2 or (at your option) any later version.
- */
-
-#include "qemu/osdep.h"
-#include "qemu/range.h"
-#include "qapi/qapi-commands-migration.h"
-#include "exec/target_page.h"
-#include "hw/xen/xen-hvm-common.h"
-#include "trace.h"
-
-static MemoryRegion *framebuffer;
-static bool xen_in_migration;
-
-static QLIST_HEAD(, XenPhysmap) xen_physmap;
-static const XenPhysmap *log_for_dirtybit;
-/* Buffer used by xen_sync_dirty_bitmap */
-static unsigned long *dirty_bitmap;
-
-static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
-                                   int page_mask)
-{
-    XenPhysmap *physmap = NULL;
-
-    start_addr &= -page_mask;
-
-    QLIST_FOREACH(physmap, &xen_physmap, list) {
-        if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
-            return physmap;
-        }
-    }
-    return NULL;
-}
-
-static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
-                                       int page_mask)
-{
-    hwaddr addr = phys_offset & -page_mask;
-    XenPhysmap *physmap = NULL;
-
-    QLIST_FOREACH(physmap, &xen_physmap, list) {
-        if (range_covers_byte(physmap->phys_offset, physmap->size, addr)) {
-            return physmap->start_addr + (phys_offset - physmap->phys_offset);
-        }
-    }
-
-    return phys_offset;
-}
-
-#ifdef XEN_COMPAT_PHYSMAP
-static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
-{
-    char path[80], value[17];
-
-    snprintf(path, sizeof(path),
-            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
-            xen_domid, (uint64_t)physmap->phys_offset);
-    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
-    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
-        return -1;
-    }
-    snprintf(path, sizeof(path),
-            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
-            xen_domid, (uint64_t)physmap->phys_offset);
-    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
-    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
-        return -1;
-    }
-    if (physmap->name) {
-        snprintf(path, sizeof(path),
-                "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
-                xen_domid, (uint64_t)physmap->phys_offset);
-        if (!xs_write(state->xenstore, 0, path,
-                      physmap->name, strlen(physmap->name))) {
-            return -1;
-        }
-    }
-    return 0;
-}
-#else
-static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
-{
-    return 0;
-}
-#endif
-
-static int xen_add_to_physmap(XenIOState *state,
-                              hwaddr start_addr,
-                              ram_addr_t size,
-                              MemoryRegion *mr,
-                              hwaddr offset_within_region)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    unsigned long nr_pages;
-    int rc = 0;
-    XenPhysmap *physmap = NULL;
-    hwaddr pfn, start_gpfn;
-    hwaddr phys_offset = memory_region_get_ram_addr(mr);
-    const char *mr_name;
-
-    if (get_physmapping(start_addr, size, page_mask)) {
-        return 0;
-    }
-    if (size <= 0) {
-        return -1;
-    }
-
-    /* Xen can only handle a single dirty log region for now and we want
-     * the linear framebuffer to be that region.
-     * Avoid tracking any regions that is not videoram and avoid tracking
-     * the legacy vga region. */
-    if (mr == framebuffer && start_addr > 0xbffff) {
-        goto go_physmap;
-    }
-    return -1;
-
-go_physmap:
-    DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
-            start_addr, start_addr + size);
-
-    mr_name = memory_region_name(mr);
-
-    physmap = g_new(XenPhysmap, 1);
-
-    physmap->start_addr = start_addr;
-    physmap->size = size;
-    physmap->name = mr_name;
-    physmap->phys_offset = phys_offset;
-
-    QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
-
-    if (runstate_check(RUN_STATE_INMIGRATE)) {
-        /* Now when we have a physmap entry we can replace a dummy mapping with
-         * a real one of guest foreign memory. */
-        uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
-        assert(p && p == memory_region_get_ram_ptr(mr));
-
-        return 0;
-    }
-
-    pfn = phys_offset >> target_page_bits;
-    start_gpfn = start_addr >> target_page_bits;
-    nr_pages = size >> target_page_bits;
-    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
-                                        start_gpfn);
-    if (rc) {
-        int saved_errno = errno;
-
-        error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
-                     " to GFN %"HWADDR_PRIx" failed: %s",
-                     nr_pages, pfn, start_gpfn, strerror(saved_errno));
-        errno = saved_errno;
-        return -1;
-    }
-
-    rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
-                                   start_addr >> target_page_bits,
-                                   (start_addr + size - 1) >> target_page_bits,
-                                   XEN_DOMCTL_MEM_CACHEATTR_WB);
-    if (rc) {
-        error_report("pin_memory_cacheattr failed: %s", strerror(errno));
-    }
-    return xen_save_physmap(state, physmap);
-}
-
-static int xen_remove_from_physmap(XenIOState *state,
-                                   hwaddr start_addr,
-                                   ram_addr_t size)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    int rc = 0;
-    XenPhysmap *physmap = NULL;
-    hwaddr phys_offset = 0;
-
-    physmap = get_physmapping(start_addr, size, page_mask);
-    if (physmap == NULL) {
-        return -1;
-    }
-
-    phys_offset = physmap->phys_offset;
-    size = physmap->size;
-
-    DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
-            "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
-
-    size >>= target_page_bits;
-    start_addr >>= target_page_bits;
-    phys_offset >>= target_page_bits;
-    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
-                                        phys_offset);
-    if (rc) {
-        int saved_errno = errno;
-
-        error_report("relocate_memory "RAM_ADDR_FMT" pages"
-                     " from GFN %"HWADDR_PRIx
-                     " to GFN %"HWADDR_PRIx" failed: %s",
-                     size, start_addr, phys_offset, strerror(saved_errno));
-        errno = saved_errno;
-        return -1;
-    }
-
-    QLIST_REMOVE(physmap, list);
-    if (log_for_dirtybit == physmap) {
-        log_for_dirtybit = NULL;
-        g_free(dirty_bitmap);
-        dirty_bitmap = NULL;
-    }
-    g_free(physmap);
-
-    return 0;
-}
-
-static void xen_sync_dirty_bitmap(XenIOState *state,
-                                  hwaddr start_addr,
-                                  ram_addr_t size)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    hwaddr npages = size >> target_page_bits;
-    const int width = sizeof(unsigned long) * 8;
-    size_t bitmap_size = DIV_ROUND_UP(npages, width);
-    int rc, i, j;
-    const XenPhysmap *physmap = NULL;
-
-    physmap = get_physmapping(start_addr, size, page_mask);
-    if (physmap == NULL) {
-        /* not handled */
-        return;
-    }
-
-    if (log_for_dirtybit == NULL) {
-        log_for_dirtybit = physmap;
-        dirty_bitmap = g_new(unsigned long, bitmap_size);
-    } else if (log_for_dirtybit != physmap) {
-        /* Only one range for dirty bitmap can be tracked. */
-        return;
-    }
-
-    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
-                              npages, dirty_bitmap);
-    if (rc < 0) {
-#ifndef ENODATA
-#define ENODATA  ENOENT
-#endif
-        if (errno == ENODATA) {
-            memory_region_set_dirty(framebuffer, 0, size);
-            DPRINTF("xen: track_dirty_vram failed (0x" HWADDR_FMT_plx
-                    ", 0x" HWADDR_FMT_plx "): %s\n",
-                    start_addr, start_addr + size, strerror(errno));
-        }
-        return;
-    }
-
-    for (i = 0; i < bitmap_size; i++) {
-        unsigned long map = dirty_bitmap[i];
-        while (map != 0) {
-            j = ctzl(map);
-            map &= ~(1ul << j);
-            memory_region_set_dirty(framebuffer,
-                                    (i * width + j) * page_size,
-                                    page_size);
-        };
-    }
-}
-
-static void xen_log_start(MemoryListener *listener,
-                          MemoryRegionSection *section,
-                          int old, int new)
-{
-    XenIOState *state = container_of(listener, XenIOState, memory_listener);
-
-    if (new & ~old & (1 << DIRTY_MEMORY_VGA)) {
-        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
-                              int128_get64(section->size));
-    }
-}
-
-static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
-                         int old, int new)
-{
-    if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
-        log_for_dirtybit = NULL;
-        g_free(dirty_bitmap);
-        dirty_bitmap = NULL;
-        /* Disable dirty bit tracking */
-        xen_track_dirty_vram(xen_domid, 0, 0, NULL);
-    }
-}
-
-static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
-{
-    XenIOState *state = container_of(listener, XenIOState, memory_listener);
-
-    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
-                          int128_get64(section->size));
-}
-
-static void xen_log_global_start(MemoryListener *listener)
-{
-    if (xen_enabled()) {
-        xen_in_migration = true;
-    }
-}
-
-static void xen_log_global_stop(MemoryListener *listener)
-{
-    xen_in_migration = false;
-}
-
-const MemoryListener xen_memory_listener = {
-    .name = "xen-memory",
-    .region_add = xen_region_add,
-    .region_del = xen_region_del,
-    .log_start = xen_log_start,
-    .log_stop = xen_log_stop,
-    .log_sync = xen_log_sync,
-    .log_global_start = xen_log_global_start,
-    .log_global_stop = xen_log_global_stop,
-    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
-};
-
-#ifdef XEN_COMPAT_PHYSMAP
-void xen_read_physmap(XenIOState *state)
-{
-    XenPhysmap *physmap = NULL;
-    unsigned int len, num, i;
-    char path[80], *value = NULL;
-    char **entries = NULL;
-
-    QLIST_INIT(&xen_physmap);
-
-    snprintf(path, sizeof(path),
-            "/local/domain/0/device-model/%d/physmap", xen_domid);
-    entries = xs_directory(state->xenstore, 0, path, &num);
-    if (entries == NULL)
-        return;
-
-    for (i = 0; i < num; i++) {
-        physmap = g_new(XenPhysmap, 1);
-        physmap->phys_offset = strtoull(entries[i], NULL, 16);
-        snprintf(path, sizeof(path),
-                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
-                xen_domid, entries[i]);
-        value = xs_read(state->xenstore, 0, path, &len);
-        if (value == NULL) {
-            g_free(physmap);
-            continue;
-        }
-        physmap->start_addr = strtoull(value, NULL, 16);
-        free(value);
-
-        snprintf(path, sizeof(path),
-                "/local/domain/0/device-model/%d/physmap/%s/size",
-                xen_domid, entries[i]);
-        value = xs_read(state->xenstore, 0, path, &len);
-        if (value == NULL) {
-            g_free(physmap);
-            continue;
-        }
-        physmap->size = strtoull(value, NULL, 16);
-        free(value);
-
-        snprintf(path, sizeof(path),
-                "/local/domain/0/device-model/%d/physmap/%s/name",
-                xen_domid, entries[i]);
-        physmap->name = xs_read(state->xenstore, 0, path, &len);
-
-        QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
-    }
-    free(entries);
-}
-#else
-void xen_read_physmap(XenIOState *state)
-{
-    QLIST_INIT(&xen_physmap);
-}
-#endif
-
-void xen_register_framebuffer(MemoryRegion *mr)
-{
-    framebuffer = mr;
-}
-
-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-
-    if (unlikely(xen_in_migration)) {
-        int rc;
-        ram_addr_t start_pfn, nb_pages;
-
-        start = xen_phys_offset_to_gaddr(start, length, page_mask);
-
-        if (length == 0) {
-            length = page_size;
-        }
-        start_pfn = start >> target_page_bits;
-        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
-            - start_pfn;
-        rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
-        if (rc) {
-            fprintf(stderr,
-                    "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n",
-                    __func__, start, nb_pages, errno, strerror(errno));
-        }
-    }
-}
-
-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
-{
-    if (enable) {
-        memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
-    } else {
-        memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
-    }
-}
-
-void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
-                         bool add)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    hwaddr start_addr = section->offset_within_address_space;
-    ram_addr_t size = int128_get64(section->size);
-    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
-    hvmmem_type_t mem_type;
-
-    if (!memory_region_is_ram(section->mr)) {
-        return;
-    }
-
-    if (log_dirty != add) {
-        return;
-    }
-
-    trace_xen_client_set_memory(start_addr, size, log_dirty);
-
-    start_addr &= page_mask;
-    size = ROUND_UP(size, page_size);
-
-    if (add) {
-        if (!memory_region_is_rom(section->mr)) {
-            xen_add_to_physmap(state, start_addr, size,
-                               section->mr, section->offset_within_region);
-        } else {
-            mem_type = HVMMEM_ram_ro;
-            if (xen_set_mem_type(xen_domid, mem_type,
-                                 start_addr >> target_page_bits,
-                                 size >> target_page_bits)) {
-                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
-                        start_addr);
-            }
-        }
-    } else {
-        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
-            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
-        }
-    }
-}
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 73fa2c414d..50ce45effc 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -1,7 +1,9 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qemu/bitops.h"
+#include "qemu/range.h"
 #include "qapi/error.h"
+#include "qapi/qapi-commands-migration.h"
 #include "exec/target_page.h"
 #include "trace.h"
 
@@ -11,6 +13,462 @@
 #include "hw/boards.h"
 
 MemoryRegion xen_memory;
+static MemoryRegion *framebuffer;
+static bool xen_in_migration;
+
+static QLIST_HEAD(, XenPhysmap) xen_physmap;
+static const XenPhysmap *log_for_dirtybit;
+/* Buffer used by xen_sync_dirty_bitmap */
+static unsigned long *dirty_bitmap;
+
+static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
+                                   int page_mask)
+{
+    XenPhysmap *physmap = NULL;
+
+    start_addr &= -page_mask;
+
+    QLIST_FOREACH(physmap, &xen_physmap, list) {
+        if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
+            return physmap;
+        }
+    }
+    return NULL;
+}
+
+static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
+                                       int page_mask)
+{
+    hwaddr addr = phys_offset & -page_mask;
+    XenPhysmap *physmap = NULL;
+
+    QLIST_FOREACH(physmap, &xen_physmap, list) {
+        if (range_covers_byte(physmap->phys_offset, physmap->size, addr)) {
+            return physmap->start_addr + (phys_offset - physmap->phys_offset);
+        }
+    }
+
+    return phys_offset;
+}
+
+#ifdef XEN_COMPAT_PHYSMAP
+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
+{
+    char path[80], value[17];
+
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
+            xen_domid, (uint64_t)physmap->phys_offset);
+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+        return -1;
+    }
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
+            xen_domid, (uint64_t)physmap->phys_offset);
+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+        return -1;
+    }
+    if (physmap->name) {
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
+                xen_domid, (uint64_t)physmap->phys_offset);
+        if (!xs_write(state->xenstore, 0, path,
+                      physmap->name, strlen(physmap->name))) {
+            return -1;
+        }
+    }
+    return 0;
+}
+#else
+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
+{
+    return 0;
+}
+#endif
+
+static int xen_add_to_physmap(XenIOState *state,
+                              hwaddr start_addr,
+                              ram_addr_t size,
+                              MemoryRegion *mr,
+                              hwaddr offset_within_region)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    unsigned long nr_pages;
+    int rc = 0;
+    XenPhysmap *physmap = NULL;
+    hwaddr pfn, start_gpfn;
+    hwaddr phys_offset = memory_region_get_ram_addr(mr);
+    const char *mr_name;
+
+    if (get_physmapping(start_addr, size, page_mask)) {
+        return 0;
+    }
+    if (size <= 0) {
+        return -1;
+    }
+
+    /* Xen can only handle a single dirty log region for now and we want
+     * the linear framebuffer to be that region.
+     * Avoid tracking any regions that is not videoram and avoid tracking
+     * the legacy vga region. */
+    if (mr == framebuffer && start_addr > 0xbffff) {
+        goto go_physmap;
+    }
+    return -1;
+
+go_physmap:
+    DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
+            start_addr, start_addr + size);
+
+    mr_name = memory_region_name(mr);
+
+    physmap = g_new(XenPhysmap, 1);
+
+    physmap->start_addr = start_addr;
+    physmap->size = size;
+    physmap->name = mr_name;
+    physmap->phys_offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
+
+    if (runstate_check(RUN_STATE_INMIGRATE)) {
+        /* Now when we have a physmap entry we can replace a dummy mapping with
+         * a real one of guest foreign memory. */
+        uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
+        assert(p && p == memory_region_get_ram_ptr(mr));
+
+        return 0;
+    }
+
+    pfn = phys_offset >> target_page_bits;
+    start_gpfn = start_addr >> target_page_bits;
+    nr_pages = size >> target_page_bits;
+    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
+                                        start_gpfn);
+    if (rc) {
+        int saved_errno = errno;
+
+        error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
+                     " to GFN %"HWADDR_PRIx" failed: %s",
+                     nr_pages, pfn, start_gpfn, strerror(saved_errno));
+        errno = saved_errno;
+        return -1;
+    }
+
+    rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
+                                   start_addr >> target_page_bits,
+                                   (start_addr + size - 1) >> target_page_bits,
+                                   XEN_DOMCTL_MEM_CACHEATTR_WB);
+    if (rc) {
+        error_report("pin_memory_cacheattr failed: %s", strerror(errno));
+    }
+    return xen_save_physmap(state, physmap);
+}
+
+static int xen_remove_from_physmap(XenIOState *state,
+                                   hwaddr start_addr,
+                                   ram_addr_t size)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    int rc = 0;
+    XenPhysmap *physmap = NULL;
+    hwaddr phys_offset = 0;
+
+    physmap = get_physmapping(start_addr, size, page_mask);
+    if (physmap == NULL) {
+        return -1;
+    }
+
+    phys_offset = physmap->phys_offset;
+    size = physmap->size;
+
+    DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
+            "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
+
+    size >>= target_page_bits;
+    start_addr >>= target_page_bits;
+    phys_offset >>= target_page_bits;
+    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
+                                        phys_offset);
+    if (rc) {
+        int saved_errno = errno;
+
+        error_report("relocate_memory "RAM_ADDR_FMT" pages"
+                     " from GFN %"HWADDR_PRIx
+                     " to GFN %"HWADDR_PRIx" failed: %s",
+                     size, start_addr, phys_offset, strerror(saved_errno));
+        errno = saved_errno;
+        return -1;
+    }
+
+    QLIST_REMOVE(physmap, list);
+    if (log_for_dirtybit == physmap) {
+        log_for_dirtybit = NULL;
+        g_free(dirty_bitmap);
+        dirty_bitmap = NULL;
+    }
+    g_free(physmap);
+
+    return 0;
+}
+
+static void xen_sync_dirty_bitmap(XenIOState *state,
+                                  hwaddr start_addr,
+                                  ram_addr_t size)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    hwaddr npages = size >> target_page_bits;
+    const int width = sizeof(unsigned long) * 8;
+    size_t bitmap_size = DIV_ROUND_UP(npages, width);
+    int rc, i, j;
+    const XenPhysmap *physmap = NULL;
+
+    physmap = get_physmapping(start_addr, size, page_mask);
+    if (physmap == NULL) {
+        /* not handled */
+        return;
+    }
+
+    if (log_for_dirtybit == NULL) {
+        log_for_dirtybit = physmap;
+        dirty_bitmap = g_new(unsigned long, bitmap_size);
+    } else if (log_for_dirtybit != physmap) {
+        /* Only one range for dirty bitmap can be tracked. */
+        return;
+    }
+
+    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
+                              npages, dirty_bitmap);
+    if (rc < 0) {
+#ifndef ENODATA
+#define ENODATA  ENOENT
+#endif
+        if (errno == ENODATA) {
+            memory_region_set_dirty(framebuffer, 0, size);
+            DPRINTF("xen: track_dirty_vram failed (0x" HWADDR_FMT_plx
+                    ", 0x" HWADDR_FMT_plx "): %s\n",
+                    start_addr, start_addr + size, strerror(errno));
+        }
+        return;
+    }
+
+    for (i = 0; i < bitmap_size; i++) {
+        unsigned long map = dirty_bitmap[i];
+        while (map != 0) {
+            j = ctzl(map);
+            map &= ~(1ul << j);
+            memory_region_set_dirty(framebuffer,
+                                    (i * width + j) * page_size,
+                                    page_size);
+        };
+    }
+}
+
+static void xen_log_start(MemoryListener *listener,
+                          MemoryRegionSection *section,
+                          int old, int new)
+{
+    XenIOState *state = container_of(listener, XenIOState, memory_listener);
+
+    if (new & ~old & (1 << DIRTY_MEMORY_VGA)) {
+        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
+                              int128_get64(section->size));
+    }
+}
+
+static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
+                         int old, int new)
+{
+    if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
+        log_for_dirtybit = NULL;
+        g_free(dirty_bitmap);
+        dirty_bitmap = NULL;
+        /* Disable dirty bit tracking */
+        xen_track_dirty_vram(xen_domid, 0, 0, NULL);
+    }
+}
+
+static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
+{
+    XenIOState *state = container_of(listener, XenIOState, memory_listener);
+
+    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
+                          int128_get64(section->size));
+}
+
+static void xen_log_global_start(MemoryListener *listener)
+{
+    if (xen_enabled()) {
+        xen_in_migration = true;
+    }
+}
+
+static void xen_log_global_stop(MemoryListener *listener)
+{
+    xen_in_migration = false;
+}
+
+const MemoryListener xen_memory_listener = {
+    .name = "xen-memory",
+    .region_add = xen_region_add,
+    .region_del = xen_region_del,
+    .log_start = xen_log_start,
+    .log_stop = xen_log_stop,
+    .log_sync = xen_log_sync,
+    .log_global_start = xen_log_global_start,
+    .log_global_stop = xen_log_global_stop,
+    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
+};
+
+#ifdef XEN_COMPAT_PHYSMAP
+void xen_read_physmap(XenIOState *state)
+{
+    XenPhysmap *physmap = NULL;
+    unsigned int len, num, i;
+    char path[80], *value = NULL;
+    char **entries = NULL;
+
+    QLIST_INIT(&xen_physmap);
+
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap", xen_domid);
+    entries = xs_directory(state->xenstore, 0, path, &num);
+    if (entries == NULL)
+        return;
+
+    for (i = 0; i < num; i++) {
+        physmap = g_new(XenPhysmap, 1);
+        physmap->phys_offset = strtoull(entries[i], NULL, 16);
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
+                xen_domid, entries[i]);
+        value = xs_read(state->xenstore, 0, path, &len);
+        if (value == NULL) {
+            g_free(physmap);
+            continue;
+        }
+        physmap->start_addr = strtoull(value, NULL, 16);
+        free(value);
+
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%s/size",
+                xen_domid, entries[i]);
+        value = xs_read(state->xenstore, 0, path, &len);
+        if (value == NULL) {
+            g_free(physmap);
+            continue;
+        }
+        physmap->size = strtoull(value, NULL, 16);
+        free(value);
+
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%s/name",
+                xen_domid, entries[i]);
+        physmap->name = xs_read(state->xenstore, 0, path, &len);
+
+        QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
+    }
+    free(entries);
+}
+#else
+void xen_read_physmap(XenIOState *state)
+{
+    QLIST_INIT(&xen_physmap);
+}
+#endif
+
+void xen_register_framebuffer(MemoryRegion *mr)
+{
+    framebuffer = mr;
+}
+
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+
+    if (unlikely(xen_in_migration)) {
+        int rc;
+        ram_addr_t start_pfn, nb_pages;
+
+        start = xen_phys_offset_to_gaddr(start, length, page_mask);
+
+        if (length == 0) {
+            length = page_size;
+        }
+        start_pfn = start >> target_page_bits;
+        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
+            - start_pfn;
+        rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
+        if (rc) {
+            fprintf(stderr,
+                    "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n",
+                    __func__, start, nb_pages, errno, strerror(errno));
+        }
+    }
+}
+
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+    if (enable) {
+        memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
+    } else {
+        memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
+    }
+}
+
+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
+                         bool add)
+{
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    hwaddr start_addr = section->offset_within_address_space;
+    ram_addr_t size = int128_get64(section->size);
+    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
+    hvmmem_type_t mem_type;
+
+    if (!memory_region_is_ram(section->mr)) {
+        return;
+    }
+
+    if (log_dirty != add) {
+        return;
+    }
+
+    trace_xen_client_set_memory(start_addr, size, log_dirty);
+
+    start_addr &= page_mask;
+    size = ROUND_UP(size, page_size);
+
+    if (add) {
+        if (!memory_region_is_rom(section->mr)) {
+            xen_add_to_physmap(state, start_addr, size,
+                               section->mr, section->offset_within_region);
+        } else {
+            mem_type = HVMMEM_ram_ro;
+            if (xen_set_mem_type(xen_domid, mem_type,
+                                 start_addr >> target_page_bits,
+                                 size >> target_page_bits)) {
+                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
+                        start_addr);
+            }
+        }
+    } else {
+        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
+            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
+        }
+    }
+}
 
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
                    Error **errp)
diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
index 53c6a6f2a0..da8912f037 100644
--- a/stubs/xen-hw-stub.c
+++ b/stubs/xen-hw-stub.c
@@ -13,7 +13,3 @@ int xen_is_pirq_msi(uint32_t msi_data)
 {
     return 0;
 }
-
-void xen_register_framebuffer(MemoryRegion *mr)
-{
-}
diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index d38759cfe4..3f0df8bc07 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -1,7 +1,6 @@
 i386_ss.add(when: 'CONFIG_XEN', if_true: files(
   'xen_apic.c',
   'xen_pvdevice.c',
-  'xen-hvm-common.c',
 ))
 i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
   'xen-hvm.c',
-- 
2.41.0

Re: [RFC PATCH-for-9.0 8/9] hw/xen/hvm: Merge xen-hvm-common.c files

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>hw/i386/xen/xen-hvm-common.c content is target agnostic,
>and should be common to all targets. Merge both files.
>Remove the now unnecessary xen_register_framebuffer() stub.
>
>ARM targets now inherit the common xen_memory_listener.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/arm/xen_arm.c             |  24 --
> hw/i386/xen/xen-hvm-common.c | 473 -----------------------------------
> hw/xen/xen-hvm-common.c      | 458 +++++++++++++++++++++++++++++++++
> stubs/xen-hw-stub.c          |   4 -
> hw/i386/xen/meson.build      |   1 -
> 5 files changed, 458 insertions(+), 502 deletions(-)
> delete mode 100644 hw/i386/xen/xen-hvm-common.c
>
>diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
>index 39dcd74d07..0ead84c9e1 100644
>--- a/hw/arm/xen_arm.c
>+++ b/hw/arm/xen_arm.c
>@@ -38,17 +38,6 @@
> #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
> OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
> 
>-const MemoryListener xen_memory_listener = {
>-    .region_add = xen_region_add,
>-    .region_del = xen_region_del,
>-    .log_start = NULL,
>-    .log_stop = NULL,
>-    .log_sync = NULL,
>-    .log_global_start = NULL,
>-    .log_global_stop = NULL,
>-    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
>-};
>-
> struct XenArmState {
>     /*< private >*/
>     MachineState parent;
>@@ -136,19 +125,6 @@ void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
>     return;
> }
> 
>-void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>-                         bool add)
>-{
>-}
>-
>-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
>-{
>-}
>-
>-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
>-{
>-}


I think this is not correct, Xen on arm does not do migration and does 
not handle memory (the hardware does). We should keep the stubs here.

And not merge the physmapping/dirty mapping of i386 to the common code.

.. which I guess means xen_memory_listener cannot be shared either. 
Unless it's non-const and i386 code sets the log_* callback fields on 
init.

>-
> #ifdef CONFIG_TPM
> static void xen_enable_tpm(XenArmState *xam)
> {
>diff --git a/hw/i386/xen/xen-hvm-common.c b/hw/i386/xen/xen-hvm-common.c
>deleted file mode 100644
>index e8ef0e0c94..0000000000
>--- a/hw/i386/xen/xen-hvm-common.c
>+++ /dev/null
>@@ -1,473 +0,0 @@
>-/*
>- * Copyright (C) 2010       Citrix Ltd.
>- *
>- * This work is licensed under the terms of the GNU GPL, version 2.  See
>- * the COPYING file in the top-level directory.
>- *
>- * Contributions after 2012-01-13 are licensed under the terms of the
>- * GNU GPL, version 2 or (at your option) any later version.
>- */
>-
>-#include "qemu/osdep.h"
>-#include "qemu/range.h"
>-#include "qapi/qapi-commands-migration.h"
>-#include "exec/target_page.h"
>-#include "hw/xen/xen-hvm-common.h"
>-#include "trace.h"
>-
>-static MemoryRegion *framebuffer;
>-static bool xen_in_migration;
>-
>-static QLIST_HEAD(, XenPhysmap) xen_physmap;
>-static const XenPhysmap *log_for_dirtybit;
>-/* Buffer used by xen_sync_dirty_bitmap */
>-static unsigned long *dirty_bitmap;
>-
>-static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
>-                                   int page_mask)
>-{
>-    XenPhysmap *physmap = NULL;
>-
>-    start_addr &= -page_mask;
>-
>-    QLIST_FOREACH(physmap, &xen_physmap, list) {
>-        if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
>-            return physmap;
>-        }
>-    }
>-    return NULL;
>-}
>-
>-static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
>-                                       int page_mask)
>-{
>-    hwaddr addr = phys_offset & -page_mask;
>-    XenPhysmap *physmap = NULL;
>-
>-    QLIST_FOREACH(physmap, &xen_physmap, list) {
>-        if (range_covers_byte(physmap->phys_offset, physmap->size, addr)) {
>-            return physmap->start_addr + (phys_offset - physmap->phys_offset);
>-        }
>-    }
>-
>-    return phys_offset;
>-}
>-
>-#ifdef XEN_COMPAT_PHYSMAP
>-static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
>-{
>-    char path[80], value[17];
>-
>-    snprintf(path, sizeof(path),
>-            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
>-            xen_domid, (uint64_t)physmap->phys_offset);
>-    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
>-    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>-        return -1;
>-    }
>-    snprintf(path, sizeof(path),
>-            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
>-            xen_domid, (uint64_t)physmap->phys_offset);
>-    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
>-    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>-        return -1;
>-    }
>-    if (physmap->name) {
>-        snprintf(path, sizeof(path),
>-                "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
>-                xen_domid, (uint64_t)physmap->phys_offset);
>-        if (!xs_write(state->xenstore, 0, path,
>-                      physmap->name, strlen(physmap->name))) {
>-            return -1;
>-        }
>-    }
>-    return 0;
>-}
>-#else
>-static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
>-{
>-    return 0;
>-}
>-#endif
>-
>-static int xen_add_to_physmap(XenIOState *state,
>-                              hwaddr start_addr,
>-                              ram_addr_t size,
>-                              MemoryRegion *mr,
>-                              hwaddr offset_within_region)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    unsigned long nr_pages;
>-    int rc = 0;
>-    XenPhysmap *physmap = NULL;
>-    hwaddr pfn, start_gpfn;
>-    hwaddr phys_offset = memory_region_get_ram_addr(mr);
>-    const char *mr_name;
>-
>-    if (get_physmapping(start_addr, size, page_mask)) {
>-        return 0;
>-    }
>-    if (size <= 0) {
>-        return -1;
>-    }
>-
>-    /* Xen can only handle a single dirty log region for now and we want
>-     * the linear framebuffer to be that region.
>-     * Avoid tracking any regions that is not videoram and avoid tracking
>-     * the legacy vga region. */
>-    if (mr == framebuffer && start_addr > 0xbffff) {
>-        goto go_physmap;
>-    }
>-    return -1;
>-
>-go_physmap:
>-    DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
>-            start_addr, start_addr + size);
>-
>-    mr_name = memory_region_name(mr);
>-
>-    physmap = g_new(XenPhysmap, 1);
>-
>-    physmap->start_addr = start_addr;
>-    physmap->size = size;
>-    physmap->name = mr_name;
>-    physmap->phys_offset = phys_offset;
>-
>-    QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
>-
>-    if (runstate_check(RUN_STATE_INMIGRATE)) {
>-        /* Now when we have a physmap entry we can replace a dummy mapping with
>-         * a real one of guest foreign memory. */
>-        uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
>-        assert(p && p == memory_region_get_ram_ptr(mr));
>-
>-        return 0;
>-    }
>-
>-    pfn = phys_offset >> target_page_bits;
>-    start_gpfn = start_addr >> target_page_bits;
>-    nr_pages = size >> target_page_bits;
>-    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
>-                                        start_gpfn);
>-    if (rc) {
>-        int saved_errno = errno;
>-
>-        error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
>-                     " to GFN %"HWADDR_PRIx" failed: %s",
>-                     nr_pages, pfn, start_gpfn, strerror(saved_errno));
>-        errno = saved_errno;
>-        return -1;
>-    }
>-
>-    rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
>-                                   start_addr >> target_page_bits,
>-                                   (start_addr + size - 1) >> target_page_bits,
>-                                   XEN_DOMCTL_MEM_CACHEATTR_WB);
>-    if (rc) {
>-        error_report("pin_memory_cacheattr failed: %s", strerror(errno));
>-    }
>-    return xen_save_physmap(state, physmap);
>-}
>-
>-static int xen_remove_from_physmap(XenIOState *state,
>-                                   hwaddr start_addr,
>-                                   ram_addr_t size)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    int rc = 0;
>-    XenPhysmap *physmap = NULL;
>-    hwaddr phys_offset = 0;
>-
>-    physmap = get_physmapping(start_addr, size, page_mask);
>-    if (physmap == NULL) {
>-        return -1;
>-    }
>-
>-    phys_offset = physmap->phys_offset;
>-    size = physmap->size;
>-
>-    DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
>-            "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
>-
>-    size >>= target_page_bits;
>-    start_addr >>= target_page_bits;
>-    phys_offset >>= target_page_bits;
>-    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
>-                                        phys_offset);
>-    if (rc) {
>-        int saved_errno = errno;
>-
>-        error_report("relocate_memory "RAM_ADDR_FMT" pages"
>-                     " from GFN %"HWADDR_PRIx
>-                     " to GFN %"HWADDR_PRIx" failed: %s",
>-                     size, start_addr, phys_offset, strerror(saved_errno));
>-        errno = saved_errno;
>-        return -1;
>-    }
>-
>-    QLIST_REMOVE(physmap, list);
>-    if (log_for_dirtybit == physmap) {
>-        log_for_dirtybit = NULL;
>-        g_free(dirty_bitmap);
>-        dirty_bitmap = NULL;
>-    }
>-    g_free(physmap);
>-
>-    return 0;
>-}
>-
>-static void xen_sync_dirty_bitmap(XenIOState *state,
>-                                  hwaddr start_addr,
>-                                  ram_addr_t size)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    hwaddr npages = size >> target_page_bits;
>-    const int width = sizeof(unsigned long) * 8;
>-    size_t bitmap_size = DIV_ROUND_UP(npages, width);
>-    int rc, i, j;
>-    const XenPhysmap *physmap = NULL;
>-
>-    physmap = get_physmapping(start_addr, size, page_mask);
>-    if (physmap == NULL) {
>-        /* not handled */
>-        return;
>-    }
>-
>-    if (log_for_dirtybit == NULL) {
>-        log_for_dirtybit = physmap;
>-        dirty_bitmap = g_new(unsigned long, bitmap_size);
>-    } else if (log_for_dirtybit != physmap) {
>-        /* Only one range for dirty bitmap can be tracked. */
>-        return;
>-    }
>-
>-    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
>-                              npages, dirty_bitmap);
>-    if (rc < 0) {
>-#ifndef ENODATA
>-#define ENODATA  ENOENT
>-#endif
>-        if (errno == ENODATA) {
>-            memory_region_set_dirty(framebuffer, 0, size);
>-            DPRINTF("xen: track_dirty_vram failed (0x" HWADDR_FMT_plx
>-                    ", 0x" HWADDR_FMT_plx "): %s\n",
>-                    start_addr, start_addr + size, strerror(errno));
>-        }
>-        return;
>-    }
>-
>-    for (i = 0; i < bitmap_size; i++) {
>-        unsigned long map = dirty_bitmap[i];
>-        while (map != 0) {
>-            j = ctzl(map);
>-            map &= ~(1ul << j);
>-            memory_region_set_dirty(framebuffer,
>-                                    (i * width + j) * page_size,
>-                                    page_size);
>-        };
>-    }
>-}
>-
>-static void xen_log_start(MemoryListener *listener,
>-                          MemoryRegionSection *section,
>-                          int old, int new)
>-{
>-    XenIOState *state = container_of(listener, XenIOState, memory_listener);
>-
>-    if (new & ~old & (1 << DIRTY_MEMORY_VGA)) {
>-        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
>-                              int128_get64(section->size));
>-    }
>-}
>-
>-static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
>-                         int old, int new)
>-{
>-    if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
>-        log_for_dirtybit = NULL;
>-        g_free(dirty_bitmap);
>-        dirty_bitmap = NULL;
>-        /* Disable dirty bit tracking */
>-        xen_track_dirty_vram(xen_domid, 0, 0, NULL);
>-    }
>-}
>-
>-static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
>-{
>-    XenIOState *state = container_of(listener, XenIOState, memory_listener);
>-
>-    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
>-                          int128_get64(section->size));
>-}
>-
>-static void xen_log_global_start(MemoryListener *listener)
>-{
>-    if (xen_enabled()) {
>-        xen_in_migration = true;
>-    }
>-}
>-
>-static void xen_log_global_stop(MemoryListener *listener)
>-{
>-    xen_in_migration = false;
>-}
>-
>-const MemoryListener xen_memory_listener = {
>-    .name = "xen-memory",
>-    .region_add = xen_region_add,
>-    .region_del = xen_region_del,
>-    .log_start = xen_log_start,
>-    .log_stop = xen_log_stop,
>-    .log_sync = xen_log_sync,
>-    .log_global_start = xen_log_global_start,
>-    .log_global_stop = xen_log_global_stop,
>-    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
>-};
>-
>-#ifdef XEN_COMPAT_PHYSMAP
>-void xen_read_physmap(XenIOState *state)
>-{
>-    XenPhysmap *physmap = NULL;
>-    unsigned int len, num, i;
>-    char path[80], *value = NULL;
>-    char **entries = NULL;
>-
>-    QLIST_INIT(&xen_physmap);
>-
>-    snprintf(path, sizeof(path),
>-            "/local/domain/0/device-model/%d/physmap", xen_domid);
>-    entries = xs_directory(state->xenstore, 0, path, &num);
>-    if (entries == NULL)
>-        return;
>-
>-    for (i = 0; i < num; i++) {
>-        physmap = g_new(XenPhysmap, 1);
>-        physmap->phys_offset = strtoull(entries[i], NULL, 16);
>-        snprintf(path, sizeof(path),
>-                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
>-                xen_domid, entries[i]);
>-        value = xs_read(state->xenstore, 0, path, &len);
>-        if (value == NULL) {
>-            g_free(physmap);
>-            continue;
>-        }
>-        physmap->start_addr = strtoull(value, NULL, 16);
>-        free(value);
>-
>-        snprintf(path, sizeof(path),
>-                "/local/domain/0/device-model/%d/physmap/%s/size",
>-                xen_domid, entries[i]);
>-        value = xs_read(state->xenstore, 0, path, &len);
>-        if (value == NULL) {
>-            g_free(physmap);
>-            continue;
>-        }
>-        physmap->size = strtoull(value, NULL, 16);
>-        free(value);
>-
>-        snprintf(path, sizeof(path),
>-                "/local/domain/0/device-model/%d/physmap/%s/name",
>-                xen_domid, entries[i]);
>-        physmap->name = xs_read(state->xenstore, 0, path, &len);
>-
>-        QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
>-    }
>-    free(entries);
>-}
>-#else
>-void xen_read_physmap(XenIOState *state)
>-{
>-    QLIST_INIT(&xen_physmap);
>-}
>-#endif
>-
>-void xen_register_framebuffer(MemoryRegion *mr)
>-{
>-    framebuffer = mr;
>-}
>-
>-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-
>-    if (unlikely(xen_in_migration)) {
>-        int rc;
>-        ram_addr_t start_pfn, nb_pages;
>-
>-        start = xen_phys_offset_to_gaddr(start, length, page_mask);
>-
>-        if (length == 0) {
>-            length = page_size;
>-        }
>-        start_pfn = start >> target_page_bits;
>-        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
>-            - start_pfn;
>-        rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
>-        if (rc) {
>-            fprintf(stderr,
>-                    "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n",
>-                    __func__, start, nb_pages, errno, strerror(errno));
>-        }
>-    }
>-}
>-
>-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
>-{
>-    if (enable) {
>-        memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
>-    } else {
>-        memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
>-    }
>-}
>-
>-void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>-                         bool add)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    hwaddr start_addr = section->offset_within_address_space;
>-    ram_addr_t size = int128_get64(section->size);
>-    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
>-    hvmmem_type_t mem_type;
>-
>-    if (!memory_region_is_ram(section->mr)) {
>-        return;
>-    }
>-
>-    if (log_dirty != add) {
>-        return;
>-    }
>-
>-    trace_xen_client_set_memory(start_addr, size, log_dirty);
>-
>-    start_addr &= page_mask;
>-    size = ROUND_UP(size, page_size);
>-
>-    if (add) {
>-        if (!memory_region_is_rom(section->mr)) {
>-            xen_add_to_physmap(state, start_addr, size,
>-                               section->mr, section->offset_within_region);
>-        } else {
>-            mem_type = HVMMEM_ram_ro;
>-            if (xen_set_mem_type(xen_domid, mem_type,
>-                                 start_addr >> target_page_bits,
>-                                 size >> target_page_bits)) {
>-                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
>-                        start_addr);
>-            }
>-        }
>-    } else {
>-        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
>-            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
>-        }
>-    }
>-}
>diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
>index 73fa2c414d..50ce45effc 100644
>--- a/hw/xen/xen-hvm-common.c
>+++ b/hw/xen/xen-hvm-common.c
>@@ -1,7 +1,9 @@
> #include "qemu/osdep.h"
> #include "qemu/units.h"
> #include "qemu/bitops.h"
>+#include "qemu/range.h"
> #include "qapi/error.h"
>+#include "qapi/qapi-commands-migration.h"
> #include "exec/target_page.h"
> #include "trace.h"
> 
>@@ -11,6 +13,462 @@
> #include "hw/boards.h"
> 
> MemoryRegion xen_memory;
>+static MemoryRegion *framebuffer;
>+static bool xen_in_migration;
>+
>+static QLIST_HEAD(, XenPhysmap) xen_physmap;
>+static const XenPhysmap *log_for_dirtybit;
>+/* Buffer used by xen_sync_dirty_bitmap */
>+static unsigned long *dirty_bitmap;
>+
>+static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
>+                                   int page_mask)
>+{
>+    XenPhysmap *physmap = NULL;
>+
>+    start_addr &= -page_mask;
>+
>+    QLIST_FOREACH(physmap, &xen_physmap, list) {
>+        if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) {
>+            return physmap;
>+        }
>+    }
>+    return NULL;
>+}
>+
>+static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
>+                                       int page_mask)
>+{
>+    hwaddr addr = phys_offset & -page_mask;
>+    XenPhysmap *physmap = NULL;
>+
>+    QLIST_FOREACH(physmap, &xen_physmap, list) {
>+        if (range_covers_byte(physmap->phys_offset, physmap->size, addr)) {
>+            return physmap->start_addr + (phys_offset - physmap->phys_offset);
>+        }
>+    }
>+
>+    return phys_offset;
>+}
>+
>+#ifdef XEN_COMPAT_PHYSMAP
>+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
>+{
>+    char path[80], value[17];
>+
>+    snprintf(path, sizeof(path),
>+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
>+            xen_domid, (uint64_t)physmap->phys_offset);
>+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
>+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>+        return -1;
>+    }
>+    snprintf(path, sizeof(path),
>+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
>+            xen_domid, (uint64_t)physmap->phys_offset);
>+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
>+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>+        return -1;
>+    }
>+    if (physmap->name) {
>+        snprintf(path, sizeof(path),
>+                "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
>+                xen_domid, (uint64_t)physmap->phys_offset);
>+        if (!xs_write(state->xenstore, 0, path,
>+                      physmap->name, strlen(physmap->name))) {
>+            return -1;
>+        }
>+    }
>+    return 0;
>+}
>+#else
>+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
>+{
>+    return 0;
>+}
>+#endif
>+
>+static int xen_add_to_physmap(XenIOState *state,
>+                              hwaddr start_addr,
>+                              ram_addr_t size,
>+                              MemoryRegion *mr,
>+                              hwaddr offset_within_region)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    unsigned long nr_pages;
>+    int rc = 0;
>+    XenPhysmap *physmap = NULL;
>+    hwaddr pfn, start_gpfn;
>+    hwaddr phys_offset = memory_region_get_ram_addr(mr);
>+    const char *mr_name;
>+
>+    if (get_physmapping(start_addr, size, page_mask)) {
>+        return 0;
>+    }
>+    if (size <= 0) {
>+        return -1;
>+    }
>+
>+    /* Xen can only handle a single dirty log region for now and we want
>+     * the linear framebuffer to be that region.
>+     * Avoid tracking any regions that is not videoram and avoid tracking
>+     * the legacy vga region. */
>+    if (mr == framebuffer && start_addr > 0xbffff) {
>+        goto go_physmap;
>+    }
>+    return -1;
>+
>+go_physmap:
>+    DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
>+            start_addr, start_addr + size);
>+
>+    mr_name = memory_region_name(mr);
>+
>+    physmap = g_new(XenPhysmap, 1);
>+
>+    physmap->start_addr = start_addr;
>+    physmap->size = size;
>+    physmap->name = mr_name;
>+    physmap->phys_offset = phys_offset;
>+
>+    QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
>+
>+    if (runstate_check(RUN_STATE_INMIGRATE)) {
>+        /* Now when we have a physmap entry we can replace a dummy mapping with
>+         * a real one of guest foreign memory. */
>+        uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
>+        assert(p && p == memory_region_get_ram_ptr(mr));
>+
>+        return 0;
>+    }
>+
>+    pfn = phys_offset >> target_page_bits;
>+    start_gpfn = start_addr >> target_page_bits;
>+    nr_pages = size >> target_page_bits;
>+    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
>+                                        start_gpfn);
>+    if (rc) {
>+        int saved_errno = errno;
>+
>+        error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
>+                     " to GFN %"HWADDR_PRIx" failed: %s",
>+                     nr_pages, pfn, start_gpfn, strerror(saved_errno));
>+        errno = saved_errno;
>+        return -1;
>+    }
>+
>+    rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
>+                                   start_addr >> target_page_bits,
>+                                   (start_addr + size - 1) >> target_page_bits,
>+                                   XEN_DOMCTL_MEM_CACHEATTR_WB);
>+    if (rc) {
>+        error_report("pin_memory_cacheattr failed: %s", strerror(errno));
>+    }
>+    return xen_save_physmap(state, physmap);
>+}
>+
>+static int xen_remove_from_physmap(XenIOState *state,
>+                                   hwaddr start_addr,
>+                                   ram_addr_t size)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    int rc = 0;
>+    XenPhysmap *physmap = NULL;
>+    hwaddr phys_offset = 0;
>+
>+    physmap = get_physmapping(start_addr, size, page_mask);
>+    if (physmap == NULL) {
>+        return -1;
>+    }
>+
>+    phys_offset = physmap->phys_offset;
>+    size = physmap->size;
>+
>+    DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
>+            "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
>+
>+    size >>= target_page_bits;
>+    start_addr >>= target_page_bits;
>+    phys_offset >>= target_page_bits;
>+    rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
>+                                        phys_offset);
>+    if (rc) {
>+        int saved_errno = errno;
>+
>+        error_report("relocate_memory "RAM_ADDR_FMT" pages"
>+                     " from GFN %"HWADDR_PRIx
>+                     " to GFN %"HWADDR_PRIx" failed: %s",
>+                     size, start_addr, phys_offset, strerror(saved_errno));
>+        errno = saved_errno;
>+        return -1;
>+    }
>+
>+    QLIST_REMOVE(physmap, list);
>+    if (log_for_dirtybit == physmap) {
>+        log_for_dirtybit = NULL;
>+        g_free(dirty_bitmap);
>+        dirty_bitmap = NULL;
>+    }
>+    g_free(physmap);
>+
>+    return 0;
>+}
>+
>+static void xen_sync_dirty_bitmap(XenIOState *state,
>+                                  hwaddr start_addr,
>+                                  ram_addr_t size)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    hwaddr npages = size >> target_page_bits;
>+    const int width = sizeof(unsigned long) * 8;
>+    size_t bitmap_size = DIV_ROUND_UP(npages, width);
>+    int rc, i, j;
>+    const XenPhysmap *physmap = NULL;
>+
>+    physmap = get_physmapping(start_addr, size, page_mask);
>+    if (physmap == NULL) {
>+        /* not handled */
>+        return;
>+    }
>+
>+    if (log_for_dirtybit == NULL) {
>+        log_for_dirtybit = physmap;
>+        dirty_bitmap = g_new(unsigned long, bitmap_size);
>+    } else if (log_for_dirtybit != physmap) {
>+        /* Only one range for dirty bitmap can be tracked. */
>+        return;
>+    }
>+
>+    rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
>+                              npages, dirty_bitmap);
>+    if (rc < 0) {
>+#ifndef ENODATA
>+#define ENODATA  ENOENT
>+#endif
>+        if (errno == ENODATA) {
>+            memory_region_set_dirty(framebuffer, 0, size);
>+            DPRINTF("xen: track_dirty_vram failed (0x" HWADDR_FMT_plx
>+                    ", 0x" HWADDR_FMT_plx "): %s\n",
>+                    start_addr, start_addr + size, strerror(errno));
>+        }
>+        return;
>+    }
>+
>+    for (i = 0; i < bitmap_size; i++) {
>+        unsigned long map = dirty_bitmap[i];
>+        while (map != 0) {
>+            j = ctzl(map);
>+            map &= ~(1ul << j);
>+            memory_region_set_dirty(framebuffer,
>+                                    (i * width + j) * page_size,
>+                                    page_size);
>+        };
>+    }
>+}
>+
>+static void xen_log_start(MemoryListener *listener,
>+                          MemoryRegionSection *section,
>+                          int old, int new)
>+{
>+    XenIOState *state = container_of(listener, XenIOState, memory_listener);
>+
>+    if (new & ~old & (1 << DIRTY_MEMORY_VGA)) {
>+        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
>+                              int128_get64(section->size));
>+    }
>+}
>+
>+static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
>+                         int old, int new)
>+{
>+    if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
>+        log_for_dirtybit = NULL;
>+        g_free(dirty_bitmap);
>+        dirty_bitmap = NULL;
>+        /* Disable dirty bit tracking */
>+        xen_track_dirty_vram(xen_domid, 0, 0, NULL);
>+    }
>+}
>+
>+static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
>+{
>+    XenIOState *state = container_of(listener, XenIOState, memory_listener);
>+
>+    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
>+                          int128_get64(section->size));
>+}
>+
>+static void xen_log_global_start(MemoryListener *listener)
>+{
>+    if (xen_enabled()) {
>+        xen_in_migration = true;
>+    }
>+}
>+
>+static void xen_log_global_stop(MemoryListener *listener)
>+{
>+    xen_in_migration = false;
>+}
>+
>+const MemoryListener xen_memory_listener = {
>+    .name = "xen-memory",
>+    .region_add = xen_region_add,
>+    .region_del = xen_region_del,
>+    .log_start = xen_log_start,
>+    .log_stop = xen_log_stop,
>+    .log_sync = xen_log_sync,
>+    .log_global_start = xen_log_global_start,
>+    .log_global_stop = xen_log_global_stop,
>+    .priority = MEMORY_LISTENER_PRIORITY_ACCEL,
>+};
>+
>+#ifdef XEN_COMPAT_PHYSMAP
>+void xen_read_physmap(XenIOState *state)
>+{
>+    XenPhysmap *physmap = NULL;
>+    unsigned int len, num, i;
>+    char path[80], *value = NULL;
>+    char **entries = NULL;
>+
>+    QLIST_INIT(&xen_physmap);
>+
>+    snprintf(path, sizeof(path),
>+            "/local/domain/0/device-model/%d/physmap", xen_domid);
>+    entries = xs_directory(state->xenstore, 0, path, &num);
>+    if (entries == NULL)
>+        return;
>+
>+    for (i = 0; i < num; i++) {
>+        physmap = g_new(XenPhysmap, 1);
>+        physmap->phys_offset = strtoull(entries[i], NULL, 16);
>+        snprintf(path, sizeof(path),
>+                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
>+                xen_domid, entries[i]);
>+        value = xs_read(state->xenstore, 0, path, &len);
>+        if (value == NULL) {
>+            g_free(physmap);
>+            continue;
>+        }
>+        physmap->start_addr = strtoull(value, NULL, 16);
>+        free(value);
>+
>+        snprintf(path, sizeof(path),
>+                "/local/domain/0/device-model/%d/physmap/%s/size",
>+                xen_domid, entries[i]);
>+        value = xs_read(state->xenstore, 0, path, &len);
>+        if (value == NULL) {
>+            g_free(physmap);
>+            continue;
>+        }
>+        physmap->size = strtoull(value, NULL, 16);
>+        free(value);
>+
>+        snprintf(path, sizeof(path),
>+                "/local/domain/0/device-model/%d/physmap/%s/name",
>+                xen_domid, entries[i]);
>+        physmap->name = xs_read(state->xenstore, 0, path, &len);
>+
>+        QLIST_INSERT_HEAD(&xen_physmap, physmap, list);
>+    }
>+    free(entries);
>+}
>+#else
>+void xen_read_physmap(XenIOState *state)
>+{
>+    QLIST_INIT(&xen_physmap);
>+}
>+#endif
>+
>+void xen_register_framebuffer(MemoryRegion *mr)
>+{
>+    framebuffer = mr;
>+}
>+
>+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+
>+    if (unlikely(xen_in_migration)) {
>+        int rc;
>+        ram_addr_t start_pfn, nb_pages;
>+
>+        start = xen_phys_offset_to_gaddr(start, length, page_mask);
>+
>+        if (length == 0) {
>+            length = page_size;
>+        }
>+        start_pfn = start >> target_page_bits;
>+        nb_pages = ((start + length + page_size - 1) >> target_page_bits)
>+            - start_pfn;
>+        rc = xen_modified_memory(xen_domid, start_pfn, nb_pages);
>+        if (rc) {
>+            fprintf(stderr,
>+                    "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n",
>+                    __func__, start, nb_pages, errno, strerror(errno));
>+        }
>+    }
>+}
>+
>+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
>+{
>+    if (enable) {
>+        memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
>+    } else {
>+        memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
>+    }
>+}
>+
>+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>+                         bool add)
>+{
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    hwaddr start_addr = section->offset_within_address_space;
>+    ram_addr_t size = int128_get64(section->size);
>+    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
>+    hvmmem_type_t mem_type;
>+
>+    if (!memory_region_is_ram(section->mr)) {
>+        return;
>+    }
>+
>+    if (log_dirty != add) {
>+        return;
>+    }
>+
>+    trace_xen_client_set_memory(start_addr, size, log_dirty);
>+
>+    start_addr &= page_mask;
>+    size = ROUND_UP(size, page_size);
>+
>+    if (add) {
>+        if (!memory_region_is_rom(section->mr)) {
>+            xen_add_to_physmap(state, start_addr, size,
>+                               section->mr, section->offset_within_region);
>+        } else {
>+            mem_type = HVMMEM_ram_ro;
>+            if (xen_set_mem_type(xen_domid, mem_type,
>+                                 start_addr >> target_page_bits,
>+                                 size >> target_page_bits)) {
>+                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
>+                        start_addr);
>+            }
>+        }
>+    } else {
>+        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
>+            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
>+        }
>+    }
>+}
> 
> void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
>                    Error **errp)
>diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
>index 53c6a6f2a0..da8912f037 100644
>--- a/stubs/xen-hw-stub.c
>+++ b/stubs/xen-hw-stub.c
>@@ -13,7 +13,3 @@ int xen_is_pirq_msi(uint32_t msi_data)
> {
>     return 0;
> }
>-
>-void xen_register_framebuffer(MemoryRegion *mr)
>-{
>-}
>diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
>index d38759cfe4..3f0df8bc07 100644
>--- a/hw/i386/xen/meson.build
>+++ b/hw/i386/xen/meson.build
>@@ -1,7 +1,6 @@
> i386_ss.add(when: 'CONFIG_XEN', if_true: files(
>   'xen_apic.c',
>   'xen_pvdevice.c',
>-  'xen-hvm-common.c',
> ))
> i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
>   'xen-hvm.c',
>-- 
>2.41.0
>
>

[PATCH-for-9.0 9/9] hw/xen/hvm: Inline xen_arch_set_memory()

[Philippe Mathieu-Daudé]

xen_arch_set_memory() is not arch-specific anymore. Being
called once, inline it in xen_set_memory().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/xen/xen-hvm-common.h |   3 -
 hw/xen/xen-hvm-common.c         | 104 ++++++++++++++++----------------
 2 files changed, 51 insertions(+), 56 deletions(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 536712dc83..a1b8a2783b 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -99,8 +99,5 @@ void cpu_ioreq_pio(ioreq_t *req);
 
 void xen_read_physmap(XenIOState *state);
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void xen_arch_set_memory(XenIOState *state,
-                         MemoryRegionSection *section,
-                         bool add);
 
 #endif /* HW_XEN_HVM_COMMON_H */
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 50ce45effc..789c6b4b7a 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -426,50 +426,6 @@ void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
     }
 }
 
-void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
-                         bool add)
-{
-    unsigned target_page_bits = qemu_target_page_bits();
-    int page_size = qemu_target_page_size();
-    int page_mask = -page_size;
-    hwaddr start_addr = section->offset_within_address_space;
-    ram_addr_t size = int128_get64(section->size);
-    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
-    hvmmem_type_t mem_type;
-
-    if (!memory_region_is_ram(section->mr)) {
-        return;
-    }
-
-    if (log_dirty != add) {
-        return;
-    }
-
-    trace_xen_client_set_memory(start_addr, size, log_dirty);
-
-    start_addr &= page_mask;
-    size = ROUND_UP(size, page_size);
-
-    if (add) {
-        if (!memory_region_is_rom(section->mr)) {
-            xen_add_to_physmap(state, start_addr, size,
-                               section->mr, section->offset_within_region);
-        } else {
-            mem_type = HVMMEM_ram_ro;
-            if (xen_set_mem_type(xen_domid, mem_type,
-                                 start_addr >> target_page_bits,
-                                 size >> target_page_bits)) {
-                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
-                        start_addr);
-            }
-        }
-    } else {
-        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
-            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
-        }
-    }
-}
-
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
                    Error **errp)
 {
@@ -512,20 +468,62 @@ static void xen_set_memory(struct MemoryListener *listener,
                            bool add)
 {
     XenIOState *state = container_of(listener, XenIOState, memory_listener);
+    unsigned target_page_bits = qemu_target_page_bits();
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
+    hwaddr start_addr;
+    ram_addr_t size;
+    bool log_dirty;
+    hvmmem_type_t mem_type;
+
 
     if (section->mr == &xen_memory) {
         return;
-    } else {
-        if (add) {
-            xen_map_memory_section(xen_domid, state->ioservid,
-                                   section);
-        } else {
-            xen_unmap_memory_section(xen_domid, state->ioservid,
-                                     section);
-        }
     }
 
-    xen_arch_set_memory(state, section, add);
+    if (add) {
+        xen_map_memory_section(xen_domid, state->ioservid,
+                               section);
+    } else {
+        xen_unmap_memory_section(xen_domid, state->ioservid,
+                                 section);
+    }
+
+    if (!memory_region_is_ram(section->mr)) {
+        return;
+    }
+
+    log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
+
+    if (log_dirty != add) {
+        return;
+    }
+
+    start_addr = section->offset_within_address_space;
+    size = int128_get64(section->size);
+    trace_xen_client_set_memory(start_addr, size, log_dirty);
+
+    start_addr &= page_mask;
+    size = ROUND_UP(size, page_size);
+
+    if (add) {
+        if (!memory_region_is_rom(section->mr)) {
+            xen_add_to_physmap(state, start_addr, size,
+                               section->mr, section->offset_within_region);
+        } else {
+            mem_type = HVMMEM_ram_ro;
+            if (xen_set_mem_type(xen_domid, mem_type,
+                                 start_addr >> target_page_bits,
+                                 size >> target_page_bits)) {
+                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
+                        start_addr);
+            }
+        }
+    } else {
+        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
+            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
+        }
+    }
 }
 
 void xen_region_add(MemoryListener *listener,
-- 
2.41.0

Re: [PATCH-for-9.0 9/9] hw/xen/hvm: Inline xen_arch_set_memory()

[Manos Pitsidianakis]

On Tue, 14 Nov 2023 18:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>xen_arch_set_memory() is not arch-specific anymore. Being
>called once, inline it in xen_set_memory().
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> include/hw/xen/xen-hvm-common.h |   3 -
> hw/xen/xen-hvm-common.c         | 104 ++++++++++++++++----------------
> 2 files changed, 51 insertions(+), 56 deletions(-)
>
>diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
>index 536712dc83..a1b8a2783b 100644
>--- a/include/hw/xen/xen-hvm-common.h
>+++ b/include/hw/xen/xen-hvm-common.h
>@@ -99,8 +99,5 @@ void cpu_ioreq_pio(ioreq_t *req);
> 
> void xen_read_physmap(XenIOState *state);
> void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
>-void xen_arch_set_memory(XenIOState *state,
>-                         MemoryRegionSection *section,
>-                         bool add);
> 
> #endif /* HW_XEN_HVM_COMMON_H */
>diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
>index 50ce45effc..789c6b4b7a 100644
>--- a/hw/xen/xen-hvm-common.c
>+++ b/hw/xen/xen-hvm-common.c
>@@ -426,50 +426,6 @@ void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
>     }
> }
> 
>-void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>-                         bool add)
>-{
>-    unsigned target_page_bits = qemu_target_page_bits();
>-    int page_size = qemu_target_page_size();
>-    int page_mask = -page_size;
>-    hwaddr start_addr = section->offset_within_address_space;
>-    ram_addr_t size = int128_get64(section->size);
>-    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
>-    hvmmem_type_t mem_type;
>-
>-    if (!memory_region_is_ram(section->mr)) {
>-        return;
>-    }
>-
>-    if (log_dirty != add) {
>-        return;
>-    }
>-
>-    trace_xen_client_set_memory(start_addr, size, log_dirty);
>-
>-    start_addr &= page_mask;
>-    size = ROUND_UP(size, page_size);
>-
>-    if (add) {
>-        if (!memory_region_is_rom(section->mr)) {
>-            xen_add_to_physmap(state, start_addr, size,
>-                               section->mr, section->offset_within_region);
>-        } else {
>-            mem_type = HVMMEM_ram_ro;
>-            if (xen_set_mem_type(xen_domid, mem_type,
>-                                 start_addr >> target_page_bits,
>-                                 size >> target_page_bits)) {
>-                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
>-                        start_addr);
>-            }
>-        }
>-    } else {
>-        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
>-            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
>-        }
>-    }
>-}
>-
> void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
>                    Error **errp)
> {
>@@ -512,20 +468,62 @@ static void xen_set_memory(struct MemoryListener *listener,
>                            bool add)
> {
>     XenIOState *state = container_of(listener, XenIOState, memory_listener);
>+    unsigned target_page_bits = qemu_target_page_bits();
>+    int page_size = qemu_target_page_size();
>+    int page_mask = -page_size;
>+    hwaddr start_addr;
>+    ram_addr_t size;
>+    bool log_dirty;
>+    hvmmem_type_t mem_type;
>+
> 
>     if (section->mr == &xen_memory) {
>         return;
>-    } else {
>-        if (add) {
>-            xen_map_memory_section(xen_domid, state->ioservid,
>-                                   section);
>-        } else {
>-            xen_unmap_memory_section(xen_domid, state->ioservid,
>-                                     section);
>-        }
>     }
> 
>-    xen_arch_set_memory(state, section, add);
>+    if (add) {
>+        xen_map_memory_section(xen_domid, state->ioservid,
>+                               section);
>+    } else {
>+        xen_unmap_memory_section(xen_domid, state->ioservid,
>+                                 section);
>+    }
>+
>+    if (!memory_region_is_ram(section->mr)) {
>+        return;
>+    }
>+
>+    log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
>+
>+    if (log_dirty != add) {
>+        return;
>+    }
>+
>+    start_addr = section->offset_within_address_space;
>+    size = int128_get64(section->size);
>+    trace_xen_client_set_memory(start_addr, size, log_dirty);
>+
>+    start_addr &= page_mask;
>+    size = ROUND_UP(size, page_size);
>+
>+    if (add) {
>+        if (!memory_region_is_rom(section->mr)) {
>+            xen_add_to_physmap(state, start_addr, size,
>+                               section->mr, section->offset_within_region);
>+        } else {
>+            mem_type = HVMMEM_ram_ro;
>+            if (xen_set_mem_type(xen_domid, mem_type,
>+                                 start_addr >> target_page_bits,
>+                                 size >> target_page_bits)) {
>+                DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n",
>+                        start_addr);
>+            }
>+        }
>+    } else {
>+        if (xen_remove_from_physmap(state, start_addr, size) < 0) {
>+            DPRINTF("physmapping does not exist at "HWADDR_FMT_plx"\n", start_addr);
>+        }
>+    }
> }
> 
> void xen_region_add(MemoryListener *listener,
>-- 
>2.41.0
>
>


Same observation as in previous patch, in Arm xen, qemu doesn't handle 
memory, it is only responsible for devices and their memory.

Re: [RFC PATCH-for-9.0 0/9] hw/xen: Have ARM targets use common xen_memory_listener

[Philippe Mathieu-Daudé]

ping?

On 14/11/23 17:31, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> While looking at Xen target-specific code, I noticed some
> generic code used by x86 which is not implemented for ARM.
> 
> Maybe ARM machines don't need it, I don't know. But I
> wanted to see if I can get this common code target agnostic
> and build it once, possibly bringing smth useful to ARM.
> 
> The changes don't break CI testing and Avocado local tests.
> If this xen_memory_listener feature isn't required for ARM,
> I'll send follow up patch that keep this series with common
> xen_memory_listener but with a runtime flag to disable.
> 
> Patches do the usual "change target-specific API to a
> target-agnostic one", in particular using "exec/target_page.h"
> at runtime. Then non-x86 code is extracted, then merged to
> the generic xen/xen-hvm-common.c.
> 
> Thoughts?
> 
> Regards,
> 
> Phil.
> 
> Based-on: <20231114143816.71079-1-philmd@linaro.org>

Re: [RFC PATCH-for-9.0 0/9] hw/xen: Have ARM targets use common xen_memory_listener

[Philippe Mathieu-Daudé]

ping ^ 2

On 13/12/23 18:00, Philippe Mathieu-Daudé wrote:
> ping?
> 
> On 14/11/23 17:31, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> While looking at Xen target-specific code, I noticed some
>> generic code used by x86 which is not implemented for ARM.
>>
>> Maybe ARM machines don't need it, I don't know. But I
>> wanted to see if I can get this common code target agnostic
>> and build it once, possibly bringing smth useful to ARM.
>>
>> The changes don't break CI testing and Avocado local tests.
>> If this xen_memory_listener feature isn't required for ARM,
>> I'll send follow up patch that keep this series with common
>> xen_memory_listener but with a runtime flag to disable.
>>
>> Patches do the usual "change target-specific API to a
>> target-agnostic one", in particular using "exec/target_page.h"
>> at runtime. Then non-x86 code is extracted, then merged to
>> the generic xen/xen-hvm-common.c.
>>
>> Thoughts?
>>
>> Regards,
>>
>> Phil.
>>
>> Based-on: <20231114143816.71079-1-philmd@linaro.org>
>