[PATCH] powerpc/xive: use hw CPU ids when configuring the CPU queues

[PATCH] powerpc/xive: use hw CPU ids when configuring the CPU queues

[Cédric Le Goater]

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(-)

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

Re: [PATCH] powerpc/xive: use hw CPU ids when configuring the CPU queues

[Michael Ellerman]

C=C3=A9dric 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=3D1.
>
> Also improve error logging by adding the CPU number.
>
> Signed-off-by: C=C3=A9dric Le Goater <clg@kaod.org>
> ---
>
>  I think we should send this one to stable also.

Fixes: eac1e731b59e ("powerpc/xive: guest exploitation of the XIVE interrup=
t 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, str=
uct xive_q *q, u8 prio,
>=20=20
>  	rc =3D 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 =3D -EIO;
>  		goto fail;
>  	}
> @@ -370,7 +371,8 @@ static int xive_spapr_configure_queue(u32 target, str=
uct xive_q *q, u8 prio,
>  	/* Configure and enable the queue in HW */
>  	rc =3D plpar_int_set_queue_config(flags, target, prio, qpage_phys, orde=
r);
>  	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 =3D -EIO;
>  	} else {
>  		q->qpage =3D qpage;
> @@ -389,8 +391,8 @@ static int xive_spapr_setup_queue(unsigned int cpu, s=
truct xive_cpu *xc,
>  	if (IS_ERR(qpage))
>  		return PTR_ERR(qpage);
>=20=20
> -	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);
>  }
>=20=20
>  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 c=
pu, struct xive_cpu *xc,
>  	struct xive_q *q =3D &xc->queue[prio];
>  	unsigned int alloc_order;
>  	long rc;
> +	int hw_cpu =3D get_hard_smp_processor_id(cpu);
>=20=20
> -	rc =3D plpar_int_set_queue_config(0, cpu, prio, 0, 0);
> +	rc =3D 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);
>=20=20
>  	alloc_order =3D xive_alloc_order(xive_queue_shift);
>  	free_pages((unsigned long)q->qpage, alloc_order);
> --=20
> 2.13.6

Re: [PATCH] powerpc/xive: use hw CPU ids when configuring the CPU queues

[Cédric Le Goater]

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

Re: [PATCH] powerpc/xive: use hw CPU ids when configuring the CPU queues

[Michael Ellerman]

C=C3=A9dric Le Goater <clg@kaod.org> writes:

> On 02/13/2018 10:18 AM, Michael Ellerman wrote:
>> C=C3=A9dric Le Goater <clg@kaod.org> writes:
>>=20
>>> 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=3D1.
>>>
>>> Also improve error logging by adding the CPU number.
>>>
>>> Signed-off-by: C=C3=A9dric Le Goater <clg@kaod.org>
>>> ---
>>>
>>>  I think we should send this one to stable also.
>>=20
>> Fixes: eac1e731b59e ("powerpc/xive: guest exploitation of the XIVE inter=
rupt controller")
>
> yes.
>
>> Cc: stable@vger.kernel.org # v4.14+
>
> yes. I just added the Cc:. I am not sure that will work with=20
> patchwork though.

They don't accept patches that way.

I'll add the tags and commit it.

cheers

Re: powerpc/xive: use hw CPU ids when configuring the CPU queues

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

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