linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL
@ 2022-09-29  1:57 Haren Myneni
  2022-09-29 22:16 ` Nathan Lynch
  2022-10-04 13:26 ` Michael Ellerman
  0 siblings, 2 replies; 5+ messages in thread
From: Haren Myneni @ 2022-09-29  1:57 UTC (permalink / raw)
  To: mpe, npiggin, nathanl, linuxppc-dev


Generally the hypervisor decides to allocate a window on different
VAS instances. But if the user space wishes to allocate on the
current VAS instance where the process is executing, the kernel has
to pass associativity domain IDs to allocate VAS window HCALL. To
determine the associativity domain IDs for the current CPU, passing
smp_processor_id() to node associativity HCALL which may return
H_P2 (-55) error during DLPAR CPU event.

This patch fixes this issue by passing hard_smp_processor_id() with
VPHN_FLAG_VCPU flag (PAPR 14.11.6.1 H_HOME_NODE_ASSOCIATIVITY).

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/vas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index fe33bdb620d5..533026fd1f40 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -348,7 +348,7 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
 		 * So no unpacking needs to be done.
 		 */
 		rc = plpar_hcall9(H_HOME_NODE_ASSOCIATIVITY, domain,
-				  VPHN_FLAG_VCPU, smp_processor_id());
+				  VPHN_FLAG_VCPU, hard_smp_processor_id());
 		if (rc != H_SUCCESS) {
 			pr_err("H_HOME_NODE_ASSOCIATIVITY error: %d\n", rc);
 			goto out;
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL
  2022-09-29  1:57 [PATCH] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL Haren Myneni
@ 2022-09-29 22:16 ` Nathan Lynch
  2022-09-30  8:55   ` Michal Suchánek
  2022-10-04 13:26 ` Michael Ellerman
  1 sibling, 1 reply; 5+ messages in thread
From: Nathan Lynch @ 2022-09-29 22:16 UTC (permalink / raw)
  To: Haren Myneni; +Cc: linuxppc-dev, npiggin

Haren Myneni <haren@linux.ibm.com> writes:
> Generally the hypervisor decides to allocate a window on different
> VAS instances. But if the user space wishes to allocate on the
> current VAS instance where the process is executing, the kernel has
> to pass associativity domain IDs to allocate VAS window HCALL. To
> determine the associativity domain IDs for the current CPU, passing
> smp_processor_id() to node associativity HCALL which may return
> H_P2 (-55) error during DLPAR CPU event.
>
> This patch fixes this issue by passing hard_smp_processor_id() with
> VPHN_FLAG_VCPU flag (PAPR 14.11.6.1 H_HOME_NODE_ASSOCIATIVITY).
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/vas.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index fe33bdb620d5..533026fd1f40 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -348,7 +348,7 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
>  		 * So no unpacking needs to be done.
>  		 */
>  		rc = plpar_hcall9(H_HOME_NODE_ASSOCIATIVITY, domain,
> -				  VPHN_FLAG_VCPU, smp_processor_id());
> +				  VPHN_FLAG_VCPU, hard_smp_processor_id());
>  		if (rc != H_SUCCESS) {
>  			pr_err("H_HOME_NODE_ASSOCIATIVITY error: %d\n", rc);
>  			goto out;

Yes, it is always wrong to pass Linux CPU numbers to the hypervisor,
which has its own numbering for hardware threads. It usually coincides
with Linux's numbering in practice, which tends to hide bugs like this.

Reviewed-by: Nathan Lynch <nathanl@linux.ibm.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL
  2022-09-29 22:16 ` Nathan Lynch
@ 2022-09-30  8:55   ` Michal Suchánek
  2022-09-30 10:35     ` Michael Ellerman
  0 siblings, 1 reply; 5+ messages in thread
From: Michal Suchánek @ 2022-09-30  8:55 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: linuxppc-dev, Haren Myneni, npiggin

Hello,

On Thu, Sep 29, 2022 at 05:16:40PM -0500, Nathan Lynch wrote:
> Haren Myneni <haren@linux.ibm.com> writes:
> > Generally the hypervisor decides to allocate a window on different
> > VAS instances. But if the user space wishes to allocate on the
> > current VAS instance where the process is executing, the kernel has
> > to pass associativity domain IDs to allocate VAS window HCALL. To
> > determine the associativity domain IDs for the current CPU, passing
> > smp_processor_id() to node associativity HCALL which may return
> > H_P2 (-55) error during DLPAR CPU event.
> >
> > This patch fixes this issue by passing hard_smp_processor_id() with
> > VPHN_FLAG_VCPU flag (PAPR 14.11.6.1 H_HOME_NODE_ASSOCIATIVITY).
> >
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/pseries/vas.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> > index fe33bdb620d5..533026fd1f40 100644
> > --- a/arch/powerpc/platforms/pseries/vas.c
> > +++ b/arch/powerpc/platforms/pseries/vas.c
> > @@ -348,7 +348,7 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
> >  		 * So no unpacking needs to be done.
> >  		 */
> >  		rc = plpar_hcall9(H_HOME_NODE_ASSOCIATIVITY, domain,
> > -				  VPHN_FLAG_VCPU, smp_processor_id());
> > +				  VPHN_FLAG_VCPU, hard_smp_processor_id());
> >  		if (rc != H_SUCCESS) {
> >  			pr_err("H_HOME_NODE_ASSOCIATIVITY error: %d\n", rc);
> >  			goto out;
> 
> Yes, it is always wrong to pass Linux CPU numbers to the hypervisor,
> which has its own numbering for hardware threads. It usually coincides
> with Linux's numbering in practice, which tends to hide bugs like this.
> 
> Reviewed-by: Nathan Lynch <nathanl@linux.ibm.com>

This is the code that introduces the problem, right?

Fixes: b22f2d88e435 ("powerpc/pseries/vas: Integrate API with open/close windows")

Thanks

Michal

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL
  2022-09-30  8:55   ` Michal Suchánek
@ 2022-09-30 10:35     ` Michael Ellerman
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2022-09-30 10:35 UTC (permalink / raw)
  To: Michal Suchánek, Nathan Lynch; +Cc: Haren Myneni, linuxppc-dev, npiggin

Michal Suchánek <msuchanek@suse.de> writes:
> On Thu, Sep 29, 2022 at 05:16:40PM -0500, Nathan Lynch wrote:
>> Haren Myneni <haren@linux.ibm.com> writes:
>> > Generally the hypervisor decides to allocate a window on different
>> > VAS instances. But if the user space wishes to allocate on the
>> > current VAS instance where the process is executing, the kernel has
>> > to pass associativity domain IDs to allocate VAS window HCALL. To
>> > determine the associativity domain IDs for the current CPU, passing
>> > smp_processor_id() to node associativity HCALL which may return
>> > H_P2 (-55) error during DLPAR CPU event.
>> >
>> > This patch fixes this issue by passing hard_smp_processor_id() with
>> > VPHN_FLAG_VCPU flag (PAPR 14.11.6.1 H_HOME_NODE_ASSOCIATIVITY).
>> >
>> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
>> > ---
>> >  arch/powerpc/platforms/pseries/vas.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
>> > index fe33bdb620d5..533026fd1f40 100644
>> > --- a/arch/powerpc/platforms/pseries/vas.c
>> > +++ b/arch/powerpc/platforms/pseries/vas.c
>> > @@ -348,7 +348,7 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
>> >  		 * So no unpacking needs to be done.
>> >  		 */
>> >  		rc = plpar_hcall9(H_HOME_NODE_ASSOCIATIVITY, domain,
>> > -				  VPHN_FLAG_VCPU, smp_processor_id());
>> > +				  VPHN_FLAG_VCPU, hard_smp_processor_id());
>> >  		if (rc != H_SUCCESS) {
>> >  			pr_err("H_HOME_NODE_ASSOCIATIVITY error: %d\n", rc);
>> >  			goto out;
>> 
>> Yes, it is always wrong to pass Linux CPU numbers to the hypervisor,
>> which has its own numbering for hardware threads. It usually coincides
>> with Linux's numbering in practice, which tends to hide bugs like this.
>> 
>> Reviewed-by: Nathan Lynch <nathanl@linux.ibm.com>
>
> This is the code that introduces the problem, right?
>
> Fixes: b22f2d88e435 ("powerpc/pseries/vas: Integrate API with open/close windows")

Yeah, I tagged it when applying. Thanks.

cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL
  2022-09-29  1:57 [PATCH] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL Haren Myneni
  2022-09-29 22:16 ` Nathan Lynch
@ 2022-10-04 13:26 ` Michael Ellerman
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2022-10-04 13:26 UTC (permalink / raw)
  To: nathanl, Haren Myneni, linuxppc-dev, npiggin, mpe

On Wed, 28 Sep 2022 18:57:33 -0700, Haren Myneni wrote:
> Generally the hypervisor decides to allocate a window on different
> VAS instances. But if the user space wishes to allocate on the
> current VAS instance where the process is executing, the kernel has
> to pass associativity domain IDs to allocate VAS window HCALL. To
> determine the associativity domain IDs for the current CPU, passing
> smp_processor_id() to node associativity HCALL which may return
> H_P2 (-55) error during DLPAR CPU event.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL
      https://git.kernel.org/powerpc/c/f3e5d9e53e74d77e711a2c90a91a8b0836a9e0b3

cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-04 13:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29  1:57 [PATCH] powerpc/pseries/vas: Pass hw_cpu_id to node associativity HCALL Haren Myneni
2022-09-29 22:16 ` Nathan Lynch
2022-09-30  8:55   ` Michal Suchánek
2022-09-30 10:35     ` Michael Ellerman
2022-10-04 13:26 ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).