Message ID | 20180213084712.18351-1-clg@kaod.org (mailing list archive) |
---|---|
State | Accepted |
Commit | 8e036c8d30a2cd9d8fc7442fbf6824e0a3e986e7 |
Headers | show |
Series | powerpc/xive: use hw CPU ids when configuring the CPU queues | expand |
Cédric Le Goater <clg@kaod.org> writes: > The CPU event notification queues on sPAPR should be configured using > a hardware CPU identifier. > > The problem did not show up on the Power Hypervisor because pHyp > supports 8 threads per core which keeps CPU number contiguous. This is > not the case on all sPAPR virtual machines, some use SMT=1. > > Also improve error logging by adding the CPU number. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > > I think we should send this one to stable also. Fixes: eac1e731b59e ("powerpc/xive: guest exploitation of the XIVE interrupt controller") Cc: stable@vger.kernel.org # v4.14+ ? cheers > diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c > index d9c4c9366049..091f1d0d0af1 100644 > --- a/arch/powerpc/sysdev/xive/spapr.c > +++ b/arch/powerpc/sysdev/xive/spapr.c > @@ -356,7 +356,8 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio, > > rc = plpar_int_get_queue_info(0, target, prio, &esn_page, &esn_size); > if (rc) { > - pr_err("Error %lld getting queue info prio %d\n", rc, prio); > + pr_err("Error %lld getting queue info CPU %d prio %d\n", rc, > + target, prio); > rc = -EIO; > goto fail; > } > @@ -370,7 +371,8 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio, > /* Configure and enable the queue in HW */ > rc = plpar_int_set_queue_config(flags, target, prio, qpage_phys, order); > if (rc) { > - pr_err("Error %lld setting queue for prio %d\n", rc, prio); > + pr_err("Error %lld setting queue for CPU %d prio %d\n", rc, > + target, prio); > rc = -EIO; > } else { > q->qpage = qpage; > @@ -389,8 +391,8 @@ static int xive_spapr_setup_queue(unsigned int cpu, struct xive_cpu *xc, > if (IS_ERR(qpage)) > return PTR_ERR(qpage); > > - return xive_spapr_configure_queue(cpu, q, prio, qpage, > - xive_queue_shift); > + return xive_spapr_configure_queue(get_hard_smp_processor_id(cpu), > + q, prio, qpage, xive_queue_shift); > } > > static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc, > @@ -399,10 +401,12 @@ static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc, > struct xive_q *q = &xc->queue[prio]; > unsigned int alloc_order; > long rc; > + int hw_cpu = get_hard_smp_processor_id(cpu); > > - rc = plpar_int_set_queue_config(0, cpu, prio, 0, 0); > + rc = plpar_int_set_queue_config(0, hw_cpu, prio, 0, 0); > if (rc) > - pr_err("Error %ld setting queue for prio %d\n", rc, prio); > + pr_err("Error %ld setting queue for CPU %d prio %d\n", rc, > + hw_cpu, prio); > > alloc_order = xive_alloc_order(xive_queue_shift); > free_pages((unsigned long)q->qpage, alloc_order); > -- > 2.13.6
On 02/13/2018 10:18 AM, Michael Ellerman wrote: > Cédric Le Goater <clg@kaod.org> writes: > >> The CPU event notification queues on sPAPR should be configured using >> a hardware CPU identifier. >> >> The problem did not show up on the Power Hypervisor because pHyp >> supports 8 threads per core which keeps CPU number contiguous. This is >> not the case on all sPAPR virtual machines, some use SMT=1. >> >> Also improve error logging by adding the CPU number. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> >> I think we should send this one to stable also. > > Fixes: eac1e731b59e ("powerpc/xive: guest exploitation of the XIVE interrupt controller") yes. > Cc: stable@vger.kernel.org # v4.14+ yes. I just added the Cc:. I am not sure that will work with patchwork though. Thanks, C. > ? > > cheers > >> diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c >> index d9c4c9366049..091f1d0d0af1 100644 >> --- a/arch/powerpc/sysdev/xive/spapr.c >> +++ b/arch/powerpc/sysdev/xive/spapr.c >> @@ -356,7 +356,8 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio, >> >> rc = plpar_int_get_queue_info(0, target, prio, &esn_page, &esn_size); >> if (rc) { >> - pr_err("Error %lld getting queue info prio %d\n", rc, prio); >> + pr_err("Error %lld getting queue info CPU %d prio %d\n", rc, >> + target, prio); >> rc = -EIO; >> goto fail; >> } >> @@ -370,7 +371,8 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio, >> /* Configure and enable the queue in HW */ >> rc = plpar_int_set_queue_config(flags, target, prio, qpage_phys, order); >> if (rc) { >> - pr_err("Error %lld setting queue for prio %d\n", rc, prio); >> + pr_err("Error %lld setting queue for CPU %d prio %d\n", rc, >> + target, prio); >> rc = -EIO; >> } else { >> q->qpage = qpage; >> @@ -389,8 +391,8 @@ static int xive_spapr_setup_queue(unsigned int cpu, struct xive_cpu *xc, >> if (IS_ERR(qpage)) >> return PTR_ERR(qpage); >> >> - return xive_spapr_configure_queue(cpu, q, prio, qpage, >> - xive_queue_shift); >> + return xive_spapr_configure_queue(get_hard_smp_processor_id(cpu), >> + q, prio, qpage, xive_queue_shift); >> } >> >> static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc, >> @@ -399,10 +401,12 @@ static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc, >> struct xive_q *q = &xc->queue[prio]; >> unsigned int alloc_order; >> long rc; >> + int hw_cpu = get_hard_smp_processor_id(cpu); >> >> - rc = plpar_int_set_queue_config(0, cpu, prio, 0, 0); >> + rc = plpar_int_set_queue_config(0, hw_cpu, prio, 0, 0); >> if (rc) >> - pr_err("Error %ld setting queue for prio %d\n", rc, prio); >> + pr_err("Error %ld setting queue for CPU %d prio %d\n", rc, >> + hw_cpu, prio); >> >> alloc_order = xive_alloc_order(xive_queue_shift); >> free_pages((unsigned long)q->qpage, alloc_order); >> -- >> 2.13.6
Cédric Le Goater <clg@kaod.org> writes: > On 02/13/2018 10:18 AM, Michael Ellerman wrote: >> Cédric Le Goater <clg@kaod.org> writes: >> >>> The CPU event notification queues on sPAPR should be configured using >>> a hardware CPU identifier. >>> >>> The problem did not show up on the Power Hypervisor because pHyp >>> supports 8 threads per core which keeps CPU number contiguous. This is >>> not the case on all sPAPR virtual machines, some use SMT=1. >>> >>> Also improve error logging by adding the CPU number. >>> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> --- >>> >>> I think we should send this one to stable also. >> >> Fixes: eac1e731b59e ("powerpc/xive: guest exploitation of the XIVE interrupt controller") > > yes. > >> Cc: stable@vger.kernel.org # v4.14+ > > yes. I just added the Cc:. I am not sure that will work with > patchwork though. They don't accept patches that way. I'll add the tags and commit it. cheers
On Tue, 2018-02-13 at 08:47:12 UTC, =?utf-8?q?C=C3=A9dric_Le_Goater?= wrote: > The CPU event notification queues on sPAPR should be configured using > a hardware CPU identifier. > > The problem did not show up on the Power Hypervisor because pHyp > supports 8 threads per core which keeps CPU number contiguous. This is > not the case on all sPAPR virtual machines, some use SMT=1. > > Also improve error logging by adding the CPU number. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/8e036c8d30a2cd9d8fc7442fbf6824 cheers
diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c index d9c4c9366049..091f1d0d0af1 100644 --- a/arch/powerpc/sysdev/xive/spapr.c +++ b/arch/powerpc/sysdev/xive/spapr.c @@ -356,7 +356,8 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio, rc = plpar_int_get_queue_info(0, target, prio, &esn_page, &esn_size); if (rc) { - pr_err("Error %lld getting queue info prio %d\n", rc, prio); + pr_err("Error %lld getting queue info CPU %d prio %d\n", rc, + target, prio); rc = -EIO; goto fail; } @@ -370,7 +371,8 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio, /* Configure and enable the queue in HW */ rc = plpar_int_set_queue_config(flags, target, prio, qpage_phys, order); if (rc) { - pr_err("Error %lld setting queue for prio %d\n", rc, prio); + pr_err("Error %lld setting queue for CPU %d prio %d\n", rc, + target, prio); rc = -EIO; } else { q->qpage = qpage; @@ -389,8 +391,8 @@ static int xive_spapr_setup_queue(unsigned int cpu, struct xive_cpu *xc, if (IS_ERR(qpage)) return PTR_ERR(qpage); - return xive_spapr_configure_queue(cpu, q, prio, qpage, - xive_queue_shift); + return xive_spapr_configure_queue(get_hard_smp_processor_id(cpu), + q, prio, qpage, xive_queue_shift); } static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc, @@ -399,10 +401,12 @@ static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc, struct xive_q *q = &xc->queue[prio]; unsigned int alloc_order; long rc; + int hw_cpu = get_hard_smp_processor_id(cpu); - rc = plpar_int_set_queue_config(0, cpu, prio, 0, 0); + rc = plpar_int_set_queue_config(0, hw_cpu, prio, 0, 0); if (rc) - pr_err("Error %ld setting queue for prio %d\n", rc, prio); + pr_err("Error %ld setting queue for CPU %d prio %d\n", rc, + hw_cpu, prio); alloc_order = xive_alloc_order(xive_queue_shift); free_pages((unsigned long)q->qpage, alloc_order);
The CPU event notification queues on sPAPR should be configured using a hardware CPU identifier. The problem did not show up on the Power Hypervisor because pHyp supports 8 threads per core which keeps CPU number contiguous. This is not the case on all sPAPR virtual machines, some use SMT=1. Also improve error logging by adding the CPU number. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- I think we should send this one to stable also. arch/powerpc/sysdev/xive/spapr.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)