Re: [PATCH 2/2] KVM: TDX: Disable pmu virtualization for TDX VMs

[Vishal Annapurve]

On Thu, May 7, 2026 at 7:25 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Wed, May 06, 2026, Vishal Annapurve wrote:
> > On Wed, May 6, 2026 at 4:56 PM Huang, Kai <kai.huang@xxxxxxxxx> 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

Michael,

I believe has_protected_pmu should be set for SNP VMs as well based on
my read of AMD SNP spec which suggests PMC virtualization managed by
hardware for SNP VMs and not by KVM.

> 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.