[SRU][Xenial][PATCH v3 0/4] Fix corruption on blocked_vcpu_on_cpu list

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH v3 0/4] Fix corruption on blocked_vcpu_on_cpu list

Ioanna Alifieraki
BugLink: https://bugs.launchpad.net/bugs/1908428

The following patches fix the bug reported in [1].
The first patch bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block)
is pulled into to facilitate the backport of the next 3 patches that fix the bug.
Original patches come from [2] and have been accepted upstream.

[1] https://marc.info/?l=kvm&m=149559827906211&w=2
[2] https://lore.kernel.org/lkml/20170606105707.23207-1-pbonzini@.../

Paolo Bonzini (3):
  KVM: VMX: extract __pi_post_block
  KVM: VMX: avoid double list add with VT-d posted interrupts
  KVM: VMX: simplify and fix vmx_vcpu_pi_load

Yunhong Jiang (1):
  kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block

 arch/x86/kvm/vmx.c | 190 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 100 insertions(+), 90 deletions(-)

--
1.9.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH v3 1/4] kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block

Ioanna Alifieraki
From: Yunhong Jiang <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1908428

Prepare to switch from preemption timer to hrtimer in the
vmx_pre/post_block. Current functions are only for posted interrupt,
rename them accordingly.

Signed-off-by: Yunhong Jiang <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
(cherry picked from commit bc22512bb24c480fae8ae96b233378ef96007590)
Signed-off-by: Ioanna Alifieraki <[hidden email]>
---
 arch/x86/kvm/vmx.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 46d1293..d73ad4b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11158,7 +11158,7 @@ static void vmx_enable_log_dirty_pt_masked(struct kvm *kvm,
  *   this case, return 1, otherwise, return 0.
  *
  */
-static int vmx_pre_block(struct kvm_vcpu *vcpu)
+static int pi_pre_block(struct kvm_vcpu *vcpu)
 {
  unsigned long flags;
  unsigned int dest;
@@ -11224,7 +11224,15 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu)
  return 0;
 }
 
-static void vmx_post_block(struct kvm_vcpu *vcpu)
+static int vmx_pre_block(struct kvm_vcpu *vcpu)
+{
+ if (pi_pre_block(vcpu))
+ return 1;
+
+ return 0;
+}
+
+static void pi_post_block(struct kvm_vcpu *vcpu)
 {
  struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu);
  struct pi_desc old, new;
@@ -11265,6 +11273,11 @@ static void vmx_post_block(struct kvm_vcpu *vcpu)
  }
 }
 
+static void vmx_post_block(struct kvm_vcpu *vcpu)
+{
+ pi_post_block(vcpu);
+}
+
 /*
  * vmx_update_pi_irte - set IRTE for Posted-Interrupts
  *
--
1.9.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH v3 2/4] KVM: VMX: extract __pi_post_block

Ioanna Alifieraki
In reply to this post by Ioanna Alifieraki
From: Paolo Bonzini <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1908428

Simple code movement patch, preparing for the next one.

Cc: Huangweidong <[hidden email]>
Cc: Gonglei <[hidden email]>
Cc: wangxin <[hidden email]>
Cc: Radim Krčmář <[hidden email]>
Tested-by: Longpeng (Mike) <[hidden email]>
Cc: [hidden email]
Signed-off-by: Paolo Bonzini <[hidden email]>
(backported from commit cd39e1176d320157831ce030b4c869bd2d5eb142)
[hunk 1 : replace cmpxchg with cmpxchg64 for i386 build.
hunk 2 : resolve if-statement conflict.]
Signed-off-by: Ioanna Alifieraki <[hidden email]>
---
 arch/x86/kvm/vmx.c | 71 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index d73ad4b..93d922a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11145,6 +11145,43 @@ static void vmx_enable_log_dirty_pt_masked(struct kvm *kvm,
  kvm_mmu_clear_dirty_pt_masked(kvm, memslot, offset, mask);
 }
 
+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;
+
+ dest = cpu_physical_id(vcpu->cpu);
+
+ if (x2apic_enabled())
+ new.ndst = dest;
+ else
+ new.ndst = (dest << 8) & 0xFF00;
+
+ /* Allow posting non-urgent interrupts */
+ new.sn = 0;
+
+ /* set 'NV' to 'notification vector' */
+ new.nv = POSTED_INTR_VECTOR;
+ } while (cmpxchg64(&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);
+ list_del(&vcpu->blocked_vcpu_list);
+ spin_unlock_irqrestore(
+ &per_cpu(blocked_vcpu_on_cpu_lock,
+ vcpu->pre_pcpu), flags);
+ vcpu->pre_pcpu = -1;
+ }
+}
+
 /*
  * This routine does the following things for vCPU which is going
  * to be blocked if VT-d PI is enabled.
@@ -11234,43 +11271,11 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu)
 
 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;
-
  if (!kvm_arch_has_assigned_device(vcpu->kvm) ||
  !irq_remapping_cap(IRQ_POSTING_CAP))
  return;
 
- do {
- old.control = new.control = pi_desc->control;
-
- dest = cpu_physical_id(vcpu->cpu);
-
- if (x2apic_enabled())
- new.ndst = dest;
- else
- new.ndst = (dest << 8) & 0xFF00;
-
- /* Allow posting non-urgent interrupts */
- new.sn = 0;
-
- /* set 'NV' to 'notification vector' */
- new.nv = POSTED_INTR_VECTOR;
- } while (cmpxchg64(&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);
- list_del(&vcpu->blocked_vcpu_list);
- spin_unlock_irqrestore(
- &per_cpu(blocked_vcpu_on_cpu_lock,
- vcpu->pre_pcpu), flags);
- vcpu->pre_pcpu = -1;
- }
+ __pi_post_block(vcpu);
 }
 
 static void vmx_post_block(struct kvm_vcpu *vcpu)
