On 16.07.25 13:05, Fuad Tabba wrote:
On Wed, 16 Jul 2025 at 12:02, Xiaoyao Li <xiaoyao.li@xxxxxxxxx> wrote:
On 7/16/2025 6:25 PM, David Hildenbrand wrote:
On 16.07.25 10:31, Xiaoyao Li wrote:
On 7/16/2025 4:11 PM, Fuad Tabba wrote:
On Wed, 16 Jul 2025 at 05:09, Xiaoyao Li<xiaoyao.li@xxxxxxxxx> wrote:
On 7/15/2025 5:33 PM, Fuad Tabba wrote:
The original name was vague regarding its functionality. This Kconfig
option specifically enables and gates the kvm_gmem_populate()
function,
which is responsible for populating a GPA range with guest data.
Well, I disagree.
The config KVM_GENERIC_PRIVATE_MEM was introduced by commit
89ea60c2c7b5
("KVM: x86: Add support for "protected VMs" that can utilize private
memory"), which is a convenient config for vm types that requires
private memory support, e.g., SNP, TDX, and KVM_X86_SW_PROTECTED_VM.
It was commit e4ee54479273 ("KVM: guest_memfd: let kvm_gmem_populate()
operate only on private gfns") that started to use
CONFIG_KVM_GENERIC_PRIVATE_MEM gates kvm_gmem_populate() function. But
CONFIG_KVM_GENERIC_PRIVATE_MEM is not for kvm_gmem_populate() only.
If using CONFIG_KVM_GENERIC_PRIVATE_MEM to gate kvm_gmem_populate() is
vague and confusing, we can introduce KVM_GENERIC_GMEM_POPULATE to gate
kvm_gmem_populate() and select KVM_GENERIC_GMEM_POPULATE under
CONFIG_KVM_GENERIC_PRIVATE_MEM.
Directly replace CONFIG_KVM_GENERIC_PRIVATE_MEM with
KVM_GENERIC_GMEM_POPULATE doesn't look correct to me.
I'll quote David's reply to an earlier version of this patch [*]:
It's not related to my concern.
My point is that CONFIG_KVM_GENERIC_PRIVATE_MEM is used for selecting
the private memory support. Rename it to KVM_GENERIC_GMEM_POPULATE is
not correct.
It protects a function that is called kvm_gmem_populate().
Can we stop the nitpicking?
I don't think it's nitpicking.
Could you loot into why it was named as KVM_GENERIC_PRIVATE_MEM in the
first place, and why it was picked to protect kvm_gmem_populate()?
That is, in part, the point of this patch. This flag protects
kvm_gmem_populate(), and the name didn't reflect that. Now it does. It
is the only thing it protects.
I'll note that the kconfig makes it clear that it depends on
KVM_GENERIC_MEMORY_ATTRIBUTES -- having support for private memory.
In any case, CONFIG_KVM_GENERIC_PRIVATE_MEM is a bad name: what on earth
is generic private memory.
If CONFIG_KVM_GENERIC_GMEM_POPULATE is for some reason I don't
understand yet not the right name, can we have something that better
expresses that is is about KVM .. GMEM ... and POPULATE?
--
Cheers,
David / dhildenb
