Re: [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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é <philmd@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!