[PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[MingWang Li]

From: Mingwang Li <limingwang@huawei.com>

If default main_mem is used to be registered as the system memory,
other memory cannot be initialized. Therefore, the system memory
should be initialized to the machine->ram, which consists of the
default main_mem and other possible memory required by applications,
such as shared hugepage memory in DPDK.
Also, the mc->defaul_ram_id should be set to the default main_mem,
which is named as "riscv_virt_board.ram".

Signed-off-by: Mingwang Li <limingwang@huawei.com>
Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
---
 hw/riscv/virt.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index ec0cb69b8c..b3b431c847 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -771,7 +771,6 @@ static void virt_machine_init(MachineState *machine)
     const MemMapEntry *memmap = virt_memmap;
     RISCVVirtState *s = RISCV_VIRT_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
-    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
     char *plic_hart_config, *soc_name;
     target_ulong start_addr = memmap[VIRT_DRAM].base;
@@ -890,10 +889,8 @@ static void virt_machine_init(MachineState *machine)
     }
 
     /* register system main memory (actual RAM) */
-    memory_region_init_ram(main_mem, NULL, "riscv_virt_board.ram",
-                           machine->ram_size, &error_fatal);
     memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
-        main_mem);
+        machine->ram);
 
     /* create device tree */
     create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
@@ -1032,6 +1029,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
     mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
     mc->numa_mem_supported = true;
+    mc->default_ram_id = "riscv_virt_board.ram";
 
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
 
-- 
2.19.1

[PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[MingWang Li]

From: Mingwang Li <limingwang@huawei.com>

If default main_mem is used to be registered as the system memory,
other memory cannot be initialized. Therefore, the system memory
should be initialized to the machine->ram, which consists of the
default main_mem and other possible memory required by applications,
such as shared hugepage memory in DPDK.
Also, the mc->defaul_ram_id should be set to the default main_mem,
which is named as "riscv_virt_board.ram".

Signed-off-by: Mingwang Li <limingwang@huawei.com>
Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
---
 hw/riscv/virt.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index ec0cb69b8c..b3b431c847 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -771,7 +771,6 @@ static void virt_machine_init(MachineState *machine)
     const MemMapEntry *memmap = virt_memmap;
     RISCVVirtState *s = RISCV_VIRT_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
-    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
     char *plic_hart_config, *soc_name;
     target_ulong start_addr = memmap[VIRT_DRAM].base;
@@ -890,10 +889,8 @@ static void virt_machine_init(MachineState *machine)
     }
 
     /* register system main memory (actual RAM) */
-    memory_region_init_ram(main_mem, NULL, "riscv_virt_board.ram",
-                           machine->ram_size, &error_fatal);
     memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
-        main_mem);
+        machine->ram);
 
     /* create device tree */
     create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
@@ -1032,6 +1029,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
     mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
     mc->numa_mem_supported = true;
+    mc->default_ram_id = "riscv_virt_board.ram";
 
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
 
-- 
2.19.1

Re: [PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[Bin Meng]

On Wed, Sep 29, 2021 at 10:03 PM MingWang Li <limingwang@huawei.com> wrote:
>
> From: Mingwang Li <limingwang@huawei.com>
>
> If default main_mem is used to be registered as the system memory,
> other memory cannot be initialized. Therefore, the system memory
> should be initialized to the machine->ram, which consists of the
> default main_mem and other possible memory required by applications,
> such as shared hugepage memory in DPDK.

Which command did you use to expose the error? It looks like not every
machine is done like this patch does.

Please reword the commit summary a little bit to help the understanding.

> Also, the mc->defaul_ram_id should be set to the default main_mem,
> which is named as "riscv_virt_board.ram".
>
> Signed-off-by: Mingwang Li <limingwang@huawei.com>
> Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> ---
>  hw/riscv/virt.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index ec0cb69b8c..b3b431c847 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -771,7 +771,6 @@ static void virt_machine_init(MachineState *machine)
>      const MemMapEntry *memmap = virt_memmap;
>      RISCVVirtState *s = RISCV_VIRT_MACHINE(machine);
>      MemoryRegion *system_memory = get_system_memory();
> -    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>      char *plic_hart_config, *soc_name;
>      target_ulong start_addr = memmap[VIRT_DRAM].base;
> @@ -890,10 +889,8 @@ static void virt_machine_init(MachineState *machine)
>      }
>
>      /* register system main memory (actual RAM) */
> -    memory_region_init_ram(main_mem, NULL, "riscv_virt_board.ram",
> -                           machine->ram_size, &error_fatal);
>      memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
> -        main_mem);
> +        machine->ram);
>
>      /* create device tree */
>      create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> @@ -1032,6 +1029,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
>      mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
>      mc->numa_mem_supported = true;
> +    mc->default_ram_id = "riscv_virt_board.ram";
>
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>
> --