--
1.9.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH v3 3/4] KVM: VMX: avoid double list add with VT-d posted interrupts

Ioanna Alifieraki
In reply to this post by Ioanna Alifieraki
From: Paolo Bonzini <[hidden email]>

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 <[hidden email]>
Cc: Gonglei <[hidden email]>
Cc: wangxin <[hidden email]>
Cc: Radim Krčmář <[hidden email]>
Tested-by: Longpeng (Mike) <[hidden email]>
Cc: [hidden email]
Signed-off-by: Paolo Bonzini <[hidden email]>
(backported from commit 8b306e2f3c41939ea528e6174c88cfbfff893ce1)
[hunk 5 : resolve if-statement conflict.]
Signed-off-by: Ioanna Alifieraki <[hidden email]>
---
 arch/x86/kvm/vmx.c | 61 ++++++++++++++++++++++--------------------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 93d922a..a9d677c 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 (cmpxchg64(&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 pi_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 pi_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");
@@ -11258,7 +11240,12 @@ static int pi_pre_block(struct kvm_vcpu *vcpu)
  } while (cmpxchg64(&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 int vmx_pre_block(struct kvm_vcpu *vcpu)
@@ -11271,11 +11258,13 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu)
 
 static void pi_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();
 }
 
 static void vmx_post_block(struct kvm_vcpu *vcpu)
--
1.9.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH v3 4/4] KVM: VMX: simplify and fix vmx_vcpu_pi_load

Ioanna Alifieraki
In reply to this post by Ioanna Alifieraki
From: Paolo Bonzini <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1908428

The simplify part: do not touch pi_desc.nv, we can set it when the
VCPU is first created.  Likewise, pi_desc.sn is only handled by
vmx_vcpu_pi_load, do not touch it in __pi_post_block.

The fix part: do not check kvm_arch_has_assigned_device, instead
check the SN bit to figure out whether vmx_vcpu_pi_put ran before.
This matches what the previous patch did in pi_post_block.

Cc: Huangweidong <[hidden email]>
Cc: Gonglei <[hidden email]>
Cc: wangxin <[hidden email]>
Cc: Radim Krčmář <[hidden email]>
Tested-by: Longpeng (Mike) <[hidden email]>
Cc: [hidden email]
Signed-off-by: Paolo Bonzini <[hidden email]>
(backported from commit 31afb2ea2b10a7d17ce3db4cdb0a12b63b2fe08a)
[hunk 1 : resolve if-statement conflict.]
Signed-off-by: Ioanna Alifieraki <[hidden email]>
---
 arch/x86/kvm/vmx.c | 67 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index a9d677c..4a56bf3 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2235,42 +2235,41 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu)
  struct pi_desc old, new;
  unsigned int dest;
 
