diff mbox series

[v3,04/70] HostMem: Add mechanism to opt in kvm guest memfd via MachineState

Message ID 20231115071519.2864957-5-xiaoyao.li@intel.com (mailing list archive)
State New, archived
Headers show
Series QEMU Guest memfd + QEMU TDX support | expand

Commit Message

Xiaoyao Li Nov. 15, 2023, 7:14 a.m. UTC
Add a new member "require_guest_memfd" to memory backends. When it's set
to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm
guest_memfd will be allocated during RAMBlock allocation.

Memory backend's @require_guest_memfd is wired with @require_guest_memfd
field of MachineState. MachineState::require_guest_memfd is supposed to
be set by any VMs that requires KVM guest memfd as private memory, e.g.,
TDX VM.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 backends/hostmem-file.c  | 1 +
 backends/hostmem-memfd.c | 1 +
 backends/hostmem-ram.c   | 1 +
 backends/hostmem.c       | 1 +
 hw/core/machine.c        | 5 +++++
 include/hw/boards.h      | 2 ++
 include/sysemu/hostmem.h | 1 +
 7 files changed, 12 insertions(+)

Comments

David Hildenbrand Nov. 15, 2023, 6:14 p.m. UTC | #1
On 15.11.23 08:14, Xiaoyao Li wrote:
> Add a new member "require_guest_memfd" to memory backends. When it's set
> to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm
> guest_memfd will be allocated during RAMBlock allocation.
> 
> Memory backend's @require_guest_memfd is wired with @require_guest_memfd
> field of MachineState. MachineState::require_guest_memfd is supposed to
> be set by any VMs that requires KVM guest memfd as private memory, e.g.,
> TDX VM.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

I'm confused, why do we need this if it's going to be the same for all 
memory backends right now?
Xiaoyao Li Nov. 16, 2023, 2:53 a.m. UTC | #2
On 11/16/2023 2:14 AM, David Hildenbrand wrote:
> On 15.11.23 08:14, Xiaoyao Li wrote:
>> Add a new member "require_guest_memfd" to memory backends. When it's set
>> to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm
>> guest_memfd will be allocated during RAMBlock allocation.
>>
>> Memory backend's @require_guest_memfd is wired with @require_guest_memfd
>> field of MachineState. MachineState::require_guest_memfd is supposed to
>> be set by any VMs that requires KVM guest memfd as private memory, e.g.,
>> TDX VM.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> 
> I'm confused, why do we need this if it's going to be the same for all 
> memory backends right now?
> 

I want to provide a elegant (in my sense) way to configure "the need of 
guest memfd" instead of checking x86machinestate->vm_type in physmem.c
David Hildenbrand Nov. 20, 2023, 9:30 a.m. UTC | #3
On 16.11.23 03:53, Xiaoyao Li wrote:
> On 11/16/2023 2:14 AM, David Hildenbrand wrote:
>> On 15.11.23 08:14, Xiaoyao Li wrote:
>>> Add a new member "require_guest_memfd" to memory backends. When it's set
>>> to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm
>>> guest_memfd will be allocated during RAMBlock allocation.
>>>
>>> Memory backend's @require_guest_memfd is wired with @require_guest_memfd
>>> field of MachineState. MachineState::require_guest_memfd is supposed to
>>> be set by any VMs that requires KVM guest memfd as private memory, e.g.,
>>> TDX VM.
>>>
>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>
>> I'm confused, why do we need this if it's going to be the same for all
>> memory backends right now?
>>
> 
> I want to provide a elegant (in my sense) way to configure "the need of
> guest memfd" instead of checking x86machinestate->vm_type in physmem.c
> 

It's suboptimal right now, but I guess you want to avoid looking up the 
machine e.g., in ram_backend_memory_alloc().

I'd suggest s/require_guest_memfd/guest_memfd/gc in "struct 
HostMemoryBackend".

