linux-kernel - Re: [PATCH v4 32/38] KVM: nVMX: Add nested virtualization support for mediated PMU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <71123914-ba9b-4ab0-b9c0-fa32fb53dfe2@linux.intel.com>
Date: Mon, 19 May 2025 13:24:43 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>,
 Mingwei Zhang <mizhang@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Paolo Bonzini <pbonzini@...hat.com>,
 Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 Adrian Hunter <adrian.hunter@...el.com>, Liang@...gle.com,
 Kan <kan.liang@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>,
 linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
 kvm@...r.kernel.org, linux-kselftest@...r.kernel.org,
 Yongwei Ma <yongwei.ma@...el.com>,
 Xiong Zhang <xiong.y.zhang@...ux.intel.com>,
 Jim Mattson <jmattson@...gle.com>, Sandipan Das <sandipan.das@....com>,
 Zide Chen <zide.chen@...el.com>, Eranian Stephane <eranian@...gle.com>,
 Shukla Manali <Manali.Shukla@....com>,
 Nikunj Dadhania <nikunj.dadhania@....com>
Subject: Re: [PATCH v4 32/38] KVM: nVMX: Add nested virtualization support for
 mediated PMU


On 5/16/2025 9:33 PM, Sean Christopherson wrote:
> This shortlog is unnecessarily confusing.  It reads as if supported for running
> L2 in a vCPU with a mediated PMU is somehow lacking.
>
> On Mon, Mar 24, 2025, Mingwei Zhang wrote:
>> Add nested virtualization support for mediated PMU by combining the MSR
>> interception bitmaps of vmcs01 and vmcs12.
> Do not phrase changelogs related to nested virtualization in terms of enabling a
> _KVM_ feature.  KVM has no control over what hypervisor runs in L1.  It's a-ok to
> provide example use cases, but they need to be just that, examples.
>
>> Readers may argue even without this patch, nested virtualization works for
>> mediated PMU because L1 will see Perfmon v2 and will have to use legacy vPMU
>> implementation if it is Linux. However, any assumption made on L1 may be
>> invalid, e.g., L1 may not even be Linux.
>>
>> If both L0 and L1 pass through PMU MSRs, the correct behavior is to allow
>> MSR access from L2 directly touch HW MSRs, since both L0 and L1 passthrough
>> the access.
>>
>> However, in current implementation, if without adding anything for nested,
>> KVM always set MSR interception bits in vmcs02. This leads to the fact that
>> L0 will emulate all MSR read/writes for L2, leading to errors, since the
>> current mediated vPMU never implements set_msr() and get_msr() for any
>> counter access except counter accesses from the VMM side.
>>
>> So fix the issue by setting up the correct MSR interception for PMU MSRs.
> This is not a fix.  
>
>     KVM: nVMX: Disable PMU MSR interception as appropriate while running L2
>     
>     Merge KVM's PMU MSR interception bitmaps with those of L1, i.e. merge the
>     bitmaps of vmcs01 and vmcs12, e.g. so that KVM doesn't interpose on MSR
>     accesses unnecessarily if L1 exposes a mediated PMU (or equivalent) to L2.

Sure. Thanks.


>
>> Signed-off-by: Mingwei Zhang <mizhang@...gle.com>
>> Co-developed-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
>> ---
>>  arch/x86/kvm/vmx/nested.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>> index cf557acf91f8..dbec40cb55bc 100644
>> --- a/arch/x86/kvm/vmx/nested.c
>> +++ b/arch/x86/kvm/vmx/nested.c
>> @@ -626,6 +626,36 @@ static inline void nested_vmx_set_intercept_for_msr(struct vcpu_vmx *vmx,
>>  #define nested_vmx_merge_msr_bitmaps_rw(msr) \
>>  	nested_vmx_merge_msr_bitmaps(msr, MSR_TYPE_RW)
>>  
>> +/*
>> + * Disable PMU MSRs interception for nested VM if L0 and L1 are
>> + * both mediated vPMU.
>> + */
> Again, KVM has no idea what is running in L1.  Drop this.

Yes. thanks.


>
>> +static void nested_vmx_merge_pmu_msr_bitmaps(struct kvm_vcpu *vcpu,
>> +					     unsigned long *msr_bitmap_l1,
>> +					     unsigned long *msr_bitmap_l0)
>> +{
>> +	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>> +	struct vcpu_vmx *vmx = to_vmx(vcpu);
>> +	int i;
>> +
>> +	if (!kvm_mediated_pmu_enabled(vcpu))
> This is a worthwhile check, but a comment would be helpful:
>
> 	/*
> 	 * Skip the merges if the vCPU doesn't have a mediated PMU MSR, i.e. if
> 	 * none of the MSRs can possibly be passed through to L1.
> 	 */
> 	if (!kvm_vcpu_has_mediated_pmu(vcpu))
> 		return;

Sure. thanks.


>
>> +		return;
>> +
>> +	for (i = 0; i < pmu->nr_arch_gp_counters; i++) {
>> +		nested_vmx_merge_msr_bitmaps_rw(MSR_ARCH_PERFMON_EVENTSEL0 + i);
> This is unnecessary, KVM always intercepts event selectors.

Yes.


>
>> +		nested_vmx_merge_msr_bitmaps_rw(MSR_IA32_PERFCTR0 + i);
>> +		nested_vmx_merge_msr_bitmaps_rw(MSR_IA32_PMC0 + i);
>> +	}
>> +
>> +	for (i = 0; i < pmu->nr_arch_fixed_counters; i++)
>> +		nested_vmx_merge_msr_bitmaps_rw(MSR_CORE_PERF_FIXED_CTR0 + i);
>> +
>> +	nested_vmx_merge_msr_bitmaps_rw(MSR_CORE_PERF_FIXED_CTR_CTRL);
> Same thing here.

Yes.


>
>> +	nested_vmx_merge_msr_bitmaps_rw(MSR_CORE_PERF_GLOBAL_CTRL);
>> +	nested_vmx_merge_msr_bitmaps_read(MSR_CORE_PERF_GLOBAL_STATUS);
>> +	nested_vmx_merge_msr_bitmaps_write(MSR_CORE_PERF_GLOBAL_OVF_CTRL);
>> +}