Re: [PATCH] hw/mem/pc-dimm: Hint it is not usable on non-NUMA machines

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 24 May 2021 19:13:52 +0200
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> When trying to use the pc-dimm device on a non-NUMA machine, we get:
> 
>   $ qemu-system-arm -M none -cpu max -S -device pc-dimm
>   Segmentation fault (core dumped)
> 
>   (gdb) bt
>   #0  pc_dimm_realize (dev=0x555556da3e90, errp=0x7fffffffcd10) at hw/mem/pc-dimm.c:184
>   #1  0x0000555555fe1f8f in device_set_realized (obj=0x555556da3e90, value=true, errp=0x7fffffffce18) at hw/core/qdev.c:761
>   #2  0x0000555555feb4a9 in property_set_bool (obj=0x555556da3e90, v=0x555556e54420, name=0x5555563c3c41 "realized", opaque=0x555556a704f0, errp=0x7fffffffce18) at qom/object.c:2257
> 
> Use a friendler error message instead:
> 
>   $ qemu-system-arm -M none -cpu max -S -device pc-dimm
>   qemu-system-arm: -device pc-dimm: NUMA is not supported by this machine-type

it's not that pc-dimm inherently depends on numa, so maybe something like this would be better:

@@ -183,12 +182,17 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
...
+    if (ms->numa_state) {
+        int nb_numa_nodes = ms->numa_state->num_nodes;
+
+        if ((nb_numa_nodes > 0) &&
+            (dimm->node >= nb_numa_nodes)) ||
+           (!nb_numa_nodes && dimm->node)) {
+               error_setg(errp, "'DIMM property " PC_DIMM_NODE_PROP " has value %"
+                          PRIu32 "' which exceeds the number of numa nodes: %d",
+                          dimm->node, nb_numa_nodes ? nb_numa_nodes : 1);
+            return;
+        }
+     } else if (dimm->node > 0)
+         error_setg(errp, "machine doesn't support numa");
 

> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/mem/pc-dimm.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index a3a2560301c..e8851a0c3b1 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -181,8 +181,13 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
>      PCDIMMDevice *dimm = PC_DIMM(dev);
>      PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>      MachineState *ms = MACHINE(qdev_get_machine());
> -    int nb_numa_nodes = ms->numa_state->num_nodes;
> +    int nb_numa_nodes;
>  
> +    if (!ms->numa_state) {
> +        error_setg(errp, "NUMA is not supported by this machine-type");
> +        return;
> +    }
> +    nb_numa_nodes = ms->numa_state->num_nodes;
>      if (!dimm->hostmem) {
>          error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
>          return;