| 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
| ||
|
Message-ID: <b8fdbde0-f5d3-484e-877b-c59e8c39bdaa@linux.intel.com>
Date: Wed, 11 Jun 2025 16:58:58 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
Adrian Hunter <adrian.hunter@...el.com>, Maxim Levitsky <mlevitsk@...hat.com>
Subject: Re: [PATCH v6 7/8] KVM: VMX: Wrap all accesses to IA32_DEBUGCTL with
getter/setter APIs
On 6/11/2025 7:20 AM, Sean Christopherson wrote:
> From: Maxim Levitsky <mlevitsk@...hat.com>
>
> Introduce vmx_guest_debugctl_{read,write}() to handle all accesses to
> vmcs.GUEST_IA32_DEBUGCTL. This will allow stuffing FREEZE_IN_SMM into
> GUEST_IA32_DEBUGCTL based on the host setting without bleeding the state
> into the guest, and without needing to copy+paste the FREEZE_IN_SMM
> logic into every patch that accesses GUEST_IA32_DEBUGCTL.
>
> No functional change intended.
>
> Cc: stable@...r.kernel.org
> Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> [sean: massage changelog, make inline, use in all prepare_vmcs02() cases]
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
> arch/x86/kvm/vmx/nested.c | 10 +++++-----
> arch/x86/kvm/vmx/pmu_intel.c | 8 ++++----
> arch/x86/kvm/vmx/vmx.c | 8 +++++---
> arch/x86/kvm/vmx/vmx.h | 10 ++++++++++
> 4 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 5a6c636954eb..9edce9f411a3 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -2662,11 +2662,11 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
> if (vmx->nested.nested_run_pending &&
> (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS)) {
> kvm_set_dr(vcpu, 7, vmcs12->guest_dr7);
> - vmcs_write64(GUEST_IA32_DEBUGCTL, vmcs12->guest_ia32_debugctl &
> - vmx_get_supported_debugctl(vcpu, false));
> + vmx_guest_debugctl_write(vcpu, vmcs12->guest_ia32_debugctl &
> + vmx_get_supported_debugctl(vcpu, false));
> } else {
> kvm_set_dr(vcpu, 7, vcpu->arch.dr7);
> - vmcs_write64(GUEST_IA32_DEBUGCTL, vmx->nested.pre_vmenter_debugctl);
> + vmx_guest_debugctl_write(vcpu, vmx->nested.pre_vmenter_debugctl);
> }
> if (kvm_mpx_supported() && (!vmx->nested.nested_run_pending ||
> !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)))
> @@ -3531,7 +3531,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
>
> if (!vmx->nested.nested_run_pending ||
> !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS))
> - vmx->nested.pre_vmenter_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
> + vmx->nested.pre_vmenter_debugctl = vmx_guest_debugctl_read();
> if (kvm_mpx_supported() &&
> (!vmx->nested.nested_run_pending ||
> !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)))
> @@ -4805,7 +4805,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
> __vmx_set_segment(vcpu, &seg, VCPU_SREG_LDTR);
>
> kvm_set_dr(vcpu, 7, 0x400);
> - vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
> + vmx_guest_debugctl_write(vcpu, 0);
>
> if (nested_vmx_load_msr(vcpu, vmcs12->vm_exit_msr_load_addr,
> vmcs12->vm_exit_msr_load_count))
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 8a94b52c5731..578b4ef58260 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -652,11 +652,11 @@ static void intel_pmu_reset(struct kvm_vcpu *vcpu)
> */
> static void intel_pmu_legacy_freezing_lbrs_on_pmi(struct kvm_vcpu *vcpu)
> {
> - u64 data = vmcs_read64(GUEST_IA32_DEBUGCTL);
> + u64 data = vmx_guest_debugctl_read();
>
> if (data & DEBUGCTLMSR_FREEZE_LBRS_ON_PMI) {
> data &= ~DEBUGCTLMSR_LBR;
> - vmcs_write64(GUEST_IA32_DEBUGCTL, data);
> + vmx_guest_debugctl_write(vcpu, data);
> }
> }
>
> @@ -729,7 +729,7 @@ void vmx_passthrough_lbr_msrs(struct kvm_vcpu *vcpu)
>
> if (!lbr_desc->event) {
> vmx_disable_lbr_msrs_passthrough(vcpu);
> - if (vmcs_read64(GUEST_IA32_DEBUGCTL) & DEBUGCTLMSR_LBR)
> + if (vmx_guest_debugctl_read() & DEBUGCTLMSR_LBR)
> goto warn;
> if (test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use))
> goto warn;
> @@ -751,7 +751,7 @@ void vmx_passthrough_lbr_msrs(struct kvm_vcpu *vcpu)
>
> static void intel_pmu_cleanup(struct kvm_vcpu *vcpu)
> {
> - if (!(vmcs_read64(GUEST_IA32_DEBUGCTL) & DEBUGCTLMSR_LBR))
> + if (!(vmx_guest_debugctl_read() & DEBUGCTLMSR_LBR))
> intel_pmu_release_guest_lbr_event(vcpu);
> }
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index b685e43de4e9..196f33d934d3 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2147,7 +2147,7 @@ int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> msr_info->data = vmx->pt_desc.guest.addr_a[index / 2];
> break;
> case MSR_IA32_DEBUGCTLMSR:
> - msr_info->data = vmcs_read64(GUEST_IA32_DEBUGCTL);
> + msr_info->data = vmx_guest_debugctl_read();
> break;
> default:
> find_uret_msr:
> @@ -2281,7 +2281,8 @@ int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> VM_EXIT_SAVE_DEBUG_CONTROLS)
> get_vmcs12(vcpu)->guest_ia32_debugctl = data;
>
> - vmcs_write64(GUEST_IA32_DEBUGCTL, data);
> + vmx_guest_debugctl_write(vcpu, data);
> +
> if (intel_pmu_lbr_is_enabled(vcpu) && !to_vmx(vcpu)->lbr_desc.event &&
> (data & DEBUGCTLMSR_LBR))
> intel_pmu_create_guest_lbr_event(vcpu);
> @@ -4796,7 +4797,8 @@ static void init_vmcs(struct vcpu_vmx *vmx)
> vmcs_write32(GUEST_SYSENTER_CS, 0);
> vmcs_writel(GUEST_SYSENTER_ESP, 0);
> vmcs_writel(GUEST_SYSENTER_EIP, 0);
> - vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
> +
> + vmx_guest_debugctl_write(&vmx->vcpu, 0);
>
> if (cpu_has_vmx_tpr_shadow()) {
> vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, 0);
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 392e66c7e5fe..c20a4185d10a 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -417,6 +417,16 @@ void vmx_update_cpu_dirty_logging(struct kvm_vcpu *vcpu);
> u64 vmx_get_supported_debugctl(struct kvm_vcpu *vcpu, bool host_initiated);
> bool vmx_is_valid_debugctl(struct kvm_vcpu *vcpu, u64 data, bool host_initiated);
>
> +static inline void vmx_guest_debugctl_write(struct kvm_vcpu *vcpu, u64 val)
> +{
> + vmcs_write64(GUEST_IA32_DEBUGCTL, val);
> +}
> +
> +static inline u64 vmx_guest_debugctl_read(void)
> +{
> + return vmcs_read64(GUEST_IA32_DEBUGCTL);
> +}
> +
> /*
> * Note, early Intel manuals have the write-low and read-high bitmap offsets
> * the wrong way round. The bitmaps control MSRs 0x00000000-0x00001fff and
Reviewed-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
Powered by blists - more mailing lists