Re: [RFC PATCH v2 03/21] HostMem: Add private property and associate it with RAM_KVM_GMEM

[Xiaoyao Li]

On 9/20/2023 11:42 PM, Markus Armbruster wrote:
> David Hildenbrand <david@redhat.com> writes:
>
> > On 20.09.23 16:35, Xiaoyao Li wrote:
> > > On 9/20/2023 3:30 PM, Markus Armbruster wrote:
> > > > Xiaoyao Li <xiaoyao.li@intel.com> writes:
> > > >
> > > > > On 9/19/2023 5:46 PM, Markus Armbruster wrote:
> > > > > > Xiaoyao Li <xiaoyao.li@intel.com> writes:
> > > > > >
> > > > > > > From: Isaku Yamahata <isaku.yamahata@intel.com>
> > > > > > >
> > > > > > > Add a new property "private" to memory backends. When it's set to true,
> > > > > > > it indicates the RAMblock of the backend also requires kvm gmem.
> > > > > > Can you add a brief explanation why you need the property?
> > > > >
> > > > > It provides a mechanism for user to specify whether the memory can serve as 
> > > > > private memory (need request kvm gmem).
> > > >
> > > > Yes, but why would a user want such memory?
> > > Because KVM demands it for confidential guest, e.g., TDX guest. KVM
> > > demands that the mem slot needs to have KVM_MEM_PRIVATE set and has
> > > valid gmem associated if the guest accesses it as private memory.
>
> Commit messages should explain why we want the patch.  Documenting "why"
> is at least as important as "what".  If "what" is missing, I can read
> the patch to find out.  If "why" is missing, I'm reduced to guesswork.

I'll try best to improve the commit message of this patch, and all other 
patches.

> > I think as long as there is no demand to have a TDX guest with this property be set to 
> > "off", then just don't add it.
> >
> > With a TDX VM, it will can be implicitly active. If we ever have to disable it for 
> > selective memory backends, we can add the property and have something like on/off/auto. 
> > For now it would be "auto".
>
> Makes sense to me.

OK. I think I get the answer of open #1 in cover letter.

If no other voice, I'll drop this patch and allocate gmem RAM when 
vm_type is TDX.