- if (!kvm_arch_has_assigned_device(vcpu->kvm) ||
- !irq_remapping_cap(IRQ_POSTING_CAP))
+ /*
+ * In case of hot-plug or hot-unplug, we may have to undo
+ * vmx_vcpu_pi_put even if there is no assigned device.  And we
+ * always keep PI.NDST up to date for simplicity: it makes the
+ * code easier, and CPU migration is not a fast path.
+ */
+ if (!pi_test_sn(pi_desc) && vcpu->cpu == cpu)
+ return;
+
+ /*
+ * First handle the simple case where no cmpxchg is necessary; just
+ * allow posting non-urgent interrupts.
+ *
+ * If the 'nv' field is POSTED_INTR_WAKEUP_VECTOR, do not change
+ * PI.NDST: pi_post_block will do it for us and the wakeup_handler
+ * expects the VCPU to be on the blocked_vcpu_list that matches
+ * PI.NDST.
+ */
+ if (pi_desc->nv == POSTED_INTR_WAKEUP_VECTOR ||
+    vcpu->cpu == cpu) {
+ pi_clear_sn(pi_desc);
  return;
+ }
 
+ /* The full case.  */
  do {
  old.control = new.control = pi_desc->control;
 
- /*
- * If 'nv' field is POSTED_INTR_WAKEUP_VECTOR, there
- * are two possible cases:
- * 1. After running 'pre_block', context switch
- *    happened. For this case, 'sn' was set in
- *    vmx_vcpu_put(), so we need to clear it here.
- * 2. After running 'pre_block', we were blocked,
- *    and woken up by some other guy. For this case,
- *    we don't need to do anything, 'pi_post_block'
- *    will do everything for us. However, we cannot
- *    check whether it is case #1 or case #2 here
- *    (maybe, not needed), so we also clear sn here,
- *    I think it is not a big deal.
- */
- if (pi_desc->nv != POSTED_INTR_WAKEUP_VECTOR) {
- if (vcpu->cpu != cpu) {
- dest = cpu_physical_id(cpu);
-
- if (x2apic_enabled())
- new.ndst = dest;
- else
- new.ndst = (dest << 8) & 0xFF00;
- }
+ dest = cpu_physical_id(cpu);
 
- /* set 'NV' to 'notification vector' */
- new.nv = POSTED_INTR_VECTOR;
- }
+ if (x2apic_enabled())
+ new.ndst = dest;
+ else
+ new.ndst = (dest << 8) & 0xFF00;
 
- /* Allow posting non-urgent interrupts */
  new.sn = 0;
  } while (cmpxchg64(&pi_desc->control, old.control,
    new.control) != old.control);
@@ -9283,6 +9282,13 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
  vmx->nested.current_vmptr = -1ull;
  vmx->nested.current_vmcs12 = NULL;
 
+ /*
+ * Enforce invariant: pi_desc.nv is always either POSTED_INTR_VECTOR
+ * or POSTED_INTR_WAKEUP_VECTOR.
+ */
+ vmx->pi_desc.nv = POSTED_INTR_VECTOR;
+ vmx->pi_desc.sn = 1;
+
  return &vmx->vcpu;
 
 free_vmcs:
@@ -11163,9 +11169,6 @@ static void __pi_post_block(struct kvm_vcpu *vcpu)
  else
  new.ndst = (dest << 8) & 0xFF00;
 
