Re: [PATCH v1 3/9] mem: appropriate handling getting mem region

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 29 Mar 2022 00:15:33 +0300
Maxim Davydov <maxim.davydov@openvz.org> wrote:

> Attempt to get memory region if the device doesn't have hostmem may not be
> an error. This can be happen immediately after initialization (getting
> value without default one).

Above statement begs for expanded explanation
Pls rephrase and explain why it's needed and how it will be used.

> 
> Signed-off-by: Maxim Davydov <maxim.davydov@openvz.org>
> ---
>  hw/i386/sgx-epc.c | 5 ++++-
>  hw/mem/nvdimm.c   | 6 ++++++
>  hw/mem/pc-dimm.c  | 5 +++++
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/sgx-epc.c b/hw/i386/sgx-epc.c
> index d664829d35..1a4c8acdcc 100644
> --- a/hw/i386/sgx-epc.c
> +++ b/hw/i386/sgx-epc.c
> @@ -121,9 +121,12 @@ static MemoryRegion *sgx_epc_md_get_memory_region(MemoryDeviceState *md,
>  {
>      SGXEPCDevice *epc = SGX_EPC(md);
>      HostMemoryBackend *hostmem;
> +    DeviceState *dev = DEVICE(epc);
>  
>      if (!epc->hostmem) {
> -        error_setg(errp, "'" SGX_EPC_MEMDEV_PROP "' property must be set");
> +        if (dev->realized) {
> +            error_setg(errp, "'" SGX_EPC_MEMDEV_PROP "' property must be set");
> +        }
>          return NULL;
>      }
>  
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index 7c7d777781..61e77e5476 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -166,9 +166,15 @@ static MemoryRegion *nvdimm_md_get_memory_region(MemoryDeviceState *md,
>                                                   Error **errp)
>  {
>      NVDIMMDevice *nvdimm = NVDIMM(md);
> +    PCDIMMDevice *dimm = PC_DIMM(nvdimm);
>      Error *local_err = NULL;
>  
>      if (!nvdimm->nvdimm_mr) {
> +        /* Not error if we try get memory region after init */
> +        if (!dimm->hostmem) {
> +            return NULL;
> +        }
> +
>          nvdimm_prepare_memory_region(nvdimm, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index f27e1a11ba..6fd74de97f 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -240,6 +240,11 @@ static void pc_dimm_md_set_addr(MemoryDeviceState *md, uint64_t addr,
>  static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md,
>                                                    Error **errp)
>  {
> +    PCDIMMDevice *dimm = PC_DIMM(md);
> +    /* Not error if we try get memory region after init */
> +    if (!dimm->hostmem) {
> +        return NULL;
> +    }
>      return pc_dimm_get_memory_region(PC_DIMM(md), errp);
>  }
>