Regards,
Bin

Re: [PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[Bin Meng]

On Wed, Sep 29, 2021 at 10:03 PM MingWang Li <limingwang@huawei.com> wrote:
>
> From: Mingwang Li <limingwang@huawei.com>
>
> If default main_mem is used to be registered as the system memory,
> other memory cannot be initialized. Therefore, the system memory
> should be initialized to the machine->ram, which consists of the
> default main_mem and other possible memory required by applications,
> such as shared hugepage memory in DPDK.

Which command did you use to expose the error? It looks like not every
machine is done like this patch does.

Please reword the commit summary a little bit to help the understanding.

> Also, the mc->defaul_ram_id should be set to the default main_mem,
> which is named as "riscv_virt_board.ram".
>
> Signed-off-by: Mingwang Li <limingwang@huawei.com>
> Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> ---
>  hw/riscv/virt.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index ec0cb69b8c..b3b431c847 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -771,7 +771,6 @@ static void virt_machine_init(MachineState *machine)
>      const MemMapEntry *memmap = virt_memmap;
>      RISCVVirtState *s = RISCV_VIRT_MACHINE(machine);
>      MemoryRegion *system_memory = get_system_memory();
> -    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>      char *plic_hart_config, *soc_name;
>      target_ulong start_addr = memmap[VIRT_DRAM].base;
> @@ -890,10 +889,8 @@ static void virt_machine_init(MachineState *machine)
>      }
>
>      /* register system main memory (actual RAM) */
> -    memory_region_init_ram(main_mem, NULL, "riscv_virt_board.ram",
> -                           machine->ram_size, &error_fatal);
>      memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
> -        main_mem);
> +        machine->ram);
>
>      /* create device tree */
>      create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> @@ -1032,6 +1029,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
>      mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
>      mc->numa_mem_supported = true;
> +    mc->default_ram_id = "riscv_virt_board.ram";
>
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>
> --

Regards,
Bin

