Received-SPF: pass (google.com: domain of linux-kernel+bounces-158437-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: "Wang, Wei W" <wei.w.wang@intel.com>
To: Sean Christopherson <seanjc@google.com>, Paolo Bonzini
	<pbonzini@redhat.com>
CC: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 0/4] KVM: x86: Collect host state snapshots into a struct
Thread-Topic: [PATCH 0/4] KVM: x86: Collect host state snapshots into a struct
Thread-Index: AQHalcvOvJHKHdG+PEK5/c2S4LZHsrF4V4XA
Date: Thu, 25 Apr 2024 11:17:10 +0000
Message-ID: <DS0PR11MB6373E404A16BD8CC55128FDFDC172@DS0PR11MB6373.namprd11.prod.outlook.com>
References: <20240423221521.2923759-1-seanjc@google.com>
In-Reply-To: <20240423221521.2923759-1-seanjc@google.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?sogQWL7SU7CMNhWsOYPnXibIlaM5+I3THdJUNQSKqMsYHoByCWDnjU1myCN6?=
 =?us-ascii?Q?v6617cIOubsO+xBBKVs3/FfxeiFTkEXL7BSlpGZI4cW3QdxWeRnX1cZfNumf?=
 =?us-ascii?Q?a/jkLkBigIjbEyRdX3zNPsTNmSvMR5D3IEmqsuZJZ7GULbKXIV43/x0uI4TN?=
 =?us-ascii?Q?Y5l2y4utMt/p925W5X5mgiBzo0m9FTpEPHO35LllEX3oRM4r0qeaU/89e7Jv?=
 =?us-ascii?Q?lDjr0LzRp5HfEnKd5WPjon8xTuRlLcB+nnA8eTHxXUL3RqayBIP7ARJuP7/5?=
 =?us-ascii?Q?4jND848iy5CIoDmoN75Sa6UQjz/x17Cw1cQ1siwV3DWE8t5eSrYxpprUfJaz?=
 =?us-ascii?Q?gEFb5TmUCHx02SCnKZ+JUyi8yP9u0w52bXS/uG/1ca0452WYRCge3i+0K98v?=
 =?us-ascii?Q?3h9GiGcT2qm0qich0GIeUtZera47cp4d1udT0A2ALDPtPYdC7oaXXeMoHheB?=
 =?us-ascii?Q?Ds9WEWJ7AanALe/YG9+v0Gi3VOZQ6GCsut/Iyjotkmt3Xu95dM2OZ1UHYBoH?=
 =?us-ascii?Q?5ddN/QEmlhvzyTao3Dug3LQ6QjEkX/2gNPdJnRDSJucL8NYi/B4QFwwcODch?=
 =?us-ascii?Q?BtCPm6DBJMQAWOdadU/SAKsXBq6dRHRtyVfoJ44uQzBeJuV/bPxxAUipvkNu?=
 =?us-ascii?Q?g4eVZErn3PoIFE4myV+79hIPYS2HgpyTmigcOcyBuHC3TFWywJgREJzbi5IT?=
 =?us-ascii?Q?3ux4jUjCXIKHzUpU4xygGO7OY9+NIbKf6d42a8ZTxH9pPFirnSgc1+N6V5c8?=
 =?us-ascii?Q?1hBnUhJVOHBGHq13ZnLroLL/mJ7bUu9jm7qqiefd845nq1bLYmQcxCr5QULg?=
 =?us-ascii?Q?OzxRlGR0rVEpDsCY4qIh7eofoTmi9OynotQtSlVG/G31CcraaeKOS2HfEvYj?=
 =?us-ascii?Q?K5mj317ji3BK9HmOrqpmPQ5QknFD+0UyL+/BWAoY6AsvhqOlCnmN49leSuk1?=
 =?us-ascii?Q?nt7vRQAClPEgUvgRJQ71znd3dLjTanm3DcHh2wymovK+jD7PdbWJ/cpBpLgo?=
 =?us-ascii?Q?Mhr+eMaVCryU2b/1p2xiDqw6OmVxCBY41wd9l+5XWfh9G9LtHK3oymGdc3oL?=
 =?us-ascii?Q?zwUo+i3TP7fu6J65xZgztjKlzJuQe9f//47dniKdV0rS/cL1bgNKLtNmB6Zs?=
 =?us-ascii?Q?aOQiaU7AGQOuW2zPedfRdpkkhXhUoIXk6wQNHuZvC57CcdrHa/5q8hNYAXYt?=
 =?us-ascii?Q?tu06jY0YlTh8Hp02qZ2pnhaCNQz+rc37OKRSsMGXal8nmvBE/ruq/hM6ZaSS?=
 =?us-ascii?Q?99bYGE/rqcPm8i+GzmvwBI06GpLCickzz1ltm3hAmR/KzTVv0jjO6/mNb9x9?=
 =?us-ascii?Q?r7JwDmpaJVhz+LzA1JxmIXRvAhf0nPRuO5+Ms0EYdvLLviZzVmE/Bh8y/wey?=
 =?us-ascii?Q?kPyls1YBlBEBhcG4ptobXv99JNpfxiYyCK8zgFqHslWl89hodaqt2BpadPpS?=
 =?us-ascii?Q?HItr1CORqHLwKxF2datTCCqS8gOJywU35yKBNywhW8IyQLXycVaYJjly0UVO?=
 =?us-ascii?Q?u48IiCPgSZTKovDsgbx6hNo+5z0iwlLpOvP2C+4U4S/aINDIs3BHaZfcyOo4?=
 =?us-ascii?Q?2MzeeVYNkjRsM1Jx9pG85/314GgD/NVoBVW/dUWj?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB6373.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 198f5ed8-215b-47fa-cc69-08dc6519404e
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Apr 2024 11:17:10.6978
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: nnghFogDEuG1MCQgpWs7EjMz+PGy2UVF8SBV+cxf3KA3jLGKNgGhCRxRMai4vC/POxUU6onq5lcTMsfvdmSsxA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8496
X-OriginatorOrg: intel.com

