Re: [PATCH v2 04/16] kvm: Return number of free memslots

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

On 25/8/23 15:21, David Hildenbrand wrote:
> Let's return the number of free slots instead of only checking if there
> is a free slot. While at it, check all address spaces, which will also
> consider SMM under x86 correctly.
> 
> Make the stub return UINT_MAX, such that we can call the function
> unconditionally.
> 
> This is a preparation for memory devices that consume multiple memslots.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   accel/kvm/kvm-all.c      | 33 ++++++++++++++++++++-------------
>   accel/stubs/kvm-stub.c   |  4 ++--
>   hw/mem/memory-device.c   |  2 +-
>   include/sysemu/kvm.h     |  2 +-
>   include/sysemu/kvm_int.h |  1 +
>   5 files changed, 25 insertions(+), 17 deletions(-)


> diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
> index 235dc661bc..f39997d86e 100644
> --- a/accel/stubs/kvm-stub.c
> +++ b/accel/stubs/kvm-stub.c
> @@ -109,9 +109,9 @@ int kvm_irqchip_remove_irqfd_notifier_gsi(KVMState *s, EventNotifier *n,
>       return -ENOSYS;
>   }
>   
> -bool kvm_has_free_slot(MachineState *ms)
> +unsigned int kvm_get_free_memslots(void)
>   {
> -    return false;
> +    return UINT_MAX;

Isn't it clearer returning 0 here and keeping kvm_enabled() below?

>   }
>   
>   void kvm_init_cpu_signals(CPUState *cpu)
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index 667d56bd29..7c24685796 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -59,7 +59,7 @@ static void memory_device_check_addable(MachineState *ms, MemoryRegion *mr,
>       const uint64_t size = memory_region_size(mr);
>   
>       /* we will need a new memory slot for kvm and vhost */
> -    if (kvm_enabled() && !kvm_has_free_slot(ms)) {
> +    if (!kvm_get_free_memslots()) {

(here)

>           error_setg(errp, "hypervisor has no free memory slots left");
>           return;
>       }


> diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
> index 511b42bde5..8b09e78b12 100644
> --- a/include/sysemu/kvm_int.h
> +++ b/include/sysemu/kvm_int.h
> @@ -40,6 +40,7 @@ typedef struct KVMMemoryUpdate {
>   typedef struct KVMMemoryListener {
>       MemoryListener listener;
>       KVMSlot *slots;
> +    int nr_used_slots;

Preferably using 'unsigned' here:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>