| Message ID | 20171212120204.6799-1-clg@kaod.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | KVM: PPC: Book3S: fix XIVE migration of pending interrupts | expand |
On 12/12/2017 13:02, Cédric Le Goater wrote: > When restoring a pending interrupt, we are setting the Q bit to force > a retrigger in xive_finish_unmask(). But we also need to force an EOI > in this case to reach the same initial state : P=1, Q=0. > > This can be done by not setting 'old_p' for pending interrupts which > will inform xive_finish_unmask() that an EOI needs to be sent. > > Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > > Tested with a guest running iozone. > > arch/powerpc/kvm/book3s_xive.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) We really need this patch to fix VM migration on POWER9. When will it be merged? Thanks, Laurent -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Laurent Vivier <lvivier@redhat.com> writes: > On 12/12/2017 13:02, Cédric Le Goater wrote: >> When restoring a pending interrupt, we are setting the Q bit to force >> a retrigger in xive_finish_unmask(). But we also need to force an EOI >> in this case to reach the same initial state : P=1, Q=0. >> >> This can be done by not setting 'old_p' for pending interrupts which >> will inform xive_finish_unmask() that an EOI needs to be sent. >> >> Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> >> Tested with a guest running iozone. >> >> arch/powerpc/kvm/book3s_xive.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) > > We really need this patch to fix VM migration on POWER9. > When will it be merged? Paul is away, so I'll merge it via the powerpc tree. I'll mark it: Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller") Cc: stable@vger.kernel.org # v4.12+ cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 22, 2017 at 03:34:20PM +1100, Michael Ellerman wrote: > Laurent Vivier <lvivier@redhat.com> writes: > > > On 12/12/2017 13:02, Cédric Le Goater wrote: > >> When restoring a pending interrupt, we are setting the Q bit to force > >> a retrigger in xive_finish_unmask(). But we also need to force an EOI > >> in this case to reach the same initial state : P=1, Q=0. > >> > >> This can be done by not setting 'old_p' for pending interrupts which > >> will inform xive_finish_unmask() that an EOI needs to be sent. > >> > >> Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >> --- > >> > >> Tested with a guest running iozone. > >> > >> arch/powerpc/kvm/book3s_xive.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > > > > We really need this patch to fix VM migration on POWER9. > > When will it be merged? > > Paul is away, so I'll merge it via the powerpc tree. > > I'll mark it: > > Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller") > Cc: stable@vger.kernel.org # v4.12+ Thanks for doing that. If you felt like merging Alexey's patch "KVM: PPC: Book3S PR: Fix WIMG handling under pHyp" with my acked-by, that would be fine too. The commit message needs a little work - the reason for using HPTE_R_M is not just because it seems to work, but because current POWER processors require M set on mappings for normal pages, and pHyp enforces that. Cheers, Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22/12/2017 08:54, Paul Mackerras wrote: > On Fri, Dec 22, 2017 at 03:34:20PM +1100, Michael Ellerman wrote: >> Laurent Vivier <lvivier@redhat.com> writes: >> >>> On 12/12/2017 13:02, Cédric Le Goater wrote: >>>> When restoring a pending interrupt, we are setting the Q bit to force >>>> a retrigger in xive_finish_unmask(). But we also need to force an EOI >>>> in this case to reach the same initial state : P=1, Q=0. >>>> >>>> This can be done by not setting 'old_p' for pending interrupts which >>>> will inform xive_finish_unmask() that an EOI needs to be sent. >>>> >>>> Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> --- >>>> >>>> Tested with a guest running iozone. >>>> >>>> arch/powerpc/kvm/book3s_xive.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> We really need this patch to fix VM migration on POWER9. >>> When will it be merged? >> >> Paul is away, so I'll merge it via the powerpc tree. >> >> I'll mark it: >> >> Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller") >> Cc: stable@vger.kernel.org # v4.12+ > > Thanks for doing that. > > If you felt like merging Alexey's patch "KVM: PPC: Book3S PR: Fix WIMG > handling under pHyp" with my acked-by, that would be fine too. The > commit message needs a little work - the reason for using HPTE_R_M is > not just because it seems to work, but because current POWER > processors require M set on mappings for normal pages, and pHyp > enforces that. We also need: KVM: PPC: Book3S HV: Fix pending_pri value in kvmppc_xive_get_icp() Thanks, Laurent -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paul Mackerras <paulus@ozlabs.org> writes: > On Fri, Dec 22, 2017 at 03:34:20PM +1100, Michael Ellerman wrote: >> Laurent Vivier <lvivier@redhat.com> writes: >> >> > On 12/12/2017 13:02, Cédric Le Goater wrote: >> >> When restoring a pending interrupt, we are setting the Q bit to force >> >> a retrigger in xive_finish_unmask(). But we also need to force an EOI >> >> in this case to reach the same initial state : P=1, Q=0. >> >> >> >> This can be done by not setting 'old_p' for pending interrupts which >> >> will inform xive_finish_unmask() that an EOI needs to be sent. >> >> >> >> Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> >> --- >> >> >> >> Tested with a guest running iozone. >> >> >> >> arch/powerpc/kvm/book3s_xive.c | 4 ++-- >> >> 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > We really need this patch to fix VM migration on POWER9. >> > When will it be merged? >> >> Paul is away, so I'll merge it via the powerpc tree. >> >> I'll mark it: >> >> Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller") >> Cc: stable@vger.kernel.org # v4.12+ > > Thanks for doing that. > > If you felt like merging Alexey's patch "KVM: PPC: Book3S PR: Fix WIMG > handling under pHyp" with my acked-by, that would be fine too. The > commit message needs a little work - the reason for using HPTE_R_M is > not just because it seems to work, but because current POWER > processors require M set on mappings for normal pages, and pHyp > enforces that. OK. I saw this too late, but I'll pick that one up next week. If someone sends me an updated change log I will merge all of their patches for ever. cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Laurent Vivier <lvivier@redhat.com> writes: > On 22/12/2017 08:54, Paul Mackerras wrote: >> On Fri, Dec 22, 2017 at 03:34:20PM +1100, Michael Ellerman wrote: >>> Laurent Vivier <lvivier@redhat.com> writes: >>> >>>> On 12/12/2017 13:02, Cédric Le Goater wrote: >>>>> When restoring a pending interrupt, we are setting the Q bit to force >>>>> a retrigger in xive_finish_unmask(). But we also need to force an EOI >>>>> in this case to reach the same initial state : P=1, Q=0. >>>>> >>>>> This can be done by not setting 'old_p' for pending interrupts which >>>>> will inform xive_finish_unmask() that an EOI needs to be sent. >>>>> >>>>> Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>>> --- >>>>> >>>>> Tested with a guest running iozone. >>>>> >>>>> arch/powerpc/kvm/book3s_xive.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> We really need this patch to fix VM migration on POWER9. >>>> When will it be merged? >>> >>> Paul is away, so I'll merge it via the powerpc tree. >>> >>> I'll mark it: >>> >>> Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller") >>> Cc: stable@vger.kernel.org # v4.12+ >> >> Thanks for doing that. >> >> If you felt like merging Alexey's patch "KVM: PPC: Book3S PR: Fix WIMG >> handling under pHyp" with my acked-by, that would be fine too. The >> commit message needs a little work - the reason for using HPTE_R_M is >> not just because it seems to work, but because current POWER >> processors require M set on mappings for normal pages, and pHyp >> enforces that. > > We also need: > > KVM: PPC: Book3S HV: Fix pending_pri value in kvmppc_xive_get_icp() I've merged that one. cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2017-12-12 at 12:02:04 UTC, =?utf-8?q?C=C3=A9dric_Le_Goater?= wrote: > When restoring a pending interrupt, we are setting the Q bit to force > a retrigger in xive_finish_unmask(). But we also need to force an EOI > in this case to reach the same initial state : P=1, Q=0. > > This can be done by not setting 'old_p' for pending interrupts which > will inform xive_finish_unmask() that an EOI needs to be sent. > > Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > Reviewed-by: Laurent Vivier <lvivier@redhat.com> > Tested-by: Laurent Vivier <lvivier@redhat.com> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/dc1c4165d189350cb51bdd3057deb6 cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 22 Dec 2017 22:22:08 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote: > Paul Mackerras <paulus@ozlabs.org> writes: > > > On Fri, Dec 22, 2017 at 03:34:20PM +1100, Michael Ellerman wrote: > >> Laurent Vivier <lvivier@redhat.com> writes: > >> > >> > On 12/12/2017 13:02, Cédric Le Goater wrote: > >> >> When restoring a pending interrupt, we are setting the Q bit to force > >> >> a retrigger in xive_finish_unmask(). But we also need to force an EOI > >> >> in this case to reach the same initial state : P=1, Q=0. > >> >> > >> >> This can be done by not setting 'old_p' for pending interrupts which > >> >> will inform xive_finish_unmask() that an EOI needs to be sent. > >> >> > >> >> Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >> >> --- > >> >> > >> >> Tested with a guest running iozone. > >> >> > >> >> arch/powerpc/kvm/book3s_xive.c | 4 ++-- > >> >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > > >> > We really need this patch to fix VM migration on POWER9. > >> > When will it be merged? > >> > >> Paul is away, so I'll merge it via the powerpc tree. > >> > >> I'll mark it: > >> > >> Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller") > >> Cc: stable@vger.kernel.org # v4.12+ > > > > Thanks for doing that. > > > > If you felt like merging Alexey's patch "KVM: PPC: Book3S PR: Fix WIMG > > handling under pHyp" with my acked-by, that would be fine too. The > > commit message needs a little work - the reason for using HPTE_R_M is > > not just because it seems to work, but because current POWER > > processors require M set on mappings for normal pages, and pHyp > > enforces that. > > OK. I saw this too late, but I'll pick that one up next week. If someone > sends me an updated change log I will merge all of their patches for > ever. > Really ? Opportunity makes the thief, so here's my take :P 8<---------------------------------------------------------------------->8 KVM: PPC: Book3S: fix XIVE migration of pending interrupts 96df226 "KVM: PPC: Book3S PR: Preserve storage control bits" added WIMG bits preserving but it missed 2 special cases: - a magic page in kvmppc_mmu_book3s_64_xlate() and - guest real mode in kvmppc_handle_pagefault(). For these ptes WIMG were 0 and pHyp failed on these causing a guest to stop in the very beginning at NIP=0x100 (due to bd9166ffe "KVM: PPC: Book3S PR: Exit KVM on failed mapping"). According to LoPAPR v1.1 14.5.4.1.2 H_ENTER: The hypervisor checks that the WIMG bits within the PTE are appropriate for the physical page number else H_Parameter return. (For System Memory pages WIMG=0010, or, 1110 if the SAO option is enabled, and for IO pages WIMG=01**.) This hence initializes WIMG to non-zero value HPTE_R_M (0x10), as expected by pHyp. Fixes: 96df226 "KVM: PPC: Book3S PR: Preserve storage control bits" Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> 8<---------------------------------------------------------------------->8 Cheers, -- Greg > cheers > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 22 Dec 2017 12:58:47 +0100 Greg Kurz <groug@kaod.org> wrote: > On Fri, 22 Dec 2017 22:22:08 +1100 > Michael Ellerman <mpe@ellerman.id.au> wrote: > > > Paul Mackerras <paulus@ozlabs.org> writes: [...] > > > > > > Thanks for doing that. > > > > > > If you felt like merging Alexey's patch "KVM: PPC: Book3S PR: Fix WIMG > > > handling under pHyp" with my acked-by, that would be fine too. The > > > commit message needs a little work - the reason for using HPTE_R_M is > > > not just because it seems to work, but because current POWER > > > processors require M set on mappings for normal pages, and pHyp > > > enforces that. > > > > OK. I saw this too late, but I'll pick that one up next week. If someone > > sends me an updated change log I will merge all of their patches for > > ever. > > > > Really ? Opportunity makes the thief, so here's my take :P > > 8<---------------------------------------------------------------------->8 > KVM: PPC: Book3S: fix XIVE migration of pending interrupts Oops! Paste error... Title should be: KVM: PPC: Book3S PR: Fix WIMG handling under pHyp > > 96df226 "KVM: PPC: Book3S PR: Preserve storage control bits" added WIMG > bits preserving but it missed 2 special cases: > - a magic page in kvmppc_mmu_book3s_64_xlate() and > - guest real mode in kvmppc_handle_pagefault(). > > For these ptes WIMG were 0 and pHyp failed on these causing a guest to > stop in the very beginning at NIP=0x100 (due to bd9166ffe > "KVM: PPC: Book3S PR: Exit KVM on failed mapping"). > > According to LoPAPR v1.1 14.5.4.1.2 H_ENTER: > > The hypervisor checks that the WIMG bits within the PTE are appropriate > for the physical page number else H_Parameter return. (For System Memory > pages WIMG=0010, or, 1110 if the SAO option is enabled, and for IO pages > WIMG=01**.) > > This hence initializes WIMG to non-zero value HPTE_R_M (0x10), as expected > by pHyp. > > Fixes: 96df226 "KVM: PPC: Book3S PR: Preserve storage control bits" > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > 8<---------------------------------------------------------------------->8 > > Cheers, > > -- > Greg > > > cheers > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c index bf457843e032..b5e6d227a034 100644 --- a/arch/powerpc/kvm/book3s_xive.c +++ b/arch/powerpc/kvm/book3s_xive.c @@ -1558,7 +1558,7 @@ static int xive_set_source(struct kvmppc_xive *xive, long irq, u64 addr) /* * Restore P and Q. If the interrupt was pending, we - * force both P and Q, which will trigger a resend. + * force Q and !P, which will trigger a resend. * * That means that a guest that had both an interrupt * pending (queued) and Q set will restore with only @@ -1566,7 +1566,7 @@ static int xive_set_source(struct kvmppc_xive *xive, long irq, u64 addr) * is perfectly fine as coalescing interrupts that haven't * been presented yet is always allowed. */ - if (val & KVM_XICS_PRESENTED || val & KVM_XICS_PENDING) + if (val & KVM_XICS_PRESENTED && !(val & KVM_XICS_PENDING)) state->old_p = true; if (val & KVM_XICS_QUEUED || val & KVM_XICS_PENDING) state->old_q = true;
When restoring a pending interrupt, we are setting the Q bit to force a retrigger in xive_finish_unmask(). But we also need to force an EOI in this case to reach the same initial state : P=1, Q=0. This can be done by not setting 'old_p' for pending interrupts which will inform xive_finish_unmask() that an EOI needs to be sent. Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> --- Tested with a guest running iozone. arch/powerpc/kvm/book3s_xive.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)