Re: [RFC QEMU PATCH 13/18] hw/i386/xen/xen-hvm: Introduce xen_ram_block_check function

From: Stefano Stabellini <sstabellini@kernel.org>
To: Huang Rui <ray.huang@amd.com>
Cc: "Gerd Hoffmann" <kraxel@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Anthony PERARD" <anthony.perard@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Antonio Caggiano" <antonio.caggiano@collabora.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	"Robert Beckett" <bob.beckett@collabora.com>,
	qemu-devel@nongnu.org, xen-devel@lists.xenproject.org,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Stewart Hildebrand" <Stewart.Hildebrand@amd.com>,
	"Xenia Ragiadakou" <burzalodowa@gmail.com>,
	"Honglei Huang" <honglei1.huang@amd.com>,
	"Julia Zhang" <julia.zhang@amd.com>,
	"Chen Jiqian" <Jiqian.Chen@amd.com>
Subject: Re: [RFC QEMU PATCH 13/18] hw/i386/xen/xen-hvm: Introduce xen_ram_block_check function
Date: Fri, 17 Mar 2023 17:38:10 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2303171735020.3359@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <20230312092244.451465-14-ray.huang@amd.com>

On Sun, 12 Mar 2023, Huang Rui wrote:
> Introduce xen_ram_block_check function to check whether current ramblock
> is xen ram memory.
> 
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  hw/i386/xen/xen-hvm.c | 15 +++++++++++++++
>  include/hw/xen/xen.h  |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index e4293d6d66..a4f12aefce 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -32,6 +32,7 @@
>  #include "sysemu/xen.h"
>  #include "sysemu/xen-mapcache.h"
>  #include "trace.h"
> +#include "include/exec/ramblock.h"
>  
>  #include <xen/hvm/ioreq.h>
>  #include <xen/hvm/e820.h>
> @@ -1564,6 +1565,20 @@ void xen_register_framebuffer(MemoryRegion *mr)
>      framebuffer = mr;
>  }
>  
> +bool xen_ram_block_check(RAMBlock *rb)
> +{
> +	bool ret;
> +
> +	if (!rb)
> +		return false;
> +
> +	ret = (rb == ram_memory.ram_block);
> +	if (ret)
> +		rb->offset = 0;

I take that this is needed because there is a ramblock that is
ram_memory but with offset != 0?  So it would fail the block->offset ==
0 check in qemu_ram_ptr_length (which is meant to capture all accesses
to ram_memory, but failing at it)?

If so, would it be possible to just do this instead:

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index fb412a56e1..3e2640dabd 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2149,7 +2149,7 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr,
          * because we don't want to map the entire memory in QEMU.
          * In that case just map the requested area.
          */
-        if (block->offset == 0) {
+        if (block->offset == 0 || block == ram_memory.ram_block) {
             return xen_map_cache(addr, *size, lock, lock);
         }
 


> +	return ret;
> +}
> +
>  void xen_shutdown_fatal_error(const char *fmt, ...)
>  {
>      va_list ap;
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index afdf9c436a..99a383eb17 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -31,5 +31,6 @@ qemu_irq *xen_interrupt_controller_init(void);
>  void xenstore_store_pv_console_info(int i, Chardev *chr);
>  
>  void xen_register_framebuffer(struct MemoryRegion *mr);
> +bool xen_ram_block_check(RAMBlock *rb);
>  
>  #endif /* QEMU_HW_XEN_H */
> -- 
> 2.25.1
> 

