Re: [PATCH v4 03/12] kvm: Provide explicit error for kvm_create_guest_memfd()

Hi Peter, Michael,

On 12/8/26 22:16, Michael Roth wrote:

From: Peter Xu <[email protected]>

So that there will be a verbal string returned when kvm not enabled, or
kvm not compiled.

Signed-off-by: Peter Xu <[email protected]>
Reviewed-by: Xiaoyao Li <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Reviewed-by: Michael Roth <[email protected]>
Signed-off-by: Michael Roth <[email protected]>
---
  accel/kvm/kvm-all.c    | 5 +++++
  accel/stubs/kvm-stub.c | 1 +
  2 files changed, 6 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 120cab1e22..bda2e25a66 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -4758,6 +4758,11 @@ int kvm_create_guest_memfd(uint64_t size, uint64_t 
flags, Error **errp)
          .flags = flags,
      };

+ if (!kvm_enabled()) {

+        error_setg(errp, "guest-memfd requires KVM accelerator");
+        return -1;

This doesn't sound right withing a KVM-specific method. We want to
assert() here.

The call in ram_block_add() is already protected:

2147 static void ram_block_add(RAMBlock *new_block, Error **errp)
2148 {
...
2185     if (new_block->flags & RAM_GUEST_MEMFD) {
2188         if (!kvm_enabled()) {

2191             goto out_free;
2192         }
...

2204                                                         0, errp);

The other one is:

2823 int ram_block_rebind(Error **errp)
2824 {
...
2829     RAMBLOCK_FOREACH(block) {
...

2835                                                         0, errp);

which is only called from KVM:

2776 static int kvm_reset_vmfd(MachineState *ms)
2777 {
...
2827     /* rebind memory to new vm fd */
2828     ret = ram_block_rebind(&err);

So maybe what we want is:

-- >8 --
diff --git a/system/physmem.c b/system/physmem.c
index b97016b1303..66ff74541aa 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2824,6 +2824,8 @@ int ram_block_rebind(Error **errp)
 {
     RAMBlock *block;

+    assert(kvm_enabled()); /* Only supported by KVM so far */
+
     qemu_mutex_lock_ramlist();

     RAMBLOCK_FOREACH(block) {
---

Or less aggressive:

-- >8 --
diff --git a/system/physmem.c b/system/physmem.c
index b97016b1303..2988d1dd6c9 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2824,6 +2824,11 @@ int ram_block_rebind(Error **errp)
 {
     RAMBlock *block;

+    if (!kvm_enabled()) {
+        error_setg(errp, "guest-memfd requires KVM accelerator");
+        return -1;
+    }
+
     qemu_mutex_lock_ramlist();

     RAMBLOCK_FOREACH(block) {
---

WDYT?

+    }
+
      if (!kvm_guest_memfd_supported) {
          error_setg(errp, "KVM does not support guest_memfd");
          return -1;
diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
index 3d34e3b99d..acbd0785e0 100644
--- a/accel/stubs/kvm-stub.c
+++ b/accel/stubs/kvm-stub.c
@@ -141,6 +141,7 @@ bool kvm_hwpoisoned_mem(void)

int kvm_create_guest_memfd(uint64_t size, uint64_t flags, Error **errp)

  {
+    error_setg(errp, "KVM is not enabled");
      return -ENOSYS;
  }