Received-SPF: pass (google.com: domain of linux-kernel+bounces-141603-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 11 Apr 2024 14:03:00 -0700
In-Reply-To: <20240126085444.324918-12-xiong.y.zhang@linux.intel.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240126085444.324918-1-xiong.y.zhang@linux.intel.com> <20240126085444.324918-12-xiong.y.zhang@linux.intel.com>
Message-ID: <ZhhQBHQ6V7Zcb8Ve@google.com>
Subject: Re: [RFC PATCH 11/41] KVM: x86/pmu: Introduce enable_passthrough_pmu
 module parameter and propage to KVM instance
From: Sean Christopherson <seanjc@google.com>
To: Xiong Zhang <xiong.y.zhang@linux.intel.com>
Cc: pbonzini@redhat.com, peterz@infradead.org, mizhang@google.com, 
	kan.liang@intel.com, zhenyuw@linux.intel.com, dapeng1.mi@linux.intel.com, 
	jmattson@google.com, kvm@vger.kernel.org, linux-perf-users@vger.kernel.org, 
	linux-kernel@vger.kernel.org, zhiyuan.lv@intel.com, eranian@google.com, 
	irogers@google.com, samantha.alt@intel.com, like.xu.linux@gmail.com, 
	chao.gao@intel.com, Xiong Zhang <xiong.y.zhang@intel.com>
Content-Type: text/plain; charset="us-ascii"

On Fri, Jan 26, 2024, Xiong Zhang wrote:
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 4432e736129f..074452aa700d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -193,6 +193,11 @@ bool __read_mostly enable_pmu = true;
>  EXPORT_SYMBOL_GPL(enable_pmu);
>  module_param(enable_pmu, bool, 0444);
>  
> +/* Enable/disable PMU virtualization */

Heh, copy+paste fail.  Just omit a comment, it's pretty self-explanatory.

> +bool __read_mostly enable_passthrough_pmu = true;
> +EXPORT_SYMBOL_GPL(enable_passthrough_pmu);
> +module_param(enable_passthrough_pmu, bool, 0444);

Almost forgot.  Two things:

 1. KVM should not enable the passthrough/mediate PMU by default until it has
    reached feature parity with the existing PMU, because otherwise we are
    essentially breaking userspace.  And if for some reason the passthrough PMU
    *can't* reach feature parity, then (a) that's super interesting, and (b) we
    need a more explicit/deliberate transition plan.

 2. The module param absolutely must not be exposed to userspace until all patches
    are in place.  The easiest way to do that without creating dependency hell is
    to simply not create the module param.

I.e. this patch should do _only_

bool __read_mostly enable_passthrough_pmu;
EXPORT_SYMBOL_GPL(enable_passthrough_pmu);