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(>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(>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(>mr);
> > sz = memory_region_size(>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(>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(>mr);
> sz = memory_region_size(>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(>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
