Re: [PATCH v7 05/12] KVM: guest_memfd: Add flag to remove from direct map

Brendan Jackman
2025-11-04T11:08:26Z

On Mon Nov 3, 2025 at 10:50 AM UTC, Mike Rapoport wrote:
> On Mon, Nov 03, 2025 at 10:35:38AM +0000, Brendan Jackman wrote:
>> On Sat Nov 1, 2025 at 9:39 AM UTC, Mike Rapoport wrote:
>> > On Fri, Oct 31, 2025 at 05:30:12PM +0000, Brendan Jackman wrote:
>> >> On Wed Sep 24, 2025 at 3:22 PM UTC, Patrick Roy wrote:
>> >> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> >> > index 1d0585616aa3..73a15cade54a 100644
>> >> > --- a/include/linux/kvm_host.h
>> >> > +++ b/include/linux/kvm_host.h
>> >> > @@ -731,6 +731,12 @@ static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
>> >> > bool kvm_arch_supports_gmem_mmap(struct kvm *kvm);
>> >> > #endif
>> >> >
>> >> > +#ifdef CONFIG_KVM_GUEST_MEMFD
>> >> > +#ifndef kvm_arch_gmem_supports_no_direct_map
>> >> > +#define kvm_arch_gmem_supports_no_direct_map can_set_direct_map
>> >> > +#endif
>> >> > +#endif /* CONFIG_KVM_GUEST_MEMFD */
>> >>
>> >> The test robot seems happy so I think I'm probably mistaken here, but
>> >> AFAICS can_set_direct_map only exists when ARCH_HAS_SET_DIRECT_MAP,
>> >> which powerpc doesn't set.
>> >
>> > We have stubs returning 0 for architectures that don't have
>> > ARCH_HAS_SET_DIRECT_MAP.
>>
>> I can't see any such stub for can_set_direct_map() specifically?
>
> include/linux/set_memory.h:
>
> #ifndef CONFIG_ARCH_HAS_SET_DIRECT_MAP
> static inline int set_direct_map_invalid_noflush(struct page *page)
> {
> return 0;
> }
> static inline int set_direct_map_default_noflush(struct page *page)
> {
> return 0;
> }
>
> static inline int set_direct_map_valid_noflush(struct page *page,
> unsigned nr, bool valid)
> {
> return 0;
> }
>
> static inline bool kernel_page_present(struct page *page)
> {
> return true;
> }
> #else /* CONFIG_ARCH_HAS_SET_DIRECT_MAP */
> /*
> * Some architectures, e.g. ARM64 can disable direct map modifications at
> * boot time. Let them overrive this query.
> */
> #ifndef can_set_direct_map
> static inline bool can_set_direct_map(void)
> {
> return true;
> }
> #define can_set_direct_map can_set_direct_map

But this is for CONFIG_ARCH_HAS_DIRECT_MAP? I am reading this as a stub
to fill in for archs that have set_direct_map_*, but don't have runtime
disablement like arm64.

Whereas my concern is archs that don't have set_direct_map_* at all,
i.e. where we need to unconditionally fail
GUEST_MEMFG_FLAG_NO_DIRECT_MAP.

(Or would we prefer to just not define it at all on those archs? Not
sure what the norms are there, I guess that's a question for KVM/arch
maintainers).