linux-kernel - Re: [PATCH v10 104/108] KVM: TDX: Silently ignore INIT/SIPI

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y5yToW++7EVrMiMN@google.com>
Date:   Fri, 16 Dec 2022 15:49:53 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     isaku.yamahata@...el.com
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        isaku.yamahata@...il.com, Paolo Bonzini <pbonzini@...hat.com>,
        erdemaktas@...gle.com, Sagi Shahar <sagis@...gle.com>,
        David Matlack <dmatlack@...gle.com>
Subject: Re: [PATCH v10 104/108] KVM: TDX: Silently ignore INIT/SIPI

On Sat, Oct 29, 2022, isaku.yamahata@...el.com wrote:
> From: Isaku Yamahata <isaku.yamahata@...el.com>
> 
> The TDX module API doesn't provide API for VMM to inject INIT IPI and SIPI.
> Instead it defines the different protocols to boot application processors.
> Ignore INIT and SIPI events for the TDX guest.
> 
> There are two options. 1) (silently) ignore INIT/SIPI request or 2) return
> error to guest TDs somehow.  Given that TDX guest is paravirtualized to
> boot AP, the option 1 is chosen for simplicity.
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@...el.com>
> ---

...

> diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
> index 4acba8d8cb27..d776d5d169d0 100644
> --- a/arch/x86/kvm/vmx/main.c
> +++ b/arch/x86/kvm/vmx/main.c
> @@ -286,6 +286,25 @@ static void vt_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
>  	vmx_deliver_interrupt(apic, delivery_mode, trig_mode, vector);
>  }
>  
> +static void vt_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
> +{
> +	if (is_td_vcpu(vcpu))
> +		return;
> +
> +	kvm_vcpu_deliver_sipi_vector(vcpu, vector);
> +}
> +
> +static void vt_vcpu_deliver_init(struct kvm_vcpu *vcpu)
> +{
> +	if (is_td_vcpu(vcpu)) {
> +		/* TDX doesn't support INIT.  Ignore INIT event */
> +		vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
> +		return;
> +	}
> +
> +	kvm_vcpu_deliver_init(vcpu);
> +}
> +
>  static void vt_flush_tlb_all(struct kvm_vcpu *vcpu)
>  {
>  	if (is_td_vcpu(vcpu))
> @@ -627,7 +646,8 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
>  	.msr_filter_changed = vmx_msr_filter_changed,
>  	.complete_emulated_msr = kvm_complete_insn_gp,
>  
> -	.vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector,
> +	.vcpu_deliver_sipi_vector = vt_vcpu_deliver_sipi_vector,
> +	.vcpu_deliver_init = vt_vcpu_deliver_init,

A simpler, and arguably more correct, appraoch would be to hook .apic_init_signal_blocked()
and have that return true for TDX.  Waiting until delivery means the vCPU will get
spurious wake events, e.g. KVM will wake the vCPU to service the INIT, but then
ignore the INIT.  Of course, sending the bogus INIT/SIPI in the first place
is a guest bug.

That would also prevent userspace from putting the vCPU into INIT/SIPI via
KVM_SET_MP_STATE.

Ideally, KVM would never mark INIT or SIPI pending in the first place, though I'm
not sure that's worth the effort.