linux-kernel - Re: [PATCH v2 09/11] KVM: x86: Differentiate Soft vs. Hard IRQs vs. reinjected in tracepoint

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: <08d0cbcd-14dd-b9c6-82cf-a0d11c5cccd9@maciej.szmigiero.name>
Date:   Tue, 26 Apr 2022 00:59:48 +0200
From:   "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Maxim Levitsky <mlevitsk@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v2 09/11] KVM: x86: Differentiate Soft vs. Hard IRQs vs.
 reinjected in tracepoint

On 23.04.2022 04:14, Sean Christopherson wrote:
> In the IRQ injection tracepoint, differentiate between Hard IRQs and Soft
> "IRQs", i.e. interrupts that are reinjected after incomplete delivery of
> a software interrupt from an INTn instruction.  Tag reinjected interrupts
> as such, even though the information is usually redundant since soft
> interrupts are only ever reinjected by KVM.  Though rare in practice, a
> hard IRQ can be reinjected.
> 
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>   arch/x86/include/asm/kvm_host.h |  2 +-
>   arch/x86/kvm/svm/svm.c          |  5 +++--
>   arch/x86/kvm/trace.h            | 16 +++++++++++-----
>   arch/x86/kvm/vmx/vmx.c          |  4 ++--
>   arch/x86/kvm/x86.c              |  4 ++--
>   5 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index f164c6c1514a..ae088c6fb287 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1400,7 +1400,7 @@ struct kvm_x86_ops {
>   	u32 (*get_interrupt_shadow)(struct kvm_vcpu *vcpu);
>   	void (*patch_hypercall)(struct kvm_vcpu *vcpu,
>   				unsigned char *hypercall_addr);
> -	void (*inject_irq)(struct kvm_vcpu *vcpu);
> +	void (*inject_irq)(struct kvm_vcpu *vcpu, bool reinjected);
>   	void (*inject_nmi)(struct kvm_vcpu *vcpu);
>   	void (*queue_exception)(struct kvm_vcpu *vcpu);
>   	void (*cancel_injection)(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index b8fb07eeeca5..4a912623b961 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3427,7 +3427,7 @@ static void svm_inject_nmi(struct kvm_vcpu *vcpu)
>   	++vcpu->stat.nmi_injections;
>   }
>   
> -static void svm_inject_irq(struct kvm_vcpu *vcpu)
> +static void svm_inject_irq(struct kvm_vcpu *vcpu, bool reinjected)
>   {
>   	struct vcpu_svm *svm = to_svm(vcpu);
>   	u32 type;
> @@ -3442,7 +3442,8 @@ static void svm_inject_irq(struct kvm_vcpu *vcpu)
>   		type = SVM_EVTINJ_TYPE_INTR;
>   	}
>   
> -	trace_kvm_inj_virq(vcpu->arch.interrupt.nr);
> +	trace_kvm_inj_virq(vcpu->arch.interrupt.nr,
> +			   vcpu->arch.interrupt.soft, reinjected);
>   	++vcpu->stat.irq_injections;
>   
>   	svm->vmcb->control.event_inj = vcpu->arch.interrupt.nr |
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index 385436d12024..e1b089285c55 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -333,18 +333,24 @@ TRACE_EVENT_KVM_EXIT(kvm_exit);
>    * Tracepoint for kvm interrupt injection:
>    */
>   TRACE_EVENT(kvm_inj_virq,
> -	TP_PROTO(unsigned int irq),
> -	TP_ARGS(irq),
> +	TP_PROTO(unsigned int vector, bool soft, bool reinjected),
> +	TP_ARGS(vector, soft, reinjected),
>   
>   	TP_STRUCT__entry(
> -		__field(	unsigned int,	irq		)
> +		__field(	unsigned int,	vector		)
> +		__field(	bool,		soft		)
> +		__field(	unsigned int,	reinjected	)

The "reinjected" field was probably supposed to be bool, just like
in the trace function prototype.

Thanks,
Maciej