| Message ID | 20230731162201.271114-9-xiaoyao.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | QEMU gmem implemention | expand |
Xiaoyao Li <xiaoyao.li@intel.com> writes: > From: Isaku Yamahata <isaku.yamahata@intel.com> > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> [...] > diff --git a/qapi/qom.json b/qapi/qom.json > index 7f92ea43e8e1..e0b2044e3d20 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -605,6 +605,9 @@ > # @reserve: if true, reserve swap space (or huge pages) if applicable > # (default: true) (since 6.1) > # > +# @private: if true, use KVM gmem private memory > +# (default: false) (since 8.1) > +# Please format like # @private: if true, use KVM gmem private memory (default: false) # (since 8.1) to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments to conform to current conventions). > # @size: size of the memory region in bytes > # > # @x-use-canonical-path-for-ramblock-id: if true, the canonical path > @@ -631,6 +634,7 @@ > '*prealloc-context': 'str', > '*share': 'bool', > '*reserve': 'bool', > + '*private': 'bool', > 'size': 'size', > '*x-use-canonical-path-for-ramblock-id': 'bool' } }
On 8/1/2023 1:22 AM, Markus Armbruster wrote: > Xiaoyao Li <xiaoyao.li@intel.com> writes: > >> From: Isaku Yamahata <isaku.yamahata@intel.com> >> >> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > > [...] > >> diff --git a/qapi/qom.json b/qapi/qom.json >> index 7f92ea43e8e1..e0b2044e3d20 100644 >> --- a/qapi/qom.json >> +++ b/qapi/qom.json >> @@ -605,6 +605,9 @@ >> # @reserve: if true, reserve swap space (or huge pages) if applicable >> # (default: true) (since 6.1) >> # >> +# @private: if true, use KVM gmem private memory >> +# (default: false) (since 8.1) >> +# > > Please format like > > # @private: if true, use KVM gmem private memory (default: false) > # (since 8.1) > > to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments > to conform to current conventions). will do it in next version. Thanks! -Xiaoyao >> # @size: size of the memory region in bytes >> # >> # @x-use-canonical-path-for-ramblock-id: if true, the canonical path >> @@ -631,6 +634,7 @@ >> '*prealloc-context': 'str', >> '*share': 'bool', >> '*reserve': 'bool', >> + '*private': 'bool', >> 'size': 'size', >> '*x-use-canonical-path-for-ramblock-id': 'bool' } } >
On Mon, Jul 31, 2023 at 07:22:05PM +0200, Markus Armbruster wrote: > Xiaoyao Li <xiaoyao.li@intel.com> writes: > > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > > [...] > > > diff --git a/qapi/qom.json b/qapi/qom.json > > index 7f92ea43e8e1..e0b2044e3d20 100644 > > --- a/qapi/qom.json > > +++ b/qapi/qom.json > > @@ -605,6 +605,9 @@ > > # @reserve: if true, reserve swap space (or huge pages) if applicable > > # (default: true) (since 6.1) > > # > > +# @private: if true, use KVM gmem private memory > > +# (default: false) (since 8.1) > > +# > > Please format like > > # @private: if true, use KVM gmem private memory (default: false) > # (since 8.1) Also QEMU 8.1.0 is in freeze right now, so there's no chance of these patches making 8.1.0. IOW, use "since 8.2" as the next release you might achieve merge for. With regards, Daniel
On 31.07.23 18:21, Xiaoyao Li wrote: > From: Isaku Yamahata <isaku.yamahata@intel.com> > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > backends/hostmem.c | 18 ++++++++++++++++++ > include/sysemu/hostmem.h | 2 +- > qapi/qom.json | 4 ++++ > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/backends/hostmem.c b/backends/hostmem.c > index 747e7838c031..dbdbb0aafd45 100644 > --- a/backends/hostmem.c > +++ b/backends/hostmem.c > @@ -461,6 +461,20 @@ static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp) > } > backend->reserve = value; > } > + > +static bool host_memory_backend_get_private(Object *o, Error **errp) > +{ > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > + > + return backend->private; > +} > + > +static void host_memory_backend_set_private(Object *o, bool value, Error **errp) > +{ > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > + > + backend->private = value; > +} > #endif /* CONFIG_LINUX */ > > static bool > @@ -541,6 +555,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data) > host_memory_backend_get_reserve, host_memory_backend_set_reserve); > object_class_property_set_description(oc, "reserve", > "Reserve swap space (or huge pages) if applicable"); > + object_class_property_add_bool(oc, "private", > + host_memory_backend_get_private, host_memory_backend_set_private); > + object_class_property_set_description(oc, "private", > + "Use KVM gmem private memory"); > #endif /* CONFIG_LINUX */ > /* > * Do not delete/rename option. This option must be considered stable > diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h > index 39326f1d4f9c..d88970395618 100644 > --- a/include/sysemu/hostmem.h > +++ b/include/sysemu/hostmem.h > @@ -65,7 +65,7 @@ struct HostMemoryBackend { > /* protected */ > uint64_t size; > bool merge, dump, use_canonical_path; > - bool prealloc, is_mapped, share, reserve; > + bool prealloc, is_mapped, share, reserve, private; > uint32_t prealloc_threads; > ThreadContext *prealloc_context; > DECLARE_BITMAP(host_nodes, MAX_NODES + 1); > diff --git a/qapi/qom.json b/qapi/qom.json > index 7f92ea43e8e1..e0b2044e3d20 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -605,6 +605,9 @@ > # @reserve: if true, reserve swap space (or huge pages) if applicable > # (default: true) (since 6.1) > # > +# @private: if true, use KVM gmem private memory > +# (default: false) (since 8.1) > +# But that's not what any of this does. This patch only adds a property and doesn't even explain what it intends to achieve with that. How will it be used from a user? What will it affect internally? What will it modify in regards of the memory backend? That all should go into the surprisingly empty patch description.
On 8/2/2023 1:21 AM, David Hildenbrand wrote: > On 31.07.23 18:21, Xiaoyao Li wrote: >> From: Isaku Yamahata <isaku.yamahata@intel.com> >> >> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> --- >> backends/hostmem.c | 18 ++++++++++++++++++ >> include/sysemu/hostmem.h | 2 +- >> qapi/qom.json | 4 ++++ >> 3 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/backends/hostmem.c b/backends/hostmem.c >> index 747e7838c031..dbdbb0aafd45 100644 >> --- a/backends/hostmem.c >> +++ b/backends/hostmem.c >> @@ -461,6 +461,20 @@ static void >> host_memory_backend_set_reserve(Object *o, bool value, Error **errp) >> } >> backend->reserve = value; >> } >> + >> +static bool host_memory_backend_get_private(Object *o, Error **errp) >> +{ >> + HostMemoryBackend *backend = MEMORY_BACKEND(o); >> + >> + return backend->private; >> +} >> + >> +static void host_memory_backend_set_private(Object *o, bool value, >> Error **errp) >> +{ >> + HostMemoryBackend *backend = MEMORY_BACKEND(o); >> + >> + backend->private = value; >> +} >> #endif /* CONFIG_LINUX */ >> static bool >> @@ -541,6 +555,10 @@ host_memory_backend_class_init(ObjectClass *oc, >> void *data) >> host_memory_backend_get_reserve, >> host_memory_backend_set_reserve); >> object_class_property_set_description(oc, "reserve", >> "Reserve swap space (or huge pages) if applicable"); >> + object_class_property_add_bool(oc, "private", >> + host_memory_backend_get_private, >> host_memory_backend_set_private); >> + object_class_property_set_description(oc, "private", >> + "Use KVM gmem private memory"); >> #endif /* CONFIG_LINUX */ >> /* >> * Do not delete/rename option. This option must be considered >> stable >> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h >> index 39326f1d4f9c..d88970395618 100644 >> --- a/include/sysemu/hostmem.h >> +++ b/include/sysemu/hostmem.h >> @@ -65,7 +65,7 @@ struct HostMemoryBackend { >> /* protected */ >> uint64_t size; >> bool merge, dump, use_canonical_path; >> - bool prealloc, is_mapped, share, reserve; >> + bool prealloc, is_mapped, share, reserve, private; >> uint32_t prealloc_threads; >> ThreadContext *prealloc_context; >> DECLARE_BITMAP(host_nodes, MAX_NODES + 1); >> diff --git a/qapi/qom.json b/qapi/qom.json >> index 7f92ea43e8e1..e0b2044e3d20 100644 >> --- a/qapi/qom.json >> +++ b/qapi/qom.json >> @@ -605,6 +605,9 @@ >> # @reserve: if true, reserve swap space (or huge pages) if applicable >> # (default: true) (since 6.1) >> # >> +# @private: if true, use KVM gmem private memory >> +# (default: false) (since 8.1) >> +# > > But that's not what any of this does. > > This patch only adds a property and doesn't even explain what it intends > to achieve with that. > > How will it be used from a user? What will it affect internally? What > will it modify in regards of the memory backend? How it will be used is in the next patch, patch 09. for kvm_x86_sw_protected_vm type VM, it will allocate private gmem with KVM ioctl if the memory backend has property "private" on. > That all should go into the surprisingly empty patch description. I'm sorry. I admit the empty commit message is really bad.
On 8/1/2023 10:57 PM, Daniel P. Berrangé wrote: > On Mon, Jul 31, 2023 at 07:22:05PM +0200, Markus Armbruster wrote: >> Xiaoyao Li <xiaoyao.li@intel.com> writes: >> >>> From: Isaku Yamahata <isaku.yamahata@intel.com> >>> >>> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> >>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> >> [...] >> >>> diff --git a/qapi/qom.json b/qapi/qom.json >>> index 7f92ea43e8e1..e0b2044e3d20 100644 >>> --- a/qapi/qom.json >>> +++ b/qapi/qom.json >>> @@ -605,6 +605,9 @@ >>> # @reserve: if true, reserve swap space (or huge pages) if applicable >>> # (default: true) (since 6.1) >>> # >>> +# @private: if true, use KVM gmem private memory >>> +# (default: false) (since 8.1) >>> +# >> >> Please format like >> >> # @private: if true, use KVM gmem private memory (default: false) >> # (since 8.1) > > Also QEMU 8.1.0 is in freeze right now, so there's no chance > of these patches making 8.1.0. IOW, use "since 8.2" as the > next release you might achieve merge for. Thanks for pointing it out. Will do it in next version. > With regards, > Daniel
On 02.08.23 10:03, Xiaoyao Li wrote: > On 8/2/2023 1:21 AM, David Hildenbrand wrote: >> On 31.07.23 18:21, Xiaoyao Li wrote: >>> From: Isaku Yamahata <isaku.yamahata@intel.com> >>> >>> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> >>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >>> --- >>> backends/hostmem.c | 18 ++++++++++++++++++ >>> include/sysemu/hostmem.h | 2 +- >>> qapi/qom.json | 4 ++++ >>> 3 files changed, 23 insertions(+), 1 deletion(-) >>> >>> diff --git a/backends/hostmem.c b/backends/hostmem.c >>> index 747e7838c031..dbdbb0aafd45 100644 >>> --- a/backends/hostmem.c >>> +++ b/backends/hostmem.c >>> @@ -461,6 +461,20 @@ static void >>> host_memory_backend_set_reserve(Object *o, bool value, Error **errp) >>> } >>> backend->reserve = value; >>> } >>> + >>> +static bool host_memory_backend_get_private(Object *o, Error **errp) >>> +{ >>> + HostMemoryBackend *backend = MEMORY_BACKEND(o); >>> + >>> + return backend->private; >>> +} >>> + >>> +static void host_memory_backend_set_private(Object *o, bool value, >>> Error **errp) >>> +{ >>> + HostMemoryBackend *backend = MEMORY_BACKEND(o); >>> + >>> + backend->private = value; >>> +} >>> #endif /* CONFIG_LINUX */ >>> static bool >>> @@ -541,6 +555,10 @@ host_memory_backend_class_init(ObjectClass *oc, >>> void *data) >>> host_memory_backend_get_reserve, >>> host_memory_backend_set_reserve); >>> object_class_property_set_description(oc, "reserve", >>> "Reserve swap space (or huge pages) if applicable"); >>> + object_class_property_add_bool(oc, "private", >>> + host_memory_backend_get_private, >>> host_memory_backend_set_private); >>> + object_class_property_set_description(oc, "private", >>> + "Use KVM gmem private memory"); >>> #endif /* CONFIG_LINUX */ >>> /* >>> * Do not delete/rename option. This option must be considered >>> stable >>> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h >>> index 39326f1d4f9c..d88970395618 100644 >>> --- a/include/sysemu/hostmem.h >>> +++ b/include/sysemu/hostmem.h >>> @@ -65,7 +65,7 @@ struct HostMemoryBackend { >>> /* protected */ >>> uint64_t size; >>> bool merge, dump, use_canonical_path; >>> - bool prealloc, is_mapped, share, reserve; >>> + bool prealloc, is_mapped, share, reserve, private; >>> uint32_t prealloc_threads; >>> ThreadContext *prealloc_context; >>> DECLARE_BITMAP(host_nodes, MAX_NODES + 1); >>> diff --git a/qapi/qom.json b/qapi/qom.json >>> index 7f92ea43e8e1..e0b2044e3d20 100644 >>> --- a/qapi/qom.json >>> +++ b/qapi/qom.json >>> @@ -605,6 +605,9 @@ >>> # @reserve: if true, reserve swap space (or huge pages) if applicable >>> # (default: true) (since 6.1) >>> # >>> +# @private: if true, use KVM gmem private memory >>> +# (default: false) (since 8.1) >>> +# >> >> But that's not what any of this does. >> >> This patch only adds a property and doesn't even explain what it intends >> to achieve with that. >> >> How will it be used from a user? What will it affect internally? What >> will it modify in regards of the memory backend? > > How it will be used is in the next patch, patch 09. > > for kvm_x86_sw_protected_vm type VM, it will allocate private gmem with > KVM ioctl if the memory backend has property "private" on. It feels wired up the wrong way. When creating/initializing the memory backend, we should also take care of allocating the gmem_fd, for example, by doing some gmem allocation callback, ideally *internally* creating the RAM memory region / RAMBlock. And we should fail if that is impossible (gmem does not apply to the VM) or creating the gmem_fd failed for other reason. Like passing a RAM_GMEM flag to memory_region_init_ram_flags_nomigrate() in ram_backend_memory_alloc(), to then handle it internally, failing if there is an error.
On Wed, Aug 02, 2023 at 04:14:29PM +0200, David Hildenbrand <david@redhat.com> wrote: > On 02.08.23 10:03, Xiaoyao Li wrote: > > On 8/2/2023 1:21 AM, David Hildenbrand wrote: > > > On 31.07.23 18:21, Xiaoyao Li wrote: > > > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > > > > > > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > > > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > > > > --- > > > > backends/hostmem.c | 18 ++++++++++++++++++ > > > > include/sysemu/hostmem.h | 2 +- > > > > qapi/qom.json | 4 ++++ > > > > 3 files changed, 23 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/backends/hostmem.c b/backends/hostmem.c > > > > index 747e7838c031..dbdbb0aafd45 100644 > > > > --- a/backends/hostmem.c > > > > +++ b/backends/hostmem.c > > > > @@ -461,6 +461,20 @@ static void > > > > host_memory_backend_set_reserve(Object *o, bool value, Error **errp) > > > > } > > > > backend->reserve = value; > > > > } > > > > + > > > > +static bool host_memory_backend_get_private(Object *o, Error **errp) > > > > +{ > > > > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > > > > + > > > > + return backend->private; > > > > +} > > > > + > > > > +static void host_memory_backend_set_private(Object *o, bool value, > > > > Error **errp) > > > > +{ > > > > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > > > > + > > > > + backend->private = value; > > > > +} > > > > #endif /* CONFIG_LINUX */ > > > > static bool > > > > @@ -541,6 +555,10 @@ host_memory_backend_class_init(ObjectClass *oc, > > > > void *data) > > > > host_memory_backend_get_reserve, > > > > host_memory_backend_set_reserve); > > > > object_class_property_set_description(oc, "reserve", > > > > "Reserve swap space (or huge pages) if applicable"); > > > > + object_class_property_add_bool(oc, "private", > > > > + host_memory_backend_get_private, > > > > host_memory_backend_set_private); > > > > + object_class_property_set_description(oc, "private", > > > > + "Use KVM gmem private memory"); > > > > #endif /* CONFIG_LINUX */ > > > > /* > > > > * Do not delete/rename option. This option must be considered > > > > stable > > > > diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h > > > > index 39326f1d4f9c..d88970395618 100644 > > > > --- a/include/sysemu/hostmem.h > > > > +++ b/include/sysemu/hostmem.h > > > > @@ -65,7 +65,7 @@ struct HostMemoryBackend { > > > > /* protected */ > > > > uint64_t size; > > > > bool merge, dump, use_canonical_path; > > > > - bool prealloc, is_mapped, share, reserve; > > > > + bool prealloc, is_mapped, share, reserve, private; > > > > uint32_t prealloc_threads; > > > > ThreadContext *prealloc_context; > > > > DECLARE_BITMAP(host_nodes, MAX_NODES + 1); > > > > diff --git a/qapi/qom.json b/qapi/qom.json > > > > index 7f92ea43e8e1..e0b2044e3d20 100644 > > > > --- a/qapi/qom.json > > > > +++ b/qapi/qom.json > > > > @@ -605,6 +605,9 @@ > > > > # @reserve: if true, reserve swap space (or huge pages) if applicable > > > > # (default: true) (since 6.1) > > > > # > > > > +# @private: if true, use KVM gmem private memory > > > > +# (default: false) (since 8.1) > > > > +# > > > > > > But that's not what any of this does. > > > > > > This patch only adds a property and doesn't even explain what it intends > > > to achieve with that. > > > > > > How will it be used from a user? What will it affect internally? What > > > will it modify in regards of the memory backend? > > > > How it will be used is in the next patch, patch 09. > > > > for kvm_x86_sw_protected_vm type VM, it will allocate private gmem with > > KVM ioctl if the memory backend has property "private" on. > > It feels wired up the wrong way. > > When creating/initializing the memory backend, we should also take care of > allocating the gmem_fd, for example, by doing some gmem allocation callback, > ideally *internally* creating the RAM memory region / RAMBlock. > > And we should fail if that is impossible (gmem does not apply to the VM) or > creating the gmem_fd failed for other reason. > > Like passing a RAM_GMEM flag to memory_region_init_ram_flags_nomigrate() in > ram_backend_memory_alloc(), to then handle it internally, failing if there > is an error. KVM gmem is tied to VM. not before creating VM. We have to delay of the allocation of kvm gmem until VM initialization. Hmm, one options is to move gmem_fd from RAMBlock to KVMSlot. Handle the allocation of KVM gmem (issuing KVM gmem ioctl) there. i.e. in kvm_set_phys_mem() or kvm_region_add() (or whatever functions of KVM memory listener). Maybe we can drop ram_block_convert_range() and can have KVM specific logic instead. We still need a way for user to specify which guest memory region is subject to KVM gmem, though.
On 03.08.23 00:53, Isaku Yamahata wrote: > On Wed, Aug 02, 2023 at 04:14:29PM +0200, > David Hildenbrand <david@redhat.com> wrote: > >> On 02.08.23 10:03, Xiaoyao Li wrote: >>> On 8/2/2023 1:21 AM, David Hildenbrand wrote: >>>> On 31.07.23 18:21, Xiaoyao Li wrote: >>>>> From: Isaku Yamahata <isaku.yamahata@intel.com> >>>>> >>>>> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> >>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >>>>> --- >>>>> backends/hostmem.c | 18 ++++++++++++++++++ >>>>> include/sysemu/hostmem.h | 2 +- >>>>> qapi/qom.json | 4 ++++ >>>>> 3 files changed, 23 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/backends/hostmem.c b/backends/hostmem.c >>>>> index 747e7838c031..dbdbb0aafd45 100644 >>>>> --- a/backends/hostmem.c >>>>> +++ b/backends/hostmem.c >>>>> @@ -461,6 +461,20 @@ static void >>>>> host_memory_backend_set_reserve(Object *o, bool value, Error **errp) >>>>> } >>>>> backend->reserve = value; >>>>> } >>>>> + >>>>> +static bool host_memory_backend_get_private(Object *o, Error **errp) >>>>> +{ >>>>> + HostMemoryBackend *backend = MEMORY_BACKEND(o); >>>>> + >>>>> + return backend->private; >>>>> +} >>>>> + >>>>> +static void host_memory_backend_set_private(Object *o, bool value, >>>>> Error **errp) >>>>> +{ >>>>> + HostMemoryBackend *backend = MEMORY_BACKEND(o); >>>>> + >>>>> + backend->private = value; >>>>> +} >>>>> #endif /* CONFIG_LINUX */ >>>>> static bool >>>>> @@ -541,6 +555,10 @@ host_memory_backend_class_init(ObjectClass *oc, >>>>> void *data) >>>>> host_memory_backend_get_reserve, >>>>> host_memory_backend_set_reserve); >>>>> object_class_property_set_description(oc, "reserve", >>>>> "Reserve swap space (or huge pages) if applicable"); >>>>> + object_class_property_add_bool(oc, "private", >>>>> + host_memory_backend_get_private, >>>>> host_memory_backend_set_private); >>>>> + object_class_property_set_description(oc, "private", >>>>> + "Use KVM gmem private memory"); >>>>> #endif /* CONFIG_LINUX */ >>>>> /* >>>>> * Do not delete/rename option. This option must be considered >>>>> stable >>>>> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h >>>>> index 39326f1d4f9c..d88970395618 100644 >>>>> --- a/include/sysemu/hostmem.h >>>>> +++ b/include/sysemu/hostmem.h >>>>> @@ -65,7 +65,7 @@ struct HostMemoryBackend { >>>>> /* protected */ >>>>> uint64_t size; >>>>> bool merge, dump, use_canonical_path; >>>>> - bool prealloc, is_mapped, share, reserve; >>>>> + bool prealloc, is_mapped, share, reserve, private; >>>>> uint32_t prealloc_threads; >>>>> ThreadContext *prealloc_context; >>>>> DECLARE_BITMAP(host_nodes, MAX_NODES + 1); >>>>> diff --git a/qapi/qom.json b/qapi/qom.json >>>>> index 7f92ea43e8e1..e0b2044e3d20 100644 >>>>> --- a/qapi/qom.json >>>>> +++ b/qapi/qom.json >>>>> @@ -605,6 +605,9 @@ >>>>> # @reserve: if true, reserve swap space (or huge pages) if applicable >>>>> # (default: true) (since 6.1) >>>>> # >>>>> +# @private: if true, use KVM gmem private memory >>>>> +# (default: false) (since 8.1) >>>>> +# >>>> >>>> But that's not what any of this does. >>>> >>>> This patch only adds a property and doesn't even explain what it intends >>>> to achieve with that. >>>> >>>> How will it be used from a user? What will it affect internally? What >>>> will it modify in regards of the memory backend? >>> >>> How it will be used is in the next patch, patch 09. >>> >>> for kvm_x86_sw_protected_vm type VM, it will allocate private gmem with >>> KVM ioctl if the memory backend has property "private" on. >> >> It feels wired up the wrong way. >> >> When creating/initializing the memory backend, we should also take care of >> allocating the gmem_fd, for example, by doing some gmem allocation callback, >> ideally *internally* creating the RAM memory region / RAMBlock. >> >> And we should fail if that is impossible (gmem does not apply to the VM) or >> creating the gmem_fd failed for other reason. >> >> Like passing a RAM_GMEM flag to memory_region_init_ram_flags_nomigrate() in >> ram_backend_memory_alloc(), to then handle it internally, failing if there >> is an error. > > KVM gmem is tied to VM. not before creating VM. We have to delay of the > allocation of kvm gmem until VM initialization. In vl.c, the flow is 1) Create machine: qemu_create_machine() 2) Configure KVM: configure_accelerators() 3) Create backends: qemu_create_late_backends() Staring at object_create_early(), "memory-backend-" area always late. So maybe, at the time memory backends are created+initialized, everything you need is already in place. > > Hmm, one options is to move gmem_fd from RAMBlock to KVMSlot. Handle the > allocation of KVM gmem (issuing KVM gmem ioctl) there. i.e. in > kvm_set_phys_mem() or kvm_region_add() (or whatever functions of KVM memory > listener). Maybe we can drop ram_block_convert_range() and can have KVM > specific logic instead. Might be doable as well. > > We still need a way for user to specify which guest memory region is subject > to KVM gmem, though. Let's minimize the gmem hacks and come up with something clean.
diff --git a/backends/hostmem.c b/backends/hostmem.c index 747e7838c031..dbdbb0aafd45 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -461,6 +461,20 @@ static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp) } backend->reserve = value; } + +static bool host_memory_backend_get_private(Object *o, Error **errp) +{ + HostMemoryBackend *backend = MEMORY_BACKEND(o); + + return backend->private; +} + +static void host_memory_backend_set_private(Object *o, bool value, Error **errp) +{ + HostMemoryBackend *backend = MEMORY_BACKEND(o); + + backend->private = value; +} #endif /* CONFIG_LINUX */ static bool @@ -541,6 +555,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data) host_memory_backend_get_reserve, host_memory_backend_set_reserve); object_class_property_set_description(oc, "reserve", "Reserve swap space (or huge pages) if applicable"); + object_class_property_add_bool(oc, "private", + host_memory_backend_get_private, host_memory_backend_set_private); + object_class_property_set_description(oc, "private", + "Use KVM gmem private memory"); #endif /* CONFIG_LINUX */ /* * Do not delete/rename option. This option must be considered stable diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h index 39326f1d4f9c..d88970395618 100644 --- a/include/sysemu/hostmem.h +++ b/include/sysemu/hostmem.h @@ -65,7 +65,7 @@ struct HostMemoryBackend { /* protected */ uint64_t size; bool merge, dump, use_canonical_path; - bool prealloc, is_mapped, share, reserve; + bool prealloc, is_mapped, share, reserve, private; uint32_t prealloc_threads; ThreadContext *prealloc_context; DECLARE_BITMAP(host_nodes, MAX_NODES + 1); diff --git a/qapi/qom.json b/qapi/qom.json index 7f92ea43e8e1..e0b2044e3d20 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -605,6 +605,9 @@ # @reserve: if true, reserve swap space (or huge pages) if applicable # (default: true) (since 6.1) # +# @private: if true, use KVM gmem private memory +# (default: false) (since 8.1) +# # @size: size of the memory region in bytes # # @x-use-canonical-path-for-ramblock-id: if true, the canonical path @@ -631,6 +634,7 @@ '*prealloc-context': 'str', '*share': 'bool', '*reserve': 'bool', + '*private': 'bool', 'size': 'size', '*x-use-canonical-path-for-ramblock-id': 'bool' } }