From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Add xen_mr_is_memory() to abstract away tests for the
xen_memory MR.

No functional changes.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: David Hildenbrand <david@redhat.com>
---
 hw/xen/xen-hvm-common.c | 10 ++++++++--
 include/sysemu/xen.h    |  8 ++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 2d1b032121..a0a0252da0 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -12,6 +12,12 @@
 
 MemoryRegion xen_memory;
 
+/* Check for xen memory.  */
+bool xen_mr_is_memory(MemoryRegion *mr)
+{
+    return mr == &xen_memory;
+}
+
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
                    Error **errp)
 {
@@ -28,7 +34,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
         return;
     }
 
-    if (mr == &xen_memory) {
+    if (xen_mr_is_memory(mr)) {
         return;
     }
 
@@ -55,7 +61,7 @@ static void xen_set_memory(struct MemoryListener *listener,
 {
     XenIOState *state = container_of(listener, XenIOState, memory_listener);
 
-    if (section->mr == &xen_memory) {
+    if (xen_mr_is_memory(section->mr)) {
         return;
     } else {
         if (add) {
diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
index 754ec2e6cb..dc72f83bcb 100644
--- a/include/sysemu/xen.h
+++ b/include/sysemu/xen.h
@@ -34,6 +34,8 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
                    struct MemoryRegion *mr, Error **errp);
 
+bool xen_mr_is_memory(MemoryRegion *mr);
+
 #else /* !CONFIG_XEN_IS_POSSIBLE */
 
 #define xen_enabled() 0
@@ -47,6 +49,12 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
     g_assert_not_reached();
 }
 
+static inline bool xen_mr_is_memory(MemoryRegion *mr)
+{
+    g_assert_not_reached();
+    return false;
+}
+
 #endif /* CONFIG_XEN_IS_POSSIBLE */
 
 #endif
-- 
2.40.1

Hi Edgar,

On 24/5/24 12:51, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> 
> Add xen_mr_is_memory() to abstract away tests for the
> xen_memory MR.
> 
> No functional changes.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> Acked-by: David Hildenbrand <david@redhat.com>
> ---
>   hw/xen/xen-hvm-common.c | 10 ++++++++--
>   include/sysemu/xen.h    |  8 ++++++++
>   2 files changed, 16 insertions(+), 2 deletions(-)

To consolidate we could add:

   static MemoryRegion xen_memory;

   MemoryRegion *xen_mr_memory_init(uint64_t block_len)
   {
      assert(!xen_memory.size);
      memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len, 
&error_fatal);
      return &xen_memory;
   }

and remove the extern declaration.

> diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
> index 754ec2e6cb..dc72f83bcb 100644
> --- a/include/sysemu/xen.h
> +++ b/include/sysemu/xen.h
> @@ -34,6 +34,8 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
>   void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
>                      struct MemoryRegion *mr, Error **errp);
>   
> +bool xen_mr_is_memory(MemoryRegion *mr);
> +
>   #else /* !CONFIG_XEN_IS_POSSIBLE */
>   
>   #define xen_enabled() 0
> @@ -47,6 +49,12 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
>       g_assert_not_reached();
>   }
>   
> +static inline bool xen_mr_is_memory(MemoryRegion *mr)
> +{
> +    g_assert_not_reached();
> +    return false;

No need for the stub, just always declare xen_mr_is_memory() ...
> +}
> +
>   #endif /* CONFIG_XEN_IS_POSSIBLE */

... here.

>   #endif

Removing the stub:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

On Mon, May 27, 2024 at 6:25 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Hi Edgar,
>
> On 24/5/24 12:51, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> >
> > Add xen_mr_is_memory() to abstract away tests for the
> > xen_memory MR.
> >
> > No functional changes.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> > Acked-by: David Hildenbrand <david@redhat.com>
> > ---
> >   hw/xen/xen-hvm-common.c | 10 ++++++++--
> >   include/sysemu/xen.h    |  8 ++++++++
> >   2 files changed, 16 insertions(+), 2 deletions(-)
>
> To consolidate we could add:
>
>    static MemoryRegion xen_memory;
>
>    MemoryRegion *xen_mr_memory_init(uint64_t block_len)
>    {
>       assert(!xen_memory.size);
>       memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len,
> &error_fatal);
>       return &xen_memory;
>    }
>
> and remove the extern declaration.
>

Thanks,

We have a future patch series in the workings that will add a PVH machine
for x86, I'll keep this in mind for then!


>
> > diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
> > index 754ec2e6cb..dc72f83bcb 100644
> > --- a/include/sysemu/xen.h
> > +++ b/include/sysemu/xen.h
> > @@ -34,6 +34,8 @@ void xen_hvm_modified_memory(ram_addr_t start,
> ram_addr_t length);
> >   void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
> >                      struct MemoryRegion *mr, Error **errp);
> >
> > +bool xen_mr_is_memory(MemoryRegion *mr);
> > +
> >   #else /* !CONFIG_XEN_IS_POSSIBLE */
> >
> >   #define xen_enabled() 0
> > @@ -47,6 +49,12 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr,
> ram_addr_t size,
> >       g_assert_not_reached();
> >   }
> >
> > +static inline bool xen_mr_is_memory(MemoryRegion *mr)
> > +{
> > +    g_assert_not_reached();
> > +    return false;
>
> No need for the stub, just always declare xen_mr_is_memory() ...
> > +}
> > +
> >   #endif /* CONFIG_XEN_IS_POSSIBLE */
>
> ... here.
>
> >   #endif
>
> Removing the stub:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
Thanks Philippe, will remove the stubs in v8.

Cheers,
Edgar