Re: [PATCH v7 02/23] KVM: Make device name configurable

[Paolo Bonzini <pbonzini@redhat.com>]

On 9/3/26 16:45, Sean Christopherson wrote:
> On Thu, Sep 03, 2026, Sean Christopherson wrote:
>> On Thu, Sep 03, 2026, Sean Christopherson wrote:
>>> On Thu, Sep 03, 2026, Steffen Eiden wrote:
>>>> On Wed, Sep 02, 2026 at 09:14:21AM -0700, Sean Christopherson wrote:
>>>>> And taking things a few steps further, we can solve the MMIO issue in a more
>>>>> elegant way, and eliminate the runtime string building in this patch (after looking
>>>>> more closely, that code needs to be jettisoned no matter what, there's simply no
>>>>> reason to specify the names at runtime since they're separate compilation units).
>>>>>
>>>>> Rather than splatter #defines throughout header files, deal with the bulk of the
>>>>> pain in Makefile.kvm.  By feeding conditionals into Makefile.kvm, the s390+arm64
>>>>> build can easily omit coalesced_mmio.o and async_pf.o, define __KVM_HAVE_ARCH_MMIO
>>>>> programatically without having to change other architectures, and solve the naming
>>>>> stuff.
>>>>
>>>> Yes, this is a great idea. Thank you. I second you, this looks more clean
>>>> and stable than the stuff we came up with :)
>>>
>>> Looking more at arch/s390/kvm/Kconfig and virt/kvm/Kconfig, we might need/want to
>>> to build out infrastructure to handle this sort of thing in a more generic fashion?
>>>
>>> Which probably isn't that much infrastructure?  It's more just changing how arch
>>> code communicates with common KVM?  I.e. instead of providing boolean configs in
>>> virt/kvm/Kconfig, formalize communicating HAVE-type macros through the Makefile.
>>> That might even be a net positive in the long run, as it will make it easier to
>>> provide defaults for the common cases.
>>>
>>> I say that because unless there's magic I'm unaware of these Kconfigs also needs
>>> to be configured per-KVM, not per-kernel:
>>>
>>>    - KVM_MMU_LOCKLESS_AGING, otherwise aging on arm64 will unintentionally be done
>>>      outside of mmu_lock.
>>>
>>>    - KVM_GENERIC_PRE_FAULT_MEMORY, so that arm64 doesn't need to provide a stub
>>>      for something it doesn't support
>>>
>>>    - HAVE_KVM_MSI, because presumably it's needed for arm64 support.
>>>
>>>    - HAVE_KVM_READONLY_MEM, same story as HAVE_KVM_MSI.
>>
>> And in the opposite direction, HAVE_KVM_VCPU_RUN_PID_CHANGE also falls into this
>> category.  That one is probably better handled as a #define in header files?
> 
> Continuing the conversation with myself, add in HAVE_KVM_NO_POLL and
> HAVE_KVM_INVALID_WAKEUPS (which reminds me, valid_wakeup should really be moved
> into s390's kvm_vcpu_arch).

Yes, these three can be handled in a similar way as this in 
arch/x86/include/asm/atomic64_64.h:

#define arch_atomic64_xchg arch_atomic64_xchg

and then a #ifndef replaces the "#ifdef CONFIG_*".  Do this, and moving 
vcpu->valid_wakeup to vcpu->arch comes almost for free.

>>> Actually, thinking about this more, what I've proposed here, plus the pattern of
>>> #define-ing macros in arch-specific kvm_host.h files, should suffice.  For things
>>> like __KVM_HAVE_ARCH_VM_FREE, it absolutely makes sense to #define the macro in
>>> kvm_host.h since it's very directly tied to an arch callback.  Whereas with KVM_MIO
>>> and KVM_ASYNC_PF, because they enable compilation of C files, it makes sense to
>>> define them in Makefiles.
Ugh, defining CONFIG symbols in Makefiles sucks.  I don't want KVM to be 
the one that does things differently *once more*.  I'd prefer to stub 
out the contents of the .c files instead.

Paolo