- /* Allow posting non-urgent interrupts */
- new.sn = 0;
-
  /* set 'NV' to 'notification vector' */
  new.nv = POSTED_INTR_VECTOR;
  } while (cmpxchg64(&pi_desc->control, old.control,
--
1.9.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Xenial][PATCH v3 3/4] KVM: VMX: avoid double list add with VT-d posted interrupts

Stefan Bader-2
In reply to this post by Ioanna Alifieraki
On 19.01.21 00:22, Ioanna Alifieraki wrote:

> From: Paolo Bonzini <[hidden email]>
>
> 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 <[hidden email]>
> Cc: Gonglei <[hidden email]>
> Cc: wangxin <[hidden email]>
> Cc: Radim Krčmář <[hidden email]>
> Tested-by: Longpeng (Mike) <[hidden email]>
> Cc: [hidden email]
> Signed-off-by: Paolo Bonzini <[hidden email]>
> (backported from commit 8b306e2f3c41939ea528e6174c88cfbfff893ce1)
> [hunk 5 : resolve if-statement conflict.]
I think this was "adjusting for context change (cmpxchg)". Generally anything
where a hunk fails because the lines above or below do not quite match but the
lines added or removed are identical to the original patch I would refer to as
some form of context adjustments.


> Signed-off-by: Ioanna Alifieraki <[hidden email]>
> ---
>  arch/x86/kvm/vmx.c | 61 ++++++++++++++++++++++--------------------------------
>  1 file changed, 25 insertions(+), 36 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 93d922a..a9d677c 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 (cmpxchg64(&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 pi_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 pi_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");
> @@ -11258,7 +11240,12 @@ static int pi_pre_block(struct kvm_vcpu *vcpu)
>   } while (cmpxchg64(&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 int vmx_pre_block(struct kvm_vcpu *vcpu)
> @@ -11271,11 +11258,13 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu)
>  
>  static void pi_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();
>  }
>  
>  static void vmx_post_block(struct kvm_vcpu *vcpu)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [SRU][Xenial][PATCH v3 0/4] Fix corruption on blocked_vcpu_on_cpu list

Stefan Bader-2
In reply to this post by Ioanna Alifieraki
On 19.01.21 00:22, Ioanna Alifieraki wrote:

> BugLink: https://bugs.launchpad.net/bugs/1908428
>
> The following patches fix the bug reported in [1].
> The first patch bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block)
> is pulled into to facilitate the backport of the next 3 patches that fix the bug.
> Original patches come from [2] and have been accepted upstream.
>
> [1] https://marc.info/?l=kvm&m=149559827906211&w=2
> [2] https://lore.kernel.org/lkml/20170606105707.23207-1-pbonzini@.../
>
> Paolo Bonzini (3):
>   KVM: VMX: extract __pi_post_block
>   KVM: VMX: avoid double list add with VT-d posted interrupts
>   KVM: VMX: simplify and fix vmx_vcpu_pi_load
>
> Yunhong Jiang (1):
>   kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block
>
>  arch/x86/kvm/vmx.c | 190 ++++++++++++++++++++++++++++-------------------------
>  1 file changed, 100 insertions(+), 90 deletions(-)
>
Statements about the regression potential in the bug report still apply. What I
mean is that that section should describe what you think would be the symptoms
or areas where regressions would show up. So beside of the verification testing
by people affected, someone could say ok, we were covering this area and had no
issues so this might be ok.

Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH v3 0/4] Fix corruption on blocked_vcpu_on_cpu list

Kamal Mostafa-2
In reply to this post by Ioanna Alifieraki
LGTM.

Acked-by: Kamal Mostafa <[hidden email]>

 -Kamal

On Mon, Jan 18, 2021 at 11:22:04PM +0000, Ioanna Alifieraki wrote:

> BugLink: https://bugs.launchpad.net/bugs/1908428
>
> The following patches fix the bug reported in [1].
> The first patch bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block)
> is pulled into to facilitate the backport of the next 3 patches that fix the bug.
> Original patches come from [2] and have been accepted upstream.
>
> [1] https://marc.info/?l=kvm&m=149559827906211&w=2
> [2] https://lore.kernel.org/lkml/20170606105707.23207-1-pbonzini@.../
>
> Paolo Bonzini (3):
>   KVM: VMX: extract __pi_post_block
>   KVM: VMX: avoid double list add with VT-d posted interrupts
>   KVM: VMX: simplify and fix vmx_vcpu_pi_load
>
> Yunhong Jiang (1):
>   kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block
>
>  arch/x86/kvm/vmx.c | 190 ++++++++++++++++++++++++++++-------------------------
>  1 file changed, 100 insertions(+), 90 deletions(-)
>
> --
> 1.9.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU][Xenial][PATCH v3 0/4] Fix corruption on blocked_vcpu_on_cpu list

Kelsey Skunberg
In reply to this post by Ioanna Alifieraki
Applied to Xenial/master-next. Thank you for fixing this up and
submitting Ioanna. :)

-Kelsey

On 2021-01-18 23:22:04 , Ioanna Alifieraki wrote:

> BugLink: https://bugs.launchpad.net/bugs/1908428
>
> The following patches fix the bug reported in [1].
> The first patch bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block)
> is pulled into to facilitate the backport of the next 3 patches that fix the bug.
> Original patches come from [2] and have been accepted upstream.
>
> [1] https://marc.info/?l=kvm&m=149559827906211&w=2
> [2] https://lore.kernel.org/lkml/20170606105707.23207-1-pbonzini@.../
>
> Paolo Bonzini (3):
>   KVM: VMX: extract __pi_post_block
>   KVM: VMX: avoid double list add with VT-d posted interrupts
>   KVM: VMX: simplify and fix vmx_vcpu_pi_load
>
> Yunhong Jiang (1):
>   kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block
>
>  arch/x86/kvm/vmx.c | 190 ++++++++++++++++++++++++++++-------------------------
>  1 file changed, 100 insertions(+), 90 deletions(-)
>
> --
> 1.9.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team