RE: [PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[limingwang (A)]

> >
> > From: Mingwang Li <limingwang@huawei.com>
> >
> > If default main_mem is used to be registered as the system memory,
> > other memory cannot be initialized. Therefore, the system memory
> > should be initialized to the machine->ram, which consists of the
> > default main_mem and other possible memory required by applications,
> > such as shared hugepage memory in DPDK.
> 
> Which command did you use to expose the error? It looks like not every
> machine is done like this patch does.
> 

The command we used to expose the error is as follows:
./qemu-system-riscv64 -M virt,accel=kvm -m 4096M -cpu host -nographic \
    -name guest=riscv-guset \
	-smp 2 \
	-bios none \
	-kernel ./Image \
	-drive file=./guest.img,format=raw,id=hd0 \
	-device virtio-blk-device,drive=hd0 \
	-append "root=/dev/vda rw console=ttyS0 earlycon=sbi"
    -object memory-backend-file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on \
    -numa node,memdev=mem -mem-prealloc \
    -chardev socket,id=char0,path=/mnt/vhost-net0 \
    -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce \
    -device virtio-net-pci,mac=52:54:00:00:00:01,netdev=mynet1,mrg_rxbuf=on,csum=on,guest_csum=on,guest_ecn=on

Note that, before starting the kvm-acceled QEMU VM, following temporarily unaccepted QEMU patches should be used:
https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg02516.html

Then, QEMU displays the following error information:
qemu-system-riscv64: Failed initializing vhost-user memory map, consider using -object memory-backend-file share=on
qemu-system-riscv64: vhost_set_mem_table failed: Interrupted system call (4)
qemu-system-riscv64: unable to start vhost net: 4: falling back on userspace virtio

If other type of machine is configured when starting a QEMU VM, corresponding codes in the machine model should be modified as well.


> Please reword the commit summary a little bit to help the understanding.
> 
> > Also, the mc->defaul_ram_id should be set to the default main_mem,
> > which is named as "riscv_virt_board.ram".
> >
> > Signed-off-by: Mingwang Li <limingwang@huawei.com>
> > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > ---
> >  hw/riscv/virt.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index
> > ec0cb69b8c..b3b431c847 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -771,7 +771,6 @@ static void virt_machine_init(MachineState
> *machine)
> >      const MemMapEntry *memmap = virt_memmap;
> >      RISCVVirtState *s = RISCV_VIRT_MACHINE(machine);
> >      MemoryRegion *system_memory = get_system_memory();
> > -    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> >      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> >      char *plic_hart_config, *soc_name;
> >      target_ulong start_addr = memmap[VIRT_DRAM].base; @@ -890,10
> > +889,8 @@ static void virt_machine_init(MachineState *machine)
> >      }
> >
> >      /* register system main memory (actual RAM) */
> > -    memory_region_init_ram(main_mem, NULL, "riscv_virt_board.ram",
> > -                           machine->ram_size, &error_fatal);
> >      memory_region_add_subregion(system_memory,
> memmap[VIRT_DRAM].base,
> > -        main_mem);
> > +        machine->ram);
> >
> >      /* create device tree */
> >      create_fdt(s, memmap, machine->ram_size,
> machine->kernel_cmdline,
> > @@ -1032,6 +1029,7 @@ static void virt_machine_class_init(ObjectClass
> *oc, void *data)
> >      mc->cpu_index_to_instance_props =
> riscv_numa_cpu_index_to_props;
> >      mc->get_default_cpu_node_id =
> riscv_numa_get_default_cpu_node_id;
> >      mc->numa_mem_supported = true;
> > +    mc->default_ram_id = "riscv_virt_board.ram";
> >
> >      machine_class_allow_dynamic_sysbus_dev(mc,
> TYPE_RAMFB_DEVICE);
> >
> > --
> 
> Regards,
> Bin

RE: [PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[limingwang (A)]

> >
> > From: Mingwang Li <limingwang@huawei.com>
> >
> > If default main_mem is used to be registered as the system memory,
> > other memory cannot be initialized. Therefore, the system memory
> > should be initialized to the machine->ram, which consists of the
> > default main_mem and other possible memory required by applications,
> > such as shared hugepage memory in DPDK.
> 
> Which command did you use to expose the error? It looks like not every
> machine is done like this patch does.
> 

The command we used to expose the error is as follows:
./qemu-system-riscv64 -M virt,accel=kvm -m 4096M -cpu host -nographic \
    -name guest=riscv-guset \
	-smp 2 \
	-bios none \
	-kernel ./Image \
	-drive file=./guest.img,format=raw,id=hd0 \
	-device virtio-blk-device,drive=hd0 \
	-append "root=/dev/vda rw console=ttyS0 earlycon=sbi"
    -object memory-backend-file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on \
    -numa node,memdev=mem -mem-prealloc \
    -chardev socket,id=char0,path=/mnt/vhost-net0 \
    -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce \
    -device virtio-net-pci,mac=52:54:00:00:00:01,netdev=mynet1,mrg_rxbuf=on,csum=on,guest_csum=on,guest_ecn=on

Note that, before starting the kvm-acceled QEMU VM, following temporarily unaccepted QEMU patches should be used:
https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg02516.html

Then, QEMU displays the following error information:
qemu-system-riscv64: Failed initializing vhost-user memory map, consider using -object memory-backend-file share=on
qemu-system-riscv64: vhost_set_mem_table failed: Interrupted system call (4)
qemu-system-riscv64: unable to start vhost net: 4: falling back on userspace virtio

If other type of machine is configured when starting a QEMU VM, corresponding codes in the machine model should be modified as well.


> Please reword the commit summary a little bit to help the understanding.
> 
> > Also, the mc->defaul_ram_id should be set to the default main_mem,
> > which is named as "riscv_virt_board.ram".
> >
> > Signed-off-by: Mingwang Li <limingwang@huawei.com>
> > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > ---
> >  hw/riscv/virt.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index
> > ec0cb69b8c..b3b431c847 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -771,7 +771,6 @@ static void virt_machine_init(MachineState
> *machine)
> >      const MemMapEntry *memmap = virt_memmap;
> >      RISCVVirtState *s = RISCV_VIRT_MACHINE(machine);
> >      MemoryRegion *system_memory = get_system_memory();
> > -    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> >      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> >      char *plic_hart_config, *soc_name;
> >      target_ulong start_addr = memmap[VIRT_DRAM].base; @@ -890,10
> > +889,8 @@ static void virt_machine_init(MachineState *machine)
> >      }
> >
> >      /* register system main memory (actual RAM) */
> > -    memory_region_init_ram(main_mem, NULL, "riscv_virt_board.ram",
> > -                           machine->ram_size, &error_fatal);
> >      memory_region_add_subregion(system_memory,
> memmap[VIRT_DRAM].base,
> > -        main_mem);
> > +        machine->ram);
> >
> >      /* create device tree */
> >      create_fdt(s, memmap, machine->ram_size,
> machine->kernel_cmdline,
> > @@ -1032,6 +1029,7 @@ static void virt_machine_class_init(ObjectClass
> *oc, void *data)
> >      mc->cpu_index_to_instance_props =
> riscv_numa_cpu_index_to_props;
> >      mc->get_default_cpu_node_id =
> riscv_numa_get_default_cpu_node_id;
> >      mc->numa_mem_supported = true;
> > +    mc->default_ram_id = "riscv_virt_board.ram";
> >
> >      machine_class_allow_dynamic_sysbus_dev(mc,
> TYPE_RAMFB_DEVICE);
> >
> > --
> 
> Regards,
> Bin

Re: [PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[Alistair Francis]

On Thu, Sep 30, 2021 at 12:03 AM MingWang Li <limingwang@huawei.com> wrote:
>
> From: Mingwang Li <limingwang@huawei.com>
>
> If default main_mem is used to be registered as the system memory,
> other memory cannot be initialized. Therefore, the system memory
> should be initialized to the machine->ram, which consists of the
> default main_mem and other possible memory required by applications,
> such as shared hugepage memory in DPDK.
> Also, the mc->defaul_ram_id should be set to the default main_mem,
> which is named as "riscv_virt_board.ram".
>
> Signed-off-by: Mingwang Li <limingwang@huawei.com>
> Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

Re: [PATCH] hw/riscv: virt: bugfix the memory-backend-file command is invalid

[Alistair Francis]

On Thu, Sep 30, 2021 at 12:03 AM MingWang Li <limingwang@huawei.com> wrote:
>
> From: Mingwang Li <limingwang@huawei.com>
>
> If default main_mem is used to be registered as the system memory,
> other memory cannot be initialized. Therefore, the system memory
> should be initialized to the machine->ram, which consists of the
> default main_mem and other possible memory required by applications,
> such as shared hugepage memory in DPDK.
> Also, the mc->defaul_ram_id should be set to the default main_mem,
> which is named as "riscv_virt_board.ram".
>
> Signed-off-by: Mingwang Li <limingwang@huawei.com>
> Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair