| Message ID | 20210105235500.20681-1-ioanna-maria.alifieraki@canonical.com |
|---|---|
| Headers | show |
| Series | Fix corruption on blocked_vcpu_on_cpu list | expand |
On Tue, Jan 05, 2021 at 11:54:57PM +0000, Ioanna Alifieraki wrote: > BugLink: https://bugs.launchpad.net/bugs/1908428 > > The following 3 patches fix the bug reported in [1]. > They are backported to apply on 4.4 kernels. > Backport is required becasue upstream commit > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > which renames vmx_pre/post_block functions to pi_pre/post_block > is missing from 4.4. > 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@redhat.com/ > > 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 > > arch/x86/kvm/vmx.c | 177 ++++++++++++++++++++++----------------------- > 1 file changed, 87 insertions(+), 90 deletions(-) > > -- > 2.17.1 Upstream commit bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) looks pretty simple. If you pick that up first, then PATCH 1 and PATCH 2 can be cherry-picks right? William Breathitt Gray
Hi William, Thanks for the feedback. Even with commit bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) the patches still won't cherry-pick because upstream commit a0052191624e(kvm: vmx: check apicv is active before using VT-d posted interrupt) is missing, however the backport is cleaner and much simpler. I'm happy to resubmit the patchset with commit bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) included if it is a better option. On Thu, Jan 7, 2021 at 8:36 AM William Breathitt Gray <william.gray@canonical.com> wrote: > > On Tue, Jan 05, 2021 at 11:54:57PM +0000, Ioanna Alifieraki wrote: > > BugLink: https://bugs.launchpad.net/bugs/1908428 > > > > The following 3 patches fix the bug reported in [1]. > > They are backported to apply on 4.4 kernels. > > Backport is required becasue upstream commit > > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > > which renames vmx_pre/post_block functions to pi_pre/post_block > > is missing from 4.4. > > 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@redhat.com/ > > > > 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 > > > > arch/x86/kvm/vmx.c | 177 ++++++++++++++++++++++----------------------- > > 1 file changed, 87 insertions(+), 90 deletions(-) > > > > -- > > 2.17.1 > > Upstream commit > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > looks pretty simple. If you pick that up first, then PATCH 1 and PATCH 2 > can be cherry-picks right? > > William Breathitt Gray
On Thu, Jan 14, 2021 at 03:21:41PM +0000, Ioanna Alifieraki wrote: > Hi William, > > Thanks for the feedback. Even with commit bc22512bb24c(kvm: vmx: > rename vmx_pre/post_block to pi_pre/post_block) > the patches still won't cherry-pick because upstream commit > a0052191624e(kvm: vmx: check apicv is active before using VT-d posted > interrupt) is missing, however the backport > is cleaner and much simpler. Okay, I see what you mean, this would have more dependencies than just commit bc22512bb24c. Well if the backport is a better solution here, then let's go with it. Thanks, Acked-by: William Breathitt Gray <william.gray@canonical.com> > I'm happy to resubmit the patchset with commit bc22512bb24c(kvm: vmx: > rename vmx_pre/post_block to pi_pre/post_block) > included if it is a better option. > > On Thu, Jan 7, 2021 at 8:36 AM William Breathitt Gray > <william.gray@canonical.com> wrote: > > > > On Tue, Jan 05, 2021 at 11:54:57PM +0000, Ioanna Alifieraki wrote: > > > BugLink: https://bugs.launchpad.net/bugs/1908428 > > > > > > The following 3 patches fix the bug reported in [1]. > > > They are backported to apply on 4.4 kernels. > > > Backport is required becasue upstream commit > > > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > > > which renames vmx_pre/post_block functions to pi_pre/post_block > > > is missing from 4.4. > > > 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@redhat.com/ > > > > > > 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 > > > > > > arch/x86/kvm/vmx.c | 177 ++++++++++++++++++++++----------------------- > > > 1 file changed, 87 insertions(+), 90 deletions(-) > > > > > > -- > > > 2.17.1 > > > > Upstream commit > > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > > looks pretty simple. If you pick that up first, then PATCH 1 and PATCH 2 > > can be cherry-picks right? > > > > William Breathitt Gray
Ok, I didn't express myself clearly earlier. What I mean is that if we pull in commit bc22512bb24c, the other patches still need backporting because of commit a0052191624e, but in this case with commit bc22512bb24c in place the backport of the other commits will be cleaner and simpler. Commit bc22512bb24c does not depend on commit a0052191624e. My apologies for the confusion. On Thu, Jan 14, 2021 at 11:11 PM William Breathitt Gray <william.gray@canonical.com> wrote: > > On Thu, Jan 14, 2021 at 03:21:41PM +0000, Ioanna Alifieraki wrote: > > Hi William, > > > > Thanks for the feedback. Even with commit bc22512bb24c(kvm: vmx: > > rename vmx_pre/post_block to pi_pre/post_block) > > the patches still won't cherry-pick because upstream commit > > a0052191624e(kvm: vmx: check apicv is active before using VT-d posted > > interrupt) is missing, however the backport > > is cleaner and much simpler. > > Okay, I see what you mean, this would have more dependencies than just > commit bc22512bb24c. Well if the backport is a better solution here, > then let's go with it. > > Thanks, > > Acked-by: William Breathitt Gray <william.gray@canonical.com> > > > I'm happy to resubmit the patchset with commit bc22512bb24c(kvm: vmx: > > rename vmx_pre/post_block to pi_pre/post_block) > > included if it is a better option. > > > > On Thu, Jan 7, 2021 at 8:36 AM William Breathitt Gray > > <william.gray@canonical.com> wrote: > > > > > > On Tue, Jan 05, 2021 at 11:54:57PM +0000, Ioanna Alifieraki wrote: > > > > BugLink: https://bugs.launchpad.net/bugs/1908428 > > > > > > > > The following 3 patches fix the bug reported in [1]. > > > > They are backported to apply on 4.4 kernels. > > > > Backport is required becasue upstream commit > > > > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > > > > which renames vmx_pre/post_block functions to pi_pre/post_block > > > > is missing from 4.4. > > > > 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@redhat.com/ > > > > > > > > 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 > > > > > > > > arch/x86/kvm/vmx.c | 177 ++++++++++++++++++++++----------------------- > > > > 1 file changed, 87 insertions(+), 90 deletions(-) > > > > > > > > -- > > > > 2.17.1 > > > > > > Upstream commit > > > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > > > looks pretty simple. If you pick that up first, then PATCH 1 and PATCH 2 > > > can be cherry-picks right? > > > > > > William Breathitt Gray
On Thu, Jan 14, 2021 at 11:31:34PM +0000, Ioanna Alifieraki wrote: > Ok, I didn't express myself clearly earlier. > What I mean is that if we pull in commit bc22512bb24c, the other > patches still need backporting > because of commit a0052191624e, but in this case with commit > bc22512bb24c in place the backport of the other > commits will be cleaner and simpler. > Commit bc22512bb24c does not depend on commit a0052191624e. > My apologies for the confusion. Ah, I understand now. In that case, resubmit a version 3 of this with commit bc22512bb24c. I think that will keep the changes in this patchset cleaner for the future. If you think commit a0052191624e will help as well, then you can pick that up too (it looks like a simple and useful bug fix). (A side note: when you reply on the mailing list, please reply underneath the message; replying underneath makes the discussion easier to follow for other readers.) Thanks, William Breathitt Gray > > On Thu, Jan 14, 2021 at 11:11 PM William Breathitt Gray > <william.gray@canonical.com> wrote: > > > > On Thu, Jan 14, 2021 at 03:21:41PM +0000, Ioanna Alifieraki wrote: > > > Hi William, > > > > > > Thanks for the feedback. Even with commit bc22512bb24c(kvm: vmx: > > > rename vmx_pre/post_block to pi_pre/post_block) > > > the patches still won't cherry-pick because upstream commit > > > a0052191624e(kvm: vmx: check apicv is active before using VT-d posted > > > interrupt) is missing, however the backport > > > is cleaner and much simpler. > > > > Okay, I see what you mean, this would have more dependencies than just > > commit bc22512bb24c. Well if the backport is a better solution here, > > then let's go with it. > > > > Thanks, > > > > Acked-by: William Breathitt Gray <william.gray@canonical.com> > > > > > I'm happy to resubmit the patchset with commit bc22512bb24c(kvm: vmx: > > > rename vmx_pre/post_block to pi_pre/post_block) > > > included if it is a better option. > > > > > > On Thu, Jan 7, 2021 at 8:36 AM William Breathitt Gray > > > <william.gray@canonical.com> wrote: > > > > > > > > On Tue, Jan 05, 2021 at 11:54:57PM +0000, Ioanna Alifieraki wrote: > > > > > BugLink: https://bugs.launchpad.net/bugs/1908428 > > > > > > > > > > The following 3 patches fix the bug reported in [1]. > > > > > They are backported to apply on 4.4 kernels. > > > > > Backport is required becasue upstream commit > > > > > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > > > > > which renames vmx_pre/post_block functions to pi_pre/post_block > > > > > is missing from 4.4. > > > > > 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@redhat.com/ > > > > > > > > > > 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 > > > > > > > > > > arch/x86/kvm/vmx.c | 177 ++++++++++++++++++++++----------------------- > > > > > 1 file changed, 87 insertions(+), 90 deletions(-) > > > > > > > > > > -- > > > > > 2.17.1 > > > > > > > > Upstream commit > > > > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > > > > looks pretty simple. If you pick that up first, then PATCH 1 and PATCH 2 > > > > can be cherry-picks right? > > > > > > > > William Breathitt Gray
On 06.01.21 00:54, Ioanna Alifieraki wrote: > BugLink: https://bugs.launchpad.net/bugs/1908428 > > The following 3 patches fix the bug reported in [1]. > They are backported to apply on 4.4 kernels. > Backport is required becasue upstream commit > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > which renames vmx_pre/post_block functions to pi_pre/post_block > is missing from 4.4. > 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@redhat.com/ > > 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 > > arch/x86/kvm/vmx.c | 177 ++++++++++++++++++++++----------------------- > 1 file changed, 87 insertions(+), 90 deletions(-) > The first patch appears like it can be safely ignored from a change point of view as it effectively just moves a code section into its own function. The other 2 are rather obscure in what is getting changed. What I mean is that its hard to tell what this code does anyway. So I would rely more on the successful testing. On the regression potential: if you have more insights on how a regression might show up, that should be rather mentioned. Very vaguely this changes around vCPU hotplug(?), so in that case regression potential would be issues in other cases of vCPU addition/removal... Acked-by: Stefan Bader <stefan.bader@canonical.com>
On Wed, Jan 20, 2021 at 8:51 AM Stefan Bader <stefan.bader@canonical.com> wrote: > > On 06.01.21 00:54, Ioanna Alifieraki wrote: > > BugLink: https://bugs.launchpad.net/bugs/1908428 > > > > The following 3 patches fix the bug reported in [1]. > > They are backported to apply on 4.4 kernels. > > Backport is required becasue upstream commit > > bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) > > which renames vmx_pre/post_block functions to pi_pre/post_block > > is missing from 4.4. > > 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@redhat.com/ > > > > 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 > > > > arch/x86/kvm/vmx.c | 177 ++++++++++++++++++++++----------------------- > > 1 file changed, 87 insertions(+), 90 deletions(-) > > > > The first patch appears like it can be safely ignored from a change point of > view as it effectively just moves a code section into its own function. > The other 2 are rather obscure in what is getting changed. What I mean is that > its hard to tell what this code does anyway. So I would rely more on the > successful testing. On the regression potential: if you have more insights on > how a regression might show up, that should be rather mentioned. Very vaguely > this changes around vCPU hotplug(?), so in that case regression potential would > be issues in other cases of vCPU addition/removal... > > Acked-by: Stefan Bader <stefan.bader@canonical.com> > Thanks for the feedback. I cannot reproduce the bug myself. The user that reported the bug has tested and so far they don't have mentioned any regression. FYI, after exchanging a couple of emails with William I sent a version 3 of the patches on Jan 18 with subject : [SRU][Xenial][PATCH v3 0/4] Fix corruption on blocked_vcpu_on_cpu list I don't know how you handle these cases in the kernel team. Both v2 and v3 are functionally the same, in v3 the backport is more straightforward because I pull in an extra commit.
On 20.01.21 11:52, Ioanna Alifieraki wrote: > On Wed, Jan 20, 2021 at 8:51 AM Stefan Bader <stefan.bader@canonical.com> wrote: >> >> On 06.01.21 00:54, Ioanna Alifieraki wrote: >>> BugLink: https://bugs.launchpad.net/bugs/1908428 >>> >>> The following 3 patches fix the bug reported in [1]. >>> They are backported to apply on 4.4 kernels. >>> Backport is required becasue upstream commit >>> bc22512bb24c(kvm: vmx: rename vmx_pre/post_block to pi_pre/post_block) >>> which renames vmx_pre/post_block functions to pi_pre/post_block >>> is missing from 4.4. >>> 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@redhat.com/ >>> >>> 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 >>> >>> arch/x86/kvm/vmx.c | 177 ++++++++++++++++++++++----------------------- >>> 1 file changed, 87 insertions(+), 90 deletions(-) >>> >> >> The first patch appears like it can be safely ignored from a change point of >> view as it effectively just moves a code section into its own function. >> The other 2 are rather obscure in what is getting changed. What I mean is that >> its hard to tell what this code does anyway. So I would rely more on the >> successful testing. On the regression potential: if you have more insights on >> how a regression might show up, that should be rather mentioned. Very vaguely >> this changes around vCPU hotplug(?), so in that case regression potential would >> be issues in other cases of vCPU addition/removal... >> >> Acked-by: Stefan Bader <stefan.bader@canonical.com> >> > > Thanks for the feedback. I cannot reproduce the bug myself. The user > that reported the bug > has tested and so far they don't have mentioned any regression. > FYI, after exchanging a couple of emails with William I sent a version > 3 of the patches on Jan 18 > with subject : > [SRU][Xenial][PATCH v3 0/4] Fix corruption on blocked_vcpu_on_cpu list > > I don't know how you handle these cases in the kernel team. Both v2 > and v3 are functionally > the same, in v3 the backport is more straightforward because I pull in > an extra commit. > It is not only me but sometimes also me. But generally to help us, please if there is a v{x}+1 make sure that all previous threads have a final NACK reply. Otherwise you get people, like me, looking into the wrong version, realizing the next morning that they wasted time, getting grumpy and confused. And that won't help either side. ;) -Stefan