| 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: <5d1d421c-3123-455e-aba1-1baf7f12e89e@suse.com>
Date: Thu, 9 Jan 2025 17:38:36 +0200
From: Nikolay Borisov <nik.borisov@...e.com>
To: Binbin Wu <binbin.wu@...ux.intel.com>, pbonzini@...hat.com,
seanjc@...gle.com, kvm@...r.kernel.org
Cc: rick.p.edgecombe@...el.com, kai.huang@...el.com, adrian.hunter@...el.com,
reinette.chatre@...el.com, xiaoyao.li@...el.com,
tony.lindgren@...ux.intel.com, isaku.yamahata@...el.com,
yan.y.zhao@...el.com, chao.gao@...el.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/16] KVM: TDX: Add support for find pending IRQ in a
protected local APIC
On 9.12.24 г. 3:07 ч., Binbin Wu wrote:
> From: Sean Christopherson <seanjc@...gle.com>
>
> Add flag and hook to KVM's local APIC management to support determining
> whether or not a TDX guest as a pending IRQ. For TDX vCPUs, the virtual
> APIC page is owned by the TDX module and cannot be accessed by KVM. As a
> result, registers that are virtualized by the CPU, e.g. PPR, cannot be
> read or written by KVM. To deliver interrupts for TDX guests, KVM must
> send an IRQ to the CPU on the posted interrupt notification vector. And
> to determine if TDX vCPU has a pending interrupt, KVM must check if there
> is an outstanding notification.
>
> Return "no interrupt" in kvm_apic_has_interrupt() if the guest APIC is
> protected to short-circuit the various other flows that try to pull an
> IRQ out of the vAPIC, the only valid operation is querying _if_ an IRQ is
> pending, KVM can't do anything based on _which_ IRQ is pending.
>
> Intentionally omit sanity checks from other flows, e.g. PPR update, so as
> not to degrade non-TDX guests with unnecessary checks. A well-behaved KVM
> and userspace will never reach those flows for TDX guests, but reaching
> them is not fatal if something does go awry.
>
> Note, this doesn't handle interrupts that have been delivered to the vCPU
> but not yet recognized by the core, i.e. interrupts that are sitting in
> vmcs.GUEST_INTR_STATUS. Querying that state requires a SEAMCALL and will
> be supported in a future patch.
>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@...el.com>
> Signed-off-by: Binbin Wu <binbin.wu@...ux.intel.com>
> ---
> TDX interrupts breakout:
> - Dropped vt_protected_apic_has_interrupt() with KVM_BUG_ON(), wire in
> tdx_protected_apic_has_interrupt() directly. (Rick)
> - Add {} on else in vt_hardware_setup()
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 1 +
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/irq.c | 3 +++
> arch/x86/kvm/lapic.c | 3 +++
> arch/x86/kvm/lapic.h | 2 ++
> arch/x86/kvm/vmx/main.c | 3 +++
> arch/x86/kvm/vmx/tdx.c | 6 ++++++
> arch/x86/kvm/vmx/x86_ops.h | 2 ++
> 8 files changed, 21 insertions(+)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index ec1b1b39c6b3..d5faaaee6ac0 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -114,6 +114,7 @@ KVM_X86_OP_OPTIONAL(pi_start_assignment)
> KVM_X86_OP_OPTIONAL(apicv_pre_state_restore)
> KVM_X86_OP_OPTIONAL(apicv_post_state_restore)
> KVM_X86_OP_OPTIONAL_RET0(dy_apicv_has_pending_interrupt)
> +KVM_X86_OP_OPTIONAL(protected_apic_has_interrupt)
> KVM_X86_OP_OPTIONAL(set_hv_timer)
> KVM_X86_OP_OPTIONAL(cancel_hv_timer)
> KVM_X86_OP(setup_mce)
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 37dc7edef1ca..32c7d58a5d68 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1811,6 +1811,7 @@ struct kvm_x86_ops {
> void (*apicv_pre_state_restore)(struct kvm_vcpu *vcpu);
> void (*apicv_post_state_restore)(struct kvm_vcpu *vcpu);
> bool (*dy_apicv_has_pending_interrupt)(struct kvm_vcpu *vcpu);
> + bool (*protected_apic_has_interrupt)(struct kvm_vcpu *vcpu);
>
> int (*set_hv_timer)(struct kvm_vcpu *vcpu, u64 guest_deadline_tsc,
> bool *expired);
> diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
> index 63f66c51975a..f0644d0bbe11 100644
> --- a/arch/x86/kvm/irq.c
> +++ b/arch/x86/kvm/irq.c
> @@ -100,6 +100,9 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v)
> if (kvm_cpu_has_extint(v))
> return 1;
>
> + if (lapic_in_kernel(v) && v->arch.apic->guest_apic_protected)
> + return static_call(kvm_x86_protected_apic_has_interrupt)(v);
> +
> return kvm_apic_has_interrupt(v) != -1; /* LAPIC */
> }
> EXPORT_SYMBOL_GPL(kvm_cpu_has_interrupt);
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 65412640cfc7..684777c2f0a4 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2920,6 +2920,9 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
> if (!kvm_apic_present(vcpu))
> return -1;
>
> + if (apic->guest_apic_protected)
> + return -1;
> +
> __apic_update_ppr(apic, &ppr);
> return apic_has_interrupt_for_ppr(apic, ppr);
> }
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 1b8ef9856422..82355faf8c0d 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -65,6 +65,8 @@ struct kvm_lapic {
> bool sw_enabled;
> bool irr_pending;
> bool lvt0_in_nmi_mode;
> + /* Select registers in the vAPIC cannot be read/written. */
> + bool guest_apic_protected;
Can't this member be eliminated and instead is_td_vcpu() used as it
stands currently that member is simply a proxy value for "is this a tdx
vcpu"?
<snip>
Powered by blists - more mailing lists