Re: [Qemu-devel] [RFC QEMU PATCH v4 03/10] hostmem-xen: add a host memory backend for Xen
On 03/02/18 11:50 +, Anthony PERARD wrote: > On Wed, Feb 28, 2018 at 03:56:54PM +0800, Haozhong Zhang wrote: > > On 02/27/18 16:41 +, Anthony PERARD wrote: > > > On Thu, Dec 07, 2017 at 06:18:05PM +0800, Haozhong Zhang wrote: > > > > @@ -108,7 +109,10 @@ void pc_dimm_memory_plug(DeviceState *dev, > > > > MemoryHotplugState *hpms, > > > > } > > > > > > > > memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr); > > > > -vmstate_register_ram(vmstate_mr, dev); > > > > +/* memory-backend-xen is not backed by RAM. */ > > > > +if (!xen_enabled()) { > > > > > > Is it possible to have the same condition as the one used in > > > host_memory_backend_memory_complete? i.e. base on whether the memory > > > region is mapped or not (backend->mr.ram_block). > > > > Like "if (!xen_enabled() || backend->mr.ram_block))"? No, it will mute > > the abortion (vmstate_register_ram --> qemu_ram_set_idstr ) caused by > > the case that !backend->mr.ram_block in the non-xen environment. > > In non-xen environment, vmstate_register_ram() will be called, because > !xen_enabled() is true, it would not matter if there is a ram_block or > not. Sorry, I really meant 'if (backend->mr.ram_block)', which may mute the abortion in non-xen environment. 'if (!xen_enabled())' keeps the original semantics in non-xen environment, so it's unlikely to break the non-xen usage. Haozhong > > But if there is a memory-backend that can run in a xen environment that > have a ram_block, vmstate_register_ram would not be called in the > origial patch, but if we use (!xen_enabled() || vmstate_mr->ram_block) > as condition then vmstate_register_ram will be called. > > Is this make sense? > > > > > +vmstate_register_ram(vmstate_mr, dev); > > > > +} > > > > numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node); > > > > > > > > out: > > -- > Anthony PERARD >
Re: [Qemu-devel] [RFC QEMU PATCH v4 03/10] hostmem-xen: add a host memory backend for Xen
On Wed, Feb 28, 2018 at 03:56:54PM +0800, Haozhong Zhang wrote: > On 02/27/18 16:41 +, Anthony PERARD wrote: > > On Thu, Dec 07, 2017 at 06:18:05PM +0800, Haozhong Zhang wrote: > > > @@ -108,7 +109,10 @@ void pc_dimm_memory_plug(DeviceState *dev, > > > MemoryHotplugState *hpms, > > > } > > > > > > memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr); > > > -vmstate_register_ram(vmstate_mr, dev); > > > +/* memory-backend-xen is not backed by RAM. */ > > > +if (!xen_enabled()) { > > > > Is it possible to have the same condition as the one used in > > host_memory_backend_memory_complete? i.e. base on whether the memory > > region is mapped or not (backend->mr.ram_block). > > Like "if (!xen_enabled() || backend->mr.ram_block))"? No, it will mute > the abortion (vmstate_register_ram --> qemu_ram_set_idstr ) caused by > the case that !backend->mr.ram_block in the non-xen environment. In non-xen environment, vmstate_register_ram() will be called, because !xen_enabled() is true, it would not matter if there is a ram_block or not. But if there is a memory-backend that can run in a xen environment that have a ram_block, vmstate_register_ram would not be called in the origial patch, but if we use (!xen_enabled() || vmstate_mr->ram_block) as condition then vmstate_register_ram will be called. Is this make sense? > > > +vmstate_register_ram(vmstate_mr, dev); > > > +} > > > numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node); > > > > > > out: -- Anthony PERARD
Re: [Qemu-devel] [RFC QEMU PATCH v4 03/10] hostmem-xen: add a host memory backend for Xen
On 02/27/18 16:41 +, Anthony PERARD wrote: > On Thu, Dec 07, 2017 at 06:18:05PM +0800, Haozhong Zhang wrote: > > diff --git a/backends/hostmem.c b/backends/hostmem.c > > index ee2c2d5bfd..ba13a52994 100644 > > --- a/backends/hostmem.c > > +++ b/backends/hostmem.c > > @@ -12,6 +12,7 @@ > > #include "qemu/osdep.h" > > #include "sysemu/hostmem.h" > > #include "hw/boards.h" > > +#include "hw/xen/xen.h" > > #include "qapi/error.h" > > #include "qapi/visitor.h" > > #include "qapi-types.h" > > @@ -277,6 +278,14 @@ host_memory_backend_memory_complete(UserCreatable *uc, > > Error **errp) > > goto out; > > } > > > > +/* > > + * The backend storage of MEMORY_BACKEND_XEN is managed by Xen, > > + * so no further work in this function is needed. > > + */ > > +if (xen_enabled() && !backend->mr.ram_block) { > > +goto out; > > +} > > + > > ptr = memory_region_get_ram_ptr(&backend->mr); > > sz = memory_region_size(&backend->mr); > > > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > > index 66eace5a5c..dcbfce33d5 100644 > > --- a/hw/mem/pc-dimm.c > > +++ b/hw/mem/pc-dimm.c > > @@ -28,6 +28,7 @@ > > #include "sysemu/kvm.h" > > #include "trace.h" > > #include "hw/virtio/vhost.h" > > +#include "hw/xen/xen.h" > > > > typedef struct pc_dimms_capacity { > > uint64_t size; > > @@ -108,7 +109,10 @@ void pc_dimm_memory_plug(DeviceState *dev, > > MemoryHotplugState *hpms, > > } > > > > memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr); > > -vmstate_register_ram(vmstate_mr, dev); > > +/* memory-backend-xen is not backed by RAM. */ > > +if (!xen_enabled()) { > > Is it possible to have the same condition as the one used in > host_memory_backend_memory_complete? i.e. base on whether the memory > region is mapped or not (backend->mr.ram_block). Like "if (!xen_enabled() || backend->mr.ram_block))"? No, it will mute the abortion (vmstate_register_ram --> qemu_ram_set_idstr ) caused by the case that !backend->mr.ram_block in the non-xen environment. Haozhong > > > +vmstate_register_ram(vmstate_mr, dev); > > +} > > numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node); > > > > out: > > -- > > 2.15.1 > > > > -- > Anthony PERARD
Re: [Qemu-devel] [RFC QEMU PATCH v4 03/10] hostmem-xen: add a host memory backend for Xen
On Thu, Dec 07, 2017 at 06:18:05PM +0800, Haozhong Zhang wrote: > diff --git a/backends/hostmem.c b/backends/hostmem.c > index ee2c2d5bfd..ba13a52994 100644 > --- a/backends/hostmem.c > +++ b/backends/hostmem.c > @@ -12,6 +12,7 @@ > #include "qemu/osdep.h" > #include "sysemu/hostmem.h" > #include "hw/boards.h" > +#include "hw/xen/xen.h" > #include "qapi/error.h" > #include "qapi/visitor.h" > #include "qapi-types.h" > @@ -277,6 +278,14 @@ host_memory_backend_memory_complete(UserCreatable *uc, > Error **errp) > goto out; > } > > +/* > + * The backend storage of MEMORY_BACKEND_XEN is managed by Xen, > + * so no further work in this function is needed. > + */ > +if (xen_enabled() && !backend->mr.ram_block) { > +goto out; > +} > + > ptr = memory_region_get_ram_ptr(&backend->mr); > sz = memory_region_size(&backend->mr); > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > index 66eace5a5c..dcbfce33d5 100644 > --- a/hw/mem/pc-dimm.c > +++ b/hw/mem/pc-dimm.c > @@ -28,6 +28,7 @@ > #include "sysemu/kvm.h" > #include "trace.h" > #include "hw/virtio/vhost.h" > +#include "hw/xen/xen.h" > > typedef struct pc_dimms_capacity { > uint64_t size; > @@ -108,7 +109,10 @@ void pc_dimm_memory_plug(DeviceState *dev, > MemoryHotplugState *hpms, > } > > memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr); > -vmstate_register_ram(vmstate_mr, dev); > +/* memory-backend-xen is not backed by RAM. */ > +if (!xen_enabled()) { Is it possible to have the same condition as the one used in host_memory_backend_memory_complete? i.e. base on whether the memory region is mapped or not (backend->mr.ram_block). > +vmstate_register_ram(vmstate_mr, dev); > +} > numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node); > > out: > -- > 2.15.1 > -- Anthony PERARD