On Wednesday, April 24, 2024 6:15 AM, Sean Christopherson wrote:
> Add a global "kvm_host" structure to hold various host values, e.g. for E=
FER,
> XCR0, raw MAXPHYADDR etc., instead of having a bunch of one-off variables
> that inevitably need to be exported, or in the case of shadow_phys_bits, =
are
> buried in a random location and are awkward to use, leading to duplicate
> code.

Looks good. How about applying similar improvements to the module
parameters as well? I've changed the "enable_pmu" parameter as an example b=
elow:

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 77352a4abd87..a221ba7b546f 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1013,7 +1013,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_ar=
ray *array, u32 function)
                union cpuid10_eax eax;
                union cpuid10_edx edx;

-               if (!enable_pmu || !static_cpu_has(X86_FEATURE_ARCH_PERFMON=
)) {
+               if (!kvm_caps.enable_pmu || !static_cpu_has(X86_FEATURE_ARC=
H_PERFMON)) {
                        entry->eax =3D entry->ebx =3D entry->ecx =3D entry-=
>edx =3D 0;
                        break;
                }
@@ -1306,7 +1306,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_ar=
ray *array, u32 function)
                union cpuid_0x80000022_ebx ebx;

                entry->ecx =3D entry->edx =3D 0;
-               if (!enable_pmu || !kvm_cpu_cap_has(X86_FEATURE_PERFMON_V2)=
) {
+               if (!kvm_caps.enable_pmu || !kvm_cpu_cap_has(X86_FEATURE_PE=
RFMON_V2)) {
                        entry->eax =3D entry->ebx;
                        break;
                }
diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
index 4d52b0b539ba..7e359db64dbd 100644
--- a/arch/x86/kvm/pmu.h
+++ b/arch/x86/kvm/pmu.h
@@ -190,9 +190,9 @@ static inline void kvm_init_pmu_capability(const struct=
 kvm_pmu_ops *pmu_ops)
         * for hybrid PMUs until KVM gains a way to let userspace opt-in.
         */
        if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU))
-               enable_pmu =3D false;
+               kvm_caps.enable_pmu =3D false;

