kvm: consistently return 0/-errno from kvm_convert_memory

[PATCH] kvm: consistently return 0/-errno from kvm_convert_memory

Posted by Paolo Bonzini 3 months, 3 weeks ago

In case of incorrect parameters, kvm_convert_memory() was returning
-1 instead of -EINVAL.  The guest won't notice because it will move
anyway to RUN_STATE_INTERNAL_ERROR, but fix this for consistency and
clarity.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/kvm/kvm-all.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 4c5cef766ad..81821af7d7a 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2998,17 +2998,17 @@ int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
     ram_addr_t offset;
     MemoryRegion *mr;
     RAMBlock *rb;
-    int ret = -1;
+    int ret = -EINVAL;
 
     trace_kvm_convert_memory(start, size, to_private ? "shared_to_private" : "private_to_shared");
 
     if (!QEMU_PTR_IS_ALIGNED(start, qemu_real_host_page_size()) ||
         !QEMU_PTR_IS_ALIGNED(size, qemu_real_host_page_size())) {
-        return -1;
+        return ret;
     }
 
     if (!size) {
-        return -1;
+        return ret;
     }
 
     section = memory_region_find(get_system_memory(), start, size);
@@ -3026,7 +3026,7 @@ int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
         if (!to_private) {
             return 0;
         }
-        return -1;
+        return ret;
     }
 
     if (!memory_region_has_guest_memfd(mr)) {
-- 
2.47.1

Re: [PATCH] kvm: consistently return 0/-errno from kvm_convert_memory

Posted by Xiaoyao Li 3 months, 3 weeks ago

On 12/12/2024 11:57 PM, Paolo Bonzini wrote:
> In case of incorrect parameters, kvm_convert_memory() was returning
> -1 instead of -EINVAL.  The guest won't notice because it will move
> anyway to RUN_STATE_INTERNAL_ERROR, but fix this for consistency and
> clarity.

I think we need add more clarification about "guest".

kvm_convert_memory() is also used to service the request of 
KVM_HC_MAP_GPA_RANGE from guest since commit 47e76d03b155 ("i386/kvm: 
Add KVM_EXIT_HYPERCALL handling for KVM_HC_MAP_GPA_RANGE"). It might 
need to return error back to guest in case of incorrect parameters, in 
the future.

For the code change,

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   accel/kvm/kvm-all.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 4c5cef766ad..81821af7d7a 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2998,17 +2998,17 @@ int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
>       ram_addr_t offset;
>       MemoryRegion *mr;
>       RAMBlock *rb;
> -    int ret = -1;
> +    int ret = -EINVAL;
>   
>       trace_kvm_convert_memory(start, size, to_private ? "shared_to_private" : "private_to_shared");
>   
>       if (!QEMU_PTR_IS_ALIGNED(start, qemu_real_host_page_size()) ||
>           !QEMU_PTR_IS_ALIGNED(size, qemu_real_host_page_size())) {
> -        return -1;
> +        return ret;
>       }
>   
>       if (!size) {
> -        return -1;
> +        return ret;
>       }
>   
>       section = memory_region_find(get_system_memory(), start, size);
> @@ -3026,7 +3026,7 @@ int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
>           if (!to_private) {
>               return 0;
>           }
> -        return -1;
> +        return ret;
>       }
>   
>       if (!memory_region_has_guest_memfd(mr)) {

Re: [PATCH] kvm: consistently return 0/-errno from kvm_convert_memory

Posted by Zhao Liu 3 months, 3 weeks ago

On Thu, Dec 12, 2024 at 04:57:19PM +0100, Paolo Bonzini wrote:
> Date: Thu, 12 Dec 2024 16:57:19 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] kvm: consistently return 0/-errno from kvm_convert_memory
> X-Mailer: git-send-email 2.47.1
> 
> In case of incorrect parameters, kvm_convert_memory() was returning
> -1 instead of -EINVAL.  The guest won't notice because it will move
> anyway to RUN_STATE_INTERNAL_ERROR, but fix this for consistency and
> clarity.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  accel/kvm/kvm-all.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH] kvm: consistently return 0/-errno from kvm_convert_memory

Posted by Philippe Mathieu-Daudé 3 months, 3 weeks ago

On 12/12/24 16:57, Paolo Bonzini wrote:
> In case of incorrect parameters, kvm_convert_memory() was returning
> -1 instead of -EINVAL.  The guest won't notice because it will move
> anyway to RUN_STATE_INTERNAL_ERROR, but fix this for consistency and
> clarity.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   accel/kvm/kvm-all.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

Please include a brief docstring:

-- >8 --
diff --git a/include/system/kvm.h b/include/system/kvm.h
index c3a60b28909..923bb07d73d 100644
--- a/include/system/kvm.h
+++ b/include/system/kvm.h
@@ -576,6 +576,11 @@ int kvm_create_guest_memfd(uint64_t size, uint64_t 
flags, Error **errp);
  int kvm_set_memory_attributes_private(hwaddr start, uint64_t size);
  int kvm_set_memory_attributes_shared(hwaddr start, uint64_t size);

+/**
+ * kvm_convert_memory:
+ *
+ * Returns: 0 on success, or a negative errno on failure.
+ */
  int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private);

  #endif
---

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