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

[Steffen Eiden <seiden@linux.ibm.com>]

On Wed, Sep 02, 2026 at 09:14:21AM -0700, Sean Christopherson wrote:
> On Wed, Sep 02, 2026, Steffen Eiden wrote:
> > On Tue, Sep 01, 2026 at 05:40:25PM -0700, Sean Christopherson wrote:
> > > On Mon, Aug 31, 2026, Steffen Eiden wrote:
> > > > Allow KVM implementations to choose alternative device names. This is
> > > > especially useful for architectures providing multiple KVM
> > > > implementations simultaneously. Architectures providing multiple KVM
> > > > implementations must compile the KVM common code once per
> > > > implementation and mange symbols.
> > > 
> > > What about tracepoints?  Or do those show up as "kvm" and "kvm-arm64"?
> > 
> > Yes, I want them to show up as kvm and  kvm-arm64.
> > 
> > Thanks for pointing that out - I just noticed that I forgot to switch
> > the trace system to kvm-arm64 for the common tracepoints in
> > trace/events/kvm.h
> > I only did it for the arch-local traces in PATCH 21.
> > 
> > I would just do the following:
> > 
> > diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
> > index b282e3a86769..5d4f8a0693a3 100644
> > --- a/include/trace/events/kvm.h
> > +++ b/include/trace/events/kvm.h
> > @@ -5,7 +5,11 @@
> >  #include <linux/tracepoint.h>
> > 
> >  #undef TRACE_SYSTEM
> > +#ifdef KVM_S390_ARM64
> 
> Side topic, I recommend choosing a macro name that doesn't have a near-collision
> with CONFIG_KVM_S390_ARM64.  This *looks* like a typo, i.e. it looks like you
> forgot the CONFIG_ prefix.  Especially since the macro is defined in the Makefile
> and won't show up with e.g. "git grep -w KVM_S390_ARM64". E.g. KVM_S390_BUILD_ARM64
> or something?

Interesting. The name was deliberately chosen to be similar. But I
see that it could be confusing.

I am not totally happy with KVM_S390_BUILD_ARM64 but I cannot find a
better name either.

> 
> Side topic #2, this entire approach seems extremely brittle unless you make it
> all but impossible for non-KVM code to get at KVM structure definitions.  Outside
> of KVM, all compilation units will see the s390 version of KVM structures.  Which
> is "fine", but obviously dangerous and IMO asking for maintenance issues down the
> road.
> 
> > +#define TRACE_SYSTEM kvm-arm64
> > +#else
> >  #define TRACE_SYSTEM kvm
> > +#endif /* KVM_S390_ARM64 */
> > 
> >  #define ERSN(x) { KVM_EXIT_##x, "KVM_EXIT_" #x }
> > 
> > 
> > This would leak a bit of arm on s390 into common KVM but I do not see
> > another way.
> 
> Morpheus: Stop trying to use macros, and use macros!
> 
> The most annoying thing is that macro shenanigans don't play well with hyphens,
> but that can be handled either by using a different macro for the trace name, or
> by creating /dev/kvm_arm64 instead of /dev/kvm-arm64.  My vote would be to have
> the device be /dev/kvm_arm64, assuming that doesn't cause problems elsewhere.

I am not aware of any problems that could cause. It was just a personal
preference IIRC. 

> 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 :) 

I'll integrate it into the series and send it with the next round.

Thank you for your input. Very much appreciated.

	Steffen

...