linux-kernel - Re: [PATCH 1/3] KVM: LAPIC: Make lapic timer unpinned when timer is injected by posted-interrupt

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]

Date:   Wed, 5 Jun 2019 15:04:04 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Wanpeng Li <kernellwp@...il.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     Radim Krčmář <rkrcmar@...hat.com>
Subject: Re: [PATCH 1/3] KVM: LAPIC: Make lapic timer unpinned when timer is
 injected by posted-interrupt

On 05/06/19 12:09, Wanpeng Li wrote:
> +static inline bool posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
> +{
> +	return (kvm_x86_ops->pi_inject_timer_enabled(vcpu) &&
> +		kvm_mwait_in_guest(vcpu->kvm));
> +}
> +

Here you need to check kvm_halt_in_guest, not kvm_mwait_in_guest,
because you need to go through kvm_apic_expired if the guest needs to be
woken up from kvm_vcpu_block.

There is a case when you get to kvm_vcpu_block with kvm_halt_in_guest,
which is when the guest disables asynchronous page faults.  Currently,
timer interrupts are delivered while apf.halted = true, with this change
they wouldn't.  I would just disable KVM_REQ_APF_HALT in
kvm_can_do_async_pf if kvm_halt_in_guest is true, let me send a patch
for that later.

When you do this, I think you don't need the
kvm_x86_ops->pi_inject_timer_enabled check at all, because if we know
that the vCPU cannot be asleep in kvm_vcpu_block, then we can inject the
timer interrupt immediately with __apic_accept_irq (if APICv is
disabled, it will set IRR and do kvm_make_request + kvm_vcpu_kick).

You can keep the module parameter, mostly for debugging reasons, but
please move it from kvm-intel to kvm, and add something like

diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 123ea07a3f3b..1cc7973c382e 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -14,6 +14,11 @@
 static cpumask_var_t housekeeping_mask;
 static unsigned int housekeeping_flags;

+bool housekeeping_enabled(enum hk_flags flags)
+{
+	return !!(housekeeping_flags & flags);
+}
+
 int housekeeping_any_cpu(enum hk_flags flags)
 {
 	if (static_branch_unlikely(&housekeeping_overridden))

so that the default for the module parameter can be
housekeeping_enabled(HK_FLAG_TIMER).

Thanks,

Paolo