diff mbox series

[v2,36/58] memory: Introduce memory_region_init_ram_gmem()

Message ID 20230818095041.1973309-37-xiaoyao.li@intel.com (mailing list archive)
State New, archived
Headers show
Series TDX QEMU support | expand

Commit Message

Xiaoyao Li Aug. 18, 2023, 9:50 a.m. UTC
Introduce memory_region_init_ram_gmem() to allocate private gmem on the
MemoryRegion initialization. It's for the usercase of TDVF, which must
be private on TDX case.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 include/exec/memory.h |  6 +++++
 softmmu/memory.c      | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

Comments

Daniel P. Berrangé Aug. 21, 2023, 9:40 a.m. UTC | #1
On Fri, Aug 18, 2023 at 05:50:19AM -0400, Xiaoyao Li wrote:
> Introduce memory_region_init_ram_gmem() to allocate private gmem on the
> MemoryRegion initialization. It's for the usercase of TDVF, which must
> be private on TDX case.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  include/exec/memory.h |  6 +++++
>  softmmu/memory.c      | 52 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)

> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index af6aa3c1e3c9..ded44dcef1aa 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -25,6 +25,7 @@
>  #include "qom/object.h"
>  #include "trace.h"
>  
> +#include <linux/kvm.h>
>  #include "exec/memory-internal.h"
>  #include "exec/ram_addr.h"
>  #include "sysemu/kvm.h"
> @@ -3602,6 +3603,57 @@ void memory_region_init_ram(MemoryRegion *mr,
>      vmstate_register_ram(mr, owner_dev);
>  }
>  
> +#ifdef CONFIG_KVM
> +void memory_region_init_ram_gmem(MemoryRegion *mr,
> +                                 Object *owner,
> +                                 const char *name,
> +                                 uint64_t size,
> +                                 Error **errp)

Since you have an 'errp' parameter here....