Apart from that LGTM.
Xiaoyao Li Nov. 30, 2023, 7:38 a.m. UTC | #4
On 11/20/2023 5:30 PM, David Hildenbrand wrote:
> On 16.11.23 03:53, Xiaoyao Li wrote:
>> On 11/16/2023 2:14 AM, David Hildenbrand wrote:
>>> On 15.11.23 08:14, Xiaoyao Li wrote:
>>>> Add a new member "require_guest_memfd" to memory backends. When it's 
>>>> set
>>>> to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm
>>>> guest_memfd will be allocated during RAMBlock allocation.
>>>>
>>>> Memory backend's @require_guest_memfd is wired with 
>>>> @require_guest_memfd
>>>> field of MachineState. MachineState::require_guest_memfd is supposed to
>>>> be set by any VMs that requires KVM guest memfd as private memory, 
>>>> e.g.,
>>>> TDX VM.
>>>>
>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>>
>>> I'm confused, why do we need this if it's going to be the same for all
>>> memory backends right now?
>>>
>>
>> I want to provide a elegant (in my sense) way to configure "the need of
>> guest memfd" instead of checking x86machinestate->vm_type in physmem.c
>>
> 
> It's suboptimal right now, but I guess you want to avoid looking up the 
> machine e.g., in ram_backend_memory_alloc().
> 
> I'd suggest s/require_guest_memfd/guest_memfd/gc in "struct 
> HostMemoryBackend".

sure!

> Apart from that LGTM.
>
diff mbox series

Patch

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 361d4a8103ef..d5ea2879f321 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -84,6 +84,7 @@  file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     ram_flags |= fb->readonly ? RAM_READONLY_FD : 0;
     ram_flags |= fb->rom == ON_OFF_AUTO_ON ? RAM_READONLY : 0;
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->require_guest_memfd ? RAM_GUEST_MEMFD : 0;
     ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
     ram_flags |= RAM_NAMED_FILE;
     memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index 3fc85c3db81b..011e2311f088 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -55,6 +55,7 @@  memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     name = host_memory_backend_get_name(backend);
     ram_flags = backend->share ? RAM_SHARED : 0;
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->require_guest_memfd ? RAM_GUEST_MEMFD : 0;
     memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
                                    backend->size, ram_flags, fd, 0, errp);
     g_free(name);
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index b8e55cdbd0f8..7d2e1327f8c8 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -30,6 +30,7 @@  ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     name = host_memory_backend_get_name(backend);
     ram_flags = backend->share ? RAM_SHARED : 0;
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->require_guest_memfd ? RAM_GUEST_MEMFD : 0;
     memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend), name,
                                            backend->size, ram_flags, errp);
     g_free(name);
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 747e7838c031..2deb2b78bcb8 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -279,6 +279,7 @@  static void host_memory_backend_init(Object *obj)
     /* TODO: convert access to globals to compat properties */
     backend->merge = machine_mem_merge(machine);
     backend->dump = machine_dump_guest_core(machine);
+    backend->require_guest_memfd = machine_require_guest_memfd(machine);
     backend->reserve = true;
     backend->prealloc_threads = machine->smp.cpus;
 }
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0c1739814124..b1b0a46ea52f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1189,6 +1189,11 @@  bool machine_mem_merge(MachineState *machine)
     return machine->mem_merge;
 }
 
+bool machine_require_guest_memfd(MachineState *machine)
+{
+    return machine->require_guest_memfd;
+}
+
 static char *cpu_slot_to_string(const CPUArchId *cpu)
 {
     GString *s = g_string_new(NULL);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index a7359992980a..227aded07209 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -30,6 +30,7 @@  bool machine_usb(MachineState *machine);
 int machine_phandle_start(MachineState *machine);
 bool machine_dump_guest_core(MachineState *machine);
 bool machine_mem_merge(MachineState *machine);
+bool machine_require_guest_memfd(MachineState *machine);
 HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine);
 void machine_set_cpu_numa_node(MachineState *machine,
                                const CpuInstanceProperties *props,
@@ -364,6 +365,7 @@  struct MachineState {
     char *dt_compatible;
     bool dump_guest_core;
     bool mem_merge;
+    bool require_guest_memfd;
     bool usb;
     bool usb_disabled;
     char *firmware;
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 39326f1d4f9c..92f7fd469639 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -66,6 +66,7 @@  struct HostMemoryBackend {
     uint64_t size;
     bool merge, dump, use_canonical_path;
     bool prealloc, is_mapped, share, reserve;
+    bool require_guest_memfd;
     uint32_t prealloc_threads;
     ThreadContext *prealloc_context;
     DECLARE_BITMAP(host_nodes, MAX_NODES + 1);