Suzuki K Poulose <[email protected]> writes: > > [...snip...] > >>>> @@ -2546,7 +2546,7 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm >>>> *kvm, >>>> bool kvm_arch_post_set_memory_attributes(struct kvm *kvm, >>>> struct kvm_gfn_range *range); >>>> >>>> -static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn) >>>> +static inline bool kvm_vm_mem_is_private(struct kvm *kvm, gfn_t gfn) >> >> Should have read the Sashiko review first, but where is this used? >> It's not used at all in this series... > > See below: > >> >> /fuad >> >>>> { >>>> return kvm_get_vm_memory_attributes(kvm, gfn) & >>>> KVM_MEMORY_ATTRIBUTE_PRIVATE; >>>> } >>>> @@ -2557,6 +2557,16 @@ static inline bool kvm_mem_range_is_private(struct >>>> kvm *kvm, gfn_t start, >>>> >>>> KVM_MEMORY_ATTRIBUTE_PRIVATE, >>>> >>>> KVM_MEMORY_ATTRIBUTE_PRIVATE); >>>> } >>>> +#endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */ >>>> + >>>> +#ifdef kvm_arch_has_private_mem >>>> +typedef bool (kvm_mem_is_private_t)(struct kvm *kvm, gfn_t gfn); >>>> +DECLARE_STATIC_CALL(__kvm_mem_is_private, kvm_mem_is_private_t); >>>> + >>>> +static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn) >>>> +{ >>>> + return static_call(__kvm_mem_is_private)(kvm, gfn); >>>> +} >>>> #else >>>> static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn) >>>> { >>>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >>>> index 6669f1477013c..8b238e461b854 100644 >>>> --- a/virt/kvm/kvm_main.c >>>> +++ b/virt/kvm/kvm_main.c >>>> @@ -2627,6 +2627,20 @@ static int kvm_vm_ioctl_set_mem_attributes(struct >>>> kvm *kvm, >>>> } >>>> #endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */ >>>> >>>> +#ifdef kvm_arch_has_private_mem >>>> +DEFINE_STATIC_CALL_RET0(__kvm_mem_is_private, kvm_mem_is_private_t); >>>> +EXPORT_STATIC_CALL_GPL(__kvm_mem_is_private); >>>> + >>>> +static void kvm_init_memory_attributes(void) >>>> +{ >>>> +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES >>>> + static_call_update(__kvm_mem_is_private, kvm_vm_mem_is_private); >>>> +#endif >>>> +} > > > Here ^^ as the static call update ? > > > Suzuki
Thanks Suzuki, it is used here. kvm_mem_is_private() was and still is the function used to check if some gfn is private or shared. Hence, in this patch, the usages of kvm_mem_is_private() were not updated. Instead, kvm_mem_is_private() is now set up as a static call, and the static call is hard-wired to kvm_vm_mem_is_private() in this patch. In the later wiring patch, all the places where attributes are looked up are updated all at once: if conversion enabled, take gmem route, else take VM route. kvm_mem_is_private() is special in that the if-else is done at KVM load time rather than runtime, and I believe that's for performance reasons since this is checked quite often from the KVM fault handling code. Buut I think perhaps Fuad was referring to kvm_mem_range_is_private(), which is indeed not used anywhere. Binbin also asked about this, I think we should drop kvm_mem_range_is_private(). My reply to Binbin is at [1]. [1] https://lore.kernel.org/all/CAEvNRgGbBcrX5Fw3vNTsTOBNC=Ypi=9-s07674ypxlu9i4a...@mail.gmail.com/