> +{
> +    DeviceState *owner_dev;
> +    Error *err = NULL;
> +    int priv_fd;
> +
> +    memory_region_init_ram_nomigrate(mr, owner, name, size, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    if (object_dynamic_cast(OBJECT(current_accel()), TYPE_KVM_ACCEL)) {
> +        KVMState *s = KVM_STATE(current_accel());
> +        struct kvm_create_guest_memfd gmem = {
> +            .size = size,
> +            /* TODO: add property to hostmem backend for huge pmd */
> +            .flags = KVM_GUEST_MEMFD_ALLOW_HUGEPAGE,
> +        };
> +
> +        priv_fd = kvm_vm_ioctl(s, KVM_CREATE_GUEST_MEMFD, &gmem);
> +        if (priv_fd < 0) {
> +            fprintf(stderr, "%s: error creating gmem: %s\n", __func__,
> +                    strerror(-priv_fd));
> +            abort();

It should be using error_setg_errno() here and return not abort

> +        }
> +    } else {
> +        fprintf(stderr, "%s: gmem unsupported accel: %s\n", __func__,
> +                current_accel_name());

and error_setg() here and return.

> +        abort();
> +    }
> +
> +    memory_region_set_gmem_fd(mr, priv_fd);
> +    memory_region_set_default_private(mr);
> +
> +    /* This will assert if owner is neither NULL nor a DeviceState.
> +     * We only want the owner here for the purposes of defining a
> +     * unique name for migration. TODO: Ideally we should implement
> +     * a naming scheme for Objects which are not DeviceStates, in
> +     * which case we can relax this restriction.
> +     */
> +    owner_dev = DEVICE(owner);
> +    vmstate_register_ram(mr, owner_dev);
> +}
> +#endif
> +
>  void memory_region_init_rom(MemoryRegion *mr,
>                              Object *owner,
>                              const char *name,
> -- 
> 2.34.1
> 

With regards,
Daniel
Philippe Mathieu-Daudé Aug. 29, 2023, 2:33 p.m. UTC | #2
On 18/8/23 11:50, Xiaoyao Li wrote:
> Introduce memory_region_init_ram_gmem() to allocate private gmem on the
> MemoryRegion initialization. It's for the usercase of TDVF, which must
> be private on TDX case.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>   include/exec/memory.h |  6 +++++
>   softmmu/memory.c      | 52 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 58 insertions(+)


> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index af6aa3c1e3c9..ded44dcef1aa 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -25,6 +25,7 @@
>   #include "qom/object.h"
>   #include "trace.h"
>   
> +#include <linux/kvm.h>

Unlikely to build on non-Linux hosts.

>   #include "exec/memory-internal.h"
>   #include "exec/ram_addr.h"
>   #include "sysemu/kvm.h"
Xiaoyao Li Aug. 30, 2023, 1:53 a.m. UTC | #3
On 8/29/2023 10:33 PM, Philippe Mathieu-Daudé wrote:
> On 18/8/23 11:50, Xiaoyao Li wrote:
>> Introduce memory_region_init_ram_gmem() to allocate private gmem on the
>> MemoryRegion initialization. It's for the usercase of TDVF, which must
>> be private on TDX case.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   include/exec/memory.h |  6 +++++
>>   softmmu/memory.c      | 52 +++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 58 insertions(+)
> 
> 
>> diff --git a/softmmu/memory.c b/softmmu/memory.c
>> index af6aa3c1e3c9..ded44dcef1aa 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -25,6 +25,7 @@
>>   #include "qom/object.h"
>>   #include "trace.h"
>> +#include <linux/kvm.h>
> 
> Unlikely to build on non-Linux hosts.

Thanks for catching it!

Will warp it with CONFIG_KVM.

Anyway, it's the main open of how to integrating KVM gmem into QEMU's 
memory system, in QMEU gmem series[*]. I'm still working on it.

[*] 
https://lore.kernel.org/qemu-devel/20230731162201.271114-1-xiaoyao.li@intel.com/

>>   #include "exec/memory-internal.h"
>>   #include "exec/ram_addr.h"
>>   #include "sysemu/kvm.h"
>
diff mbox series

Patch

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 759f797b6acd..127ffb6556b9 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1564,6 +1564,12 @@  void memory_region_init_ram(MemoryRegion *mr,
                             uint64_t size,
                             Error **errp);
 
+void memory_region_init_ram_gmem(MemoryRegion *mr,
+                                 Object *owner,
+                                 const char *name,
+                                 uint64_t size,
+                                 Error **errp);
+
 /**
  * memory_region_init_rom: Initialize a ROM memory region.
  *
diff --git a/softmmu/memory.c b/softmmu/memory.c
index af6aa3c1e3c9..ded44dcef1aa 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -25,6 +25,7 @@ 
 #include "qom/object.h"
 #include "trace.h"
 
+#include <linux/kvm.h>
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
 #include "sysemu/kvm.h"
@@ -3602,6 +3603,57 @@  void memory_region_init_ram(MemoryRegion *mr,
     vmstate_register_ram(mr, owner_dev);
 }
 
+#ifdef CONFIG_KVM
+void memory_region_init_ram_gmem(MemoryRegion *mr,
+                                 Object *owner,
+                                 const char *name,
+                                 uint64_t size,
+                                 Error **errp)
+{
+    DeviceState *owner_dev;
+    Error *err = NULL;
+    int priv_fd;
+
+    memory_region_init_ram_nomigrate(mr, owner, name, size, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    if (object_dynamic_cast(OBJECT(current_accel()), TYPE_KVM_ACCEL)) {
+        KVMState *s = KVM_STATE(current_accel());
+        struct kvm_create_guest_memfd gmem = {
+            .size = size,
+            /* TODO: add property to hostmem backend for huge pmd */
+            .flags = KVM_GUEST_MEMFD_ALLOW_HUGEPAGE,
+        };
+
+        priv_fd = kvm_vm_ioctl(s, KVM_CREATE_GUEST_MEMFD, &gmem);
+        if (priv_fd < 0) {
+            fprintf(stderr, "%s: error creating gmem: %s\n", __func__,
+                    strerror(-priv_fd));
+            abort();
+        }
+    } else {
+        fprintf(stderr, "%s: gmem unsupported accel: %s\n", __func__,
+                current_accel_name());
+        abort();
+    }
+
+    memory_region_set_gmem_fd(mr, priv_fd);
+    memory_region_set_default_private(mr);
+
+    /* This will assert if owner is neither NULL nor a DeviceState.
+     * We only want the owner here for the purposes of defining a
+     * unique name for migration. TODO: Ideally we should implement
+     * a naming scheme for Objects which are not DeviceStates, in
+     * which case we can relax this restriction.
+     */
+    owner_dev = DEVICE(owner);
+    vmstate_register_ram(mr, owner_dev);
+}
+#endif
+
 void memory_region_init_rom(MemoryRegion *mr,
                             Object *owner,
                             const char *name,