On Wed, May 06, 2026, Vishal Annapurve wrote:
> On Wed, May 6, 2026 at 4:56 PM Huang, Kai <kai.huang@intel.com> wrote:
> >
> > On Wed, 2026-05-06 at 16:03 -0700, Sean Christopherson wrote:
> > > The question then becomes, do we keep patch 1 and also clear enable_pmu in tdx.c,
> > > or do we keep the ordering and have kvm_arch_init_vm() consume has_protected_pmu?
> > > Neither one is particularly awesome :-/
> >
> > Maybe keeping patch 1 is slightly better?  This allows the vm_init() to toggle
> > all the values and flags that the default ones don't fit.  If we go with latter,
> > when there's similar cases where we need more flags, then kvm_arch_init_vm()
> > needs to consume more flags.
> 
> For now I have sent a v2 without moving around the default flags. It's
> definitely better to move them up as done in patch1,

Yes and no.  No matter what we do, we're subtly relying on ordering between
initialization in kvm_arch_init_vm() and kvm_x86_ops.vm_init().  I don't see an
easy way around that.

For these fields:

	kvm->arch.default_tsc_khz = max_tsc_khz ? : tsc_khz;
	kvm->arch.apic_bus_cycle_ns = APIC_BUS_CYCLE_NS_DEFAULT;
	kvm->arch.guest_can_read_msr_platform_info = true;

and any other fields that KVM initializes with hardcoded values, from globals, or
from local variables (including @type), I 100% agree that they should be initialized
before calling kvm_x86_ops.vm_init().

But with the proposed has_protected_pmu, I'm leaning towards keeping this:

	kvm->arch.enable_pmu = enable_pmu && !kvm->arch.has_protected_pmu;

which obviously means keeping it after kvm_x86_ops.vm_init().  Because if we make
vendor code responsible for clearing kvm->arch.enable_pmu, then (a) we may end
up with duplicate code for SEV-ES+ (does SEV-ES/SNP allow for a virtual/emulated
PMU?), and (b) the x86 code _looks_ wrong, because kvm_arch_init_vm() will ignore
kvm->arch.has_protected_pmu, while the KVM_CAP_PMU_CAPABILITY case-statement does
not.