-       if (enable_pmu) {
+       if (kvm_caps.enable_pmu) {
                perf_get_x86_pmu_capability(&kvm_pmu_cap);

                /*
@@ -203,12 +203,12 @@ static inline void kvm_init_pmu_capability(const stru=
ct kvm_pmu_ops *pmu_ops)
                 */
                if (!kvm_pmu_cap.num_counters_gp ||
                    WARN_ON_ONCE(kvm_pmu_cap.num_counters_gp < min_nr_gp_ct=
rs))
-                       enable_pmu =3D false;
+                       kvm_caps.enable_pmu =3D false;
                else if (is_intel && !kvm_pmu_cap.version)
-                       enable_pmu =3D false;
+                       kvm_caps.enable_pmu =3D false;
        }

-       if (!enable_pmu) {
+       if (!kvm_caps.enable_pmu) {
                memset(&kvm_pmu_cap, 0, sizeof(kvm_pmu_cap));
                return;
        }
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 7ea0e7f13da4..4ed8c73f88e4 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7869,7 +7869,7 @@ static __init u64 vmx_get_perf_capabilities(void)
        u64 perf_cap =3D PMU_CAP_FW_WRITES;
        u64 host_perf_cap =3D 0;

-       if (!enable_pmu)
+       if (!kvm_caps.enable_pmu)
                return 0;

        if (boot_cpu_has(X86_FEATURE_PDCM))
@@ -7938,7 +7938,7 @@ static __init void vmx_set_cpu_caps(void)
                kvm_cpu_cap_check_and_set(X86_FEATURE_DTES64);
        }

-       if (!enable_pmu)
+       if (!kvm_caps.enable_pmu)
                kvm_cpu_cap_clear(X86_FEATURE_PDCM);
        kvm_caps.supported_perf_cap =3D vmx_get_perf_capabilities();

@@ -8683,7 +8683,7 @@ static __init int hardware_setup(void)

        if (pt_mode !=3D PT_MODE_SYSTEM && pt_mode !=3D PT_MODE_HOST_GUEST)
                return -EINVAL;
-       if (!enable_ept || !enable_pmu || !cpu_has_vmx_intel_pt())
+       if (!enable_ept || !kvm_caps.enable_pmu || !cpu_has_vmx_intel_pt())
                pt_mode =3D PT_MODE_SYSTEM;
        if (pt_mode =3D=3D PT_MODE_HOST_GUEST)
                vmx_init_ops.handle_intel_pt_intr =3D vmx_handle_intel_pt_i=
ntr;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cdcda1bbf5a3..36d471a7af87 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -94,6 +94,7 @@

 struct kvm_caps kvm_caps __read_mostly =3D {
        .supported_mce_cap =3D MCG_CTL_P | MCG_SER_P,
+       .enable_pmu =3D true,
 };
 EXPORT_SYMBOL_GPL(kvm_caps);

@@ -192,9 +193,7 @@ int __read_mostly pi_inject_timer =3D -1;
 module_param(pi_inject_timer, bint, 0644);

 /* Enable/disable PMU virtualization */
-bool __read_mostly enable_pmu =3D true;
-EXPORT_SYMBOL_GPL(enable_pmu);
-module_param(enable_pmu, bool, 0444);
+module_param_named(enable_pmu, kvm_caps.enable_pmu, bool, 0444);

 bool __read_mostly eager_page_split =3D true;
 module_param(eager_page_split, bool, 0644);
@@ -4815,7 +4814,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, lon=
g ext)
                break;
        }
        case KVM_CAP_PMU_CAPABILITY:
-               r =3D enable_pmu ? KVM_CAP_PMU_VALID_MASK : 0;
+               r =3D kvm_caps.enable_pmu ? KVM_CAP_PMU_VALID_MASK : 0;
                break;
        case KVM_CAP_DISABLE_QUIRKS2:
                r =3D KVM_X86_VALID_QUIRKS;
@@ -6652,7 +6651,7 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
                break;
        case KVM_CAP_PMU_CAPABILITY:
                r =3D -EINVAL;
-               if (!enable_pmu || (cap->args[0] & ~KVM_CAP_PMU_VALID_MASK)=
)
+               if (!kvm_caps.enable_pmu || (cap->args[0] & ~KVM_CAP_PMU_VA=
LID_MASK))
                        break;

                mutex_lock(&kvm->lock);
@@ -7438,7 +7437,7 @@ static void kvm_init_msr_lists(void)
        for (i =3D 0; i < ARRAY_SIZE(msrs_to_save_base); i++)
                kvm_probe_msr_to_save(msrs_to_save_base[i]);

-       if (enable_pmu) {
+       if (kvm_caps.enable_pmu) {
                for (i =3D 0; i < ARRAY_SIZE(msrs_to_save_pmu); i++)
                        kvm_probe_msr_to_save(msrs_to_save_pmu[i]);
        }
@@ -12555,7 +12554,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long=
 type)

        kvm->arch.default_tsc_khz =3D max_tsc_khz ? : tsc_khz;
        kvm->arch.guest_can_read_msr_platform_info =3D true;
-       kvm->arch.enable_pmu =3D enable_pmu;
+       kvm->arch.enable_pmu =3D kvm_caps.enable_pmu;

 #if IS_ENABLED(CONFIG_HYPERV)
        spin_lock_init(&kvm->arch.hv_root_tdp_lock);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 102754dc85bc..c4d99338aaa1 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -29,6 +29,9 @@ struct kvm_caps {
        u64 supported_xcr0;
        u64 supported_xss;
        u64 supported_perf_cap;
+
+       /* KVM module parameters */
+       bool enable_pmu;
 };

 struct kvm_host_values {
@@ -340,8 +343,6 @@ fastpath_t handle_fastpath_set_msr_irqoff(struct kvm_vc=
pu *vcpu);
 extern struct kvm_caps kvm_caps;
 extern struct kvm_host_values kvm_host;

-extern bool enable_pmu;