| Message ID | 20190325191017.145263-1-liran.alon@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Raise #GP when guest vCPU do not support PMU | expand |
Ping. > On 25 Mar 2019, at 21:10, Liran Alon <liran.alon@oracle.com> wrote: > > Before this change, reading a VMware pseduo PMC will succeed even when > PMU is not supported by guest. This can easily be seen by running > kvm-unit-test vmware_backdoors with "-cpu host,-pmu" option. > > Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com> > Signed-off-by: Liran Alon <liran.alon@oracle.com> > --- > arch/x86/kvm/pmu.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c > index 58ead7db71a3..e39741997893 100644 > --- a/arch/x86/kvm/pmu.c > +++ b/arch/x86/kvm/pmu.c > @@ -281,9 +281,13 @@ static int kvm_pmu_rdpmc_vmware(struct kvm_vcpu *vcpu, unsigned idx, u64 *data) > int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data) > { > bool fast_mode = idx & (1u << 31); > + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); > struct kvm_pmc *pmc; > u64 ctr_val; > > + if (!pmu->version) > + return 1; > + > if (is_vmware_backdoor_pmc(idx)) > return kvm_pmu_rdpmc_vmware(vcpu, idx, data); > > -- > 2.20.1 >
On 25/03/19 20:10, Liran Alon wrote: > Before this change, reading a VMware pseduo PMC will succeed even when > PMU is not supported by guest. This can easily be seen by running > kvm-unit-test vmware_backdoors with "-cpu host,-pmu" option. > > Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com> > Signed-off-by: Liran Alon <liran.alon@oracle.com> > --- > arch/x86/kvm/pmu.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c > index 58ead7db71a3..e39741997893 100644 > --- a/arch/x86/kvm/pmu.c > +++ b/arch/x86/kvm/pmu.c > @@ -281,9 +281,13 @@ static int kvm_pmu_rdpmc_vmware(struct kvm_vcpu *vcpu, unsigned idx, u64 *data) > int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data) > { > bool fast_mode = idx & (1u << 31); > + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); > struct kvm_pmc *pmc; > u64 ctr_val; > > + if (!pmu->version) > + return 1; > + > if (is_vmware_backdoor_pmc(idx)) > return kvm_pmu_rdpmc_vmware(vcpu, idx, data); > > Queued, thanks. I wonder if we should expose the state of vmware_backdoors as a capability. It would surely help writing a unit test for this stuff. Paolo
> On 10 Apr 2019, at 12:14, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 25/03/19 20:10, Liran Alon wrote: >> Before this change, reading a VMware pseduo PMC will succeed even when >> PMU is not supported by guest. This can easily be seen by running >> kvm-unit-test vmware_backdoors with "-cpu host,-pmu" option. >> >> Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com> >> Signed-off-by: Liran Alon <liran.alon@oracle.com> >> --- >> arch/x86/kvm/pmu.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c >> index 58ead7db71a3..e39741997893 100644 >> --- a/arch/x86/kvm/pmu.c >> +++ b/arch/x86/kvm/pmu.c >> @@ -281,9 +281,13 @@ static int kvm_pmu_rdpmc_vmware(struct kvm_vcpu *vcpu, unsigned idx, u64 *data) >> int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data) >> { >> bool fast_mode = idx & (1u << 31); >> + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); >> struct kvm_pmc *pmc; >> u64 ctr_val; >> >> + if (!pmu->version) >> + return 1; >> + >> if (is_vmware_backdoor_pmc(idx)) >> return kvm_pmu_rdpmc_vmware(vcpu, idx, data); >> >> > > Queued, thanks. I wonder if we should expose the state of > vmware_backdoors as a capability. It would surely help writing a unit > test for this stuff. > > Paolo I agree. In fact, I think that we currently also others KVM module parameters that should actually be controlled on a per-VM basis. I will include in this list the following: 1) ignore_msrs 2) enable_vmware_backdoor 3) kvmclock_periodic_sync 4) force_emulation_prefix 5) fasteoi 6) vmentry_l1d_flush -Liran
On 10/04/19 13:16, Liran Alon wrote: > > In fact, I think that we currently also others KVM module parameters that should actually be controlled on a per-VM basis. > I will include in this list the following: > 1) ignore_msrs > 2) enable_vmware_backdoor > 3) kvmclock_periodic_sync > 4) force_emulation_prefix > 5) fasteoi > 6) vmentry_l1d_flush I agree about 1/2/3. 6 is about security and it allows leaking host data, so absolutely not. 4 is about debugging so it's not a big deal. Regarding 5, in theory it may make sense but in practice I don't think a guest that does not support fasteoi has ever materialized, and it's not needed on more recent (APICv, by now it's been more than 5 years) server hardware. Paolo
> On 10 Apr 2019, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 10/04/19 13:16, Liran Alon wrote: >> >> In fact, I think that we currently also others KVM module parameters that should actually be controlled on a per-VM basis. >> I will include in this list the following: >> 1) ignore_msrs >> 2) enable_vmware_backdoor >> 3) kvmclock_periodic_sync >> 4) force_emulation_prefix >> 5) fasteoi >> 6) vmentry_l1d_flush > > I agree about 1/2/3. 6 is about security and it allows leaking host > data, so absolutely not. 4 is about debugging so it's not a big deal. > Regarding 5, in theory it may make sense but in practice I don't think a > guest that does not support fasteoi has ever materialized, and it's not > needed on more recent (APICv, by now it's been more than 5 years) server > hardware. > > Paolo Hmm yes I agree with all you say here. :) -Liran
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c index 58ead7db71a3..e39741997893 100644 --- a/arch/x86/kvm/pmu.c +++ b/arch/x86/kvm/pmu.c @@ -281,9 +281,13 @@ static int kvm_pmu_rdpmc_vmware(struct kvm_vcpu *vcpu, unsigned idx, u64 *data) int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data) { bool fast_mode = idx & (1u << 31); + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); struct kvm_pmc *pmc; u64 ctr_val; + if (!pmu->version) + return 1; + if (is_vmware_backdoor_pmc(idx)) return kvm_pmu_rdpmc_vmware(vcpu, idx, data);