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

Philippe Mathieu-Daudé
2023-08-28T22:27:23Z

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>