On 10/4/25 19:29, Pierrick Bouvier wrote:
On 4/10/25 10:21, Philippe Mathieu-Daudé wrote:
On 10/4/25 16:36, Pierrick Bouvier wrote:
On 4/10/25 05:14, Philippe Mathieu-Daudé wrote:
Hi Pierrick,

On 13/12/22 00:05, Philippe Mathieu-Daudé wrote:
The current definition of VHOST_USER_MAX_RAM_SLOTS is
target specific. By converting this definition to a runtime
vhost_user_ram_slots_max() helper declared in a target
specific unit, we can have the rest of vhost-user.c target
independent.

To avoid variable length array or using the heap to store
arrays of vhost_user_ram_slots_max() elements, we simply
declare an array of the biggest VHOST_USER_MAX_RAM_SLOTS,
and each target uses up to vhost_user_ram_slots_max()
elements of it. Ensure arrays are big enough by adding an
assertion in vhost_user_init().

Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
RFC: Should I add VHOST_USER_MAX_RAM_SLOTS to vhost-user.h
        or create an internal header for it?
---
    hw/virtio/meson.build          |  1 +
    hw/virtio/vhost-user-target.c  | 29 +++++++++++++++++++++++++++++
    hw/virtio/vhost-user.c         | 26 +++++---------------------
    include/hw/virtio/vhost-user.h |  7 +++++++
    4 files changed, 42 insertions(+), 21 deletions(-)
    create mode 100644 hw/virtio/vhost-user-target.c

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index eb7ee8ea92..bf7e35fa8a 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -11,6 +11,7 @@ if have_vhost
      specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c',
'vhost-iova-tree.c'))
      if have_vhost_user
        specific_virtio_ss.add(files('vhost-user.c'))
+    specific_virtio_ss.add(files('vhost-user-target.c'))
      endif
      if have_vhost_vdpa
        specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-
virtqueue.c'))
diff --git a/hw/virtio/vhost-user-target.c b/hw/virtio/vhost-user-
target.c
new file mode 100644
index 0000000000..6a0d0f53d0
--- /dev/null
+++ b/hw/virtio/vhost-user-target.c
@@ -0,0 +1,29 @@
+/*
+ * vhost-user target-specific helpers
+ *
+ * Copyright (c) 2013 Virtual Open Systems Sarl.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/vhost-user.h"
+
+#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
+    defined(TARGET_ARM) || defined(TARGET_ARM_64)
+#include "hw/acpi/acpi.h"
+#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
+#include "hw/ppc/spapr.h"
+#endif
+
+unsigned int vhost_user_ram_slots_max(void)
+{
+#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
+    defined(TARGET_ARM) || defined(TARGET_ARM_64)
+    return ACPI_MAX_RAM_SLOTS;
+#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
+    return SPAPR_MAX_RAM_SLOTS;
+#else
+    return 512;

Should vhost_user_ram_slots_max be another TargetInfo field?


I don't think so, it would be better to transform the existing function
in something like:

switch (target_current()) {
case TARGET_X86:
case TARGET_ARM:
case TARGET_X86_64:
case TARGET_ARM_64:
      return ACPI_MAX_RAM_SLOTS;
case TARGET PPC:
case TARGET PPC64:
      return SPAPR_MAX_RAM_SLOTS;
default:
      return 512;
}

Clever, I like it, thanks!

It's a pattern we can reuse in all places where it'll be needed.
It's better if we keep in TargetInfo only global information, that is used through all the codebase, and not specifics about a given subsystem/device/file.

By the way, TARGET_ARM_64 is probably TARGET_AARCH64.

Correct, it has been fixed by Akihiko:

commit 744734ccc9eff28394a453de462b2a155f364118
Author: Akihiko Odaki <akihiko.od...@daynix.com>
Date:   Mon Jan 9 15:31:30 2023 +0900

    vhost-user: Correct a reference of TARGET_AARCH64

    Presumably TARGET_ARM_64 should be a mistake of TARGET_AARCH64.

    Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
    Message-Id: <20230109063130.81296-1-akihiko.od...@daynix.com>
    Fixes: 27598393a2 ("Lift max memory slots limit imposed by vhost-user")
    Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
    Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
    Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
    Signed-off-by: Michael S. Tsirkin <m...@redhat.com>

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index d9ce0501b2c..6c79da953b3 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -48,7 +48,7 @@
  * hardware plaform.
  */
 #if defined(TARGET_X86) || defined(TARGET_X86_64) || \
-    defined(TARGET_ARM) || defined(TARGET_ARM_64)
+    defined(TARGET_ARM) || defined(TARGET_AARCH64)
 #include "hw/acpi/acpi.h"
 #define VHOST_USER_MAX_RAM_SLOTS ACPI_MAX_RAM_SLOTS



Reply via email to