From patchwork Wed Dec 16 19:41:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ioanna Alifieraki X-Patchwork-Id: 1417377 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Cx59Z1Z4Bz9sVj; Thu, 17 Dec 2020 06:41:53 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1kpcgI-00011H-V0; Wed, 16 Dec 2020 19:41:50 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kpcgH-00010f-CI for kernel-team@lists.ubuntu.com; Wed, 16 Dec 2020 19:41:49 +0000 Received: from mail-wr1-f71.google.com ([209.85.221.71]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kpcgH-00031l-49 for kernel-team@lists.ubuntu.com; Wed, 16 Dec 2020 19:41:49 +0000 Received: by mail-wr1-f71.google.com with SMTP id w8so9895143wrv.18 for ; Wed, 16 Dec 2020 11:41:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=sR+nw5PNGkroaYeF2Gj/W6i93vUO4jHEPFQ3vcU9lBs=; b=eFrT6SQRi5JJEtEPyHZrvwyWg9H/CTCOfyWQ8TdS1sVVePAeZmgJcVPX5U7SJEtHxP kRuU3QhxbZBIC23sERwoW4LbPx+bVnfE/s4nGhEzwiY7XkVgFk8rMR7fAOQN+LSkzddA BTNktcHi1FQ2wsKF6ZFAN8Ar+u8OR+W8yS7qRhcdZkEtyD8v/+/GeWKTwgKt5wB+kfZY 5CjdQCUiwWqgytSAa+xfRkGnFiJlBKLAtK8ZukiMrvw+TlrP0lz7Q+VpbgSDLoCBLnyB 5SVVwL0n2e2F3YoLNyPwNhkMFUmD1l8f28ySfFsIiP8mTUxXXhiFmPEaMNNr0iyMPiWh GoCQ== X-Gm-Message-State: AOAM530nA6HWM6poQ74KPtob3p7RhFoeUsYTALityLOMR/JaYqElvEhj Krvqzzw3P+O0LnMI/0f25HjqDXZQ4v1KjXH1WV81WQLYZBVXwoFsuFkwXa9Z2QaP28yGK58keW1 AduBIpjsCXiuRJQXF70z5ss7nkh+e4C5sn2WxD/Shmg== X-Received: by 2002:a5d:4ad0:: with SMTP id y16mr14218945wrs.424.1608147708627; Wed, 16 Dec 2020 11:41:48 -0800 (PST) X-Google-Smtp-Source: ABdhPJyCGLe9yJXRLdWLXUyrewC+YbuOpeLe2fWpe6yNqhSLl5u6ALsVwDzOF21MgwLeO0LNu25aeQ== X-Received: by 2002:a5d:4ad0:: with SMTP id y16mr14218929wrs.424.1608147708350; Wed, 16 Dec 2020 11:41:48 -0800 (PST) Received: from localhost ([2001:67c:1560:8007::aac:c2e0]) by smtp.gmail.com with ESMTPSA id b7sm4276291wrv.47.2020.12.16.11.41.47 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 16 Dec 2020 11:41:47 -0800 (PST) From: Ioanna Alifieraki To: kernel-team@lists.ubuntu.com Subject: [SRU][Xenial][PATCH 2/3] KVM: VMX: avoid double list add with VT-d posted interrupts Date: Wed, 16 Dec 2020 19:41:44 +0000 Message-Id: <20201216194145.24239-3-ioanna-maria.alifieraki@canonical.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20201216194145.24239-1-ioanna-maria.alifieraki@canonical.com> References: <20201216194145.24239-1-ioanna-maria.alifieraki@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Paolo Bonzini BugLink: https://bugs.launchpad.net/bugs/1908428 In some cases, for example involving hot-unplug of assigned devices, pi_post_block can forget to remove the vCPU from the blocked_vcpu_list. When this happens, the next call to pi_pre_block corrupts the list. Fix this in two ways. First, check vcpu->pre_pcpu in pi_pre_block and WARN instead of adding the element twice in the list. Second, always do the list removal in pi_post_block if vcpu->pre_pcpu is set (not -1). The new code keeps interrupts disabled for the whole duration of pi_pre_block/pi_post_block. This is not strictly necessary, but easier to follow. For the same reason, PI.ON is checked only after the cmpxchg, and to handle it we just call the post-block code. This removes duplication of the list removal code. Cc: Huangweidong Cc: Gonglei Cc: wangxin Cc: Radim Krčmář Tested-by: Longpeng (Mike) Cc: stable@vger.kernel.org Signed-off-by: Paolo Bonzini (backported from commit 8b306e2f3c41939ea528e6174c88cfbfff893ce1) [hunk 4 and 5 : Original commit changes pi_pre_block function. This function is not present in 4.4 kernel, see upstream commit bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block). Make relevant changes in vmx_pre_block function.] Signed-off-by: Ioanna Alifieraki --- arch/x86/kvm/vmx.c | 65 +++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 472d089f0bd2..19589b1107b2 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -11150,10 +11150,11 @@ static void __pi_post_block(struct kvm_vcpu *vcpu) struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); struct pi_desc old, new; unsigned int dest; - unsigned long flags; do { old.control = new.control = pi_desc->control; + WARN(old.nv != POSTED_INTR_WAKEUP_VECTOR, + "Wakeup handler not enabled while the VCPU is blocked\n"); dest = cpu_physical_id(vcpu->cpu); @@ -11170,14 +11171,10 @@ static void __pi_post_block(struct kvm_vcpu *vcpu) } while (cmpxchg(&pi_desc->control, old.control, new.control) != old.control); - if(vcpu->pre_pcpu != -1) { - spin_lock_irqsave( - &per_cpu(blocked_vcpu_on_cpu_lock, - vcpu->pre_pcpu), flags); + if (!WARN_ON_ONCE(vcpu->pre_pcpu == -1)) { + spin_lock(&per_cpu(blocked_vcpu_on_cpu_lock, vcpu->pre_pcpu)); list_del(&vcpu->blocked_vcpu_list); - spin_unlock_irqrestore( - &per_cpu(blocked_vcpu_on_cpu_lock, - vcpu->pre_pcpu), flags); + spin_unlock(&per_cpu(blocked_vcpu_on_cpu_lock, vcpu->pre_pcpu)); vcpu->pre_pcpu = -1; } } @@ -11197,7 +11194,6 @@ static void __pi_post_block(struct kvm_vcpu *vcpu) */ static int vmx_pre_block(struct kvm_vcpu *vcpu) { - unsigned long flags; unsigned int dest; struct pi_desc old, new; struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); @@ -11206,34 +11202,20 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu) !irq_remapping_cap(IRQ_POSTING_CAP)) return 0; - vcpu->pre_pcpu = vcpu->cpu; - spin_lock_irqsave(&per_cpu(blocked_vcpu_on_cpu_lock, - vcpu->pre_pcpu), flags); - list_add_tail(&vcpu->blocked_vcpu_list, - &per_cpu(blocked_vcpu_on_cpu, - vcpu->pre_pcpu)); - spin_unlock_irqrestore(&per_cpu(blocked_vcpu_on_cpu_lock, - vcpu->pre_pcpu), flags); + WARN_ON(irqs_disabled()); + local_irq_disable(); + if (!WARN_ON_ONCE(vcpu->pre_pcpu != -1)) { + vcpu->pre_pcpu = vcpu->cpu; + spin_lock(&per_cpu(blocked_vcpu_on_cpu_lock, vcpu->pre_pcpu)); + list_add_tail(&vcpu->blocked_vcpu_list, + &per_cpu(blocked_vcpu_on_cpu, + vcpu->pre_pcpu)); + spin_unlock(&per_cpu(blocked_vcpu_on_cpu_lock, vcpu->pre_pcpu)); + } do { old.control = new.control = pi_desc->control; - /* - * We should not block the vCPU if - * an interrupt is posted for it. - */ - if (pi_test_on(pi_desc) == 1) { - spin_lock_irqsave(&per_cpu(blocked_vcpu_on_cpu_lock, - vcpu->pre_pcpu), flags); - list_del(&vcpu->blocked_vcpu_list); - spin_unlock_irqrestore( - &per_cpu(blocked_vcpu_on_cpu_lock, - vcpu->pre_pcpu), flags); - vcpu->pre_pcpu = -1; - - return 1; - } - WARN((pi_desc->sn == 1), "Warning: SN field of posted-interrupts " "is set before blocking\n"); @@ -11255,19 +11237,26 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu) /* set 'NV' to 'wakeup vector' */ new.nv = POSTED_INTR_WAKEUP_VECTOR; - } while (cmpxchg64(&pi_desc->control, old.control, - new.control) != old.control); + } while (cmpxchg(&pi_desc->control, old.control, + new.control) != old.control); - return 0; + /* We should not block the vCPU if an interrupt is posted for it. */ + if (pi_test_on(pi_desc) == 1) + __pi_post_block(vcpu); + + local_irq_enable(); + return (vcpu->pre_pcpu == -1); } static void vmx_post_block(struct kvm_vcpu *vcpu) { - if (!kvm_arch_has_assigned_device(vcpu->kvm) || - !irq_remapping_cap(IRQ_POSTING_CAP)) + if (vcpu->pre_pcpu == -1) return; + WARN_ON(irqs_disabled()); + local_irq_disable(); __pi_post_block(vcpu); + local_irq_enable(); } /*