diff mbox series

[v2,08/11] powerpc: Mark writes registering ipi to host cpu through kvm and polling

Message ID 20230510033117.1395895-9-rmclure@linux.ibm.com (mailing list archive)
State Accepted
Commit 86dacd967b80114c0c6cf0648ed1dcaea8853937
Headers show
Series powerpc: KCSAN fix warnings and mark accesses | expand

Commit Message

Rohan McLure May 10, 2023, 3:31 a.m. UTC
Mark writes to hypervisor ipi state so that KCSAN recognises these
asynchronous issue of kvmppc_{set,clear}_host_ipi to be intended, with
atomic writes. Mark asynchronous polls to this variable in
kvm_ppc_read_one_intr().

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
v2: Add read-side annotations to both polling locations in
kvm_ppc_read_one_intr().
---
 arch/powerpc/include/asm/kvm_ppc.h   | 4 ++--
 arch/powerpc/kvm/book3s_hv_builtin.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Nicholas Piggin May 15, 2023, 5:53 a.m. UTC | #1
On Wed May 10, 2023 at 1:31 PM AEST, Rohan McLure wrote:
> Mark writes to hypervisor ipi state so that KCSAN recognises these
> asynchronous issue of kvmppc_{set,clear}_host_ipi to be intended, with
> atomic writes. Mark asynchronous polls to this variable in
> kvm_ppc_read_one_intr().
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>

What's the go with accesses in asm? Does it just assume you know
what you're doing?

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> v2: Add read-side annotations to both polling locations in
> kvm_ppc_read_one_intr().
> ---
>  arch/powerpc/include/asm/kvm_ppc.h   | 4 ++--
>  arch/powerpc/kvm/book3s_hv_builtin.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index bc57d058ad5b..d701df006c08 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -548,12 +548,12 @@ static inline void kvmppc_set_host_ipi(int cpu)
>  	 * pairs with the barrier in kvmppc_clear_host_ipi()
>  	 */
>  	smp_mb();
> -	paca_ptrs[cpu]->kvm_hstate.host_ipi = 1;
> +	WRITE_ONCE(paca_ptrs[cpu]->kvm_hstate.host_ipi, 1);
>  }
>  
>  static inline void kvmppc_clear_host_ipi(int cpu)
>  {
> -	paca_ptrs[cpu]->kvm_hstate.host_ipi = 0;
> +	WRITE_ONCE(paca_ptrs[cpu]->kvm_hstate.host_ipi, 0);
>  	/*
>  	 * order clearing of host_ipi flag vs. processing of IPI messages
>  	 *
> diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
> index da85f046377a..0f5b021fa559 100644
> --- a/arch/powerpc/kvm/book3s_hv_builtin.c
> +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
> @@ -406,7 +406,7 @@ static long kvmppc_read_one_intr(bool *again)
>  		return 1;
>  
>  	/* see if a host IPI is pending */
> -	host_ipi = local_paca->kvm_hstate.host_ipi;
> +	host_ipi = READ_ONCE(local_paca->kvm_hstate.host_ipi);
>  	if (host_ipi)
>  		return 1;
>  
> @@ -466,7 +466,7 @@ static long kvmppc_read_one_intr(bool *again)
>  		 * meantime. If it's clear, we bounce the interrupt to the
>  		 * guest
>  		 */
> -		host_ipi = local_paca->kvm_hstate.host_ipi;
> +		host_ipi = READ_ONCE(local_paca->kvm_hstate.host_ipi);
>  		if (unlikely(host_ipi != 0)) {
>  			/* We raced with the host,
>  			 * we need to resend that IPI, bummer
> -- 
> 2.37.2
Rohan McLure May 15, 2023, 10:19 p.m. UTC | #2
> On 15 May 2023, at 3:53 pm, Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> On Wed May 10, 2023 at 1:31 PM AEST, Rohan McLure wrote:
>> Mark writes to hypervisor ipi state so that KCSAN recognises these
>> asynchronous issue of kvmppc_{set,clear}_host_ipi to be intended, with
>> atomic writes. Mark asynchronous polls to this variable in
>> kvm_ppc_read_one_intr().
>> 
>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> 
> What's the go with accesses in asm? Does it just assume you know
> what you're doing?

Exactly, KCSAN only emits instrumentation calls to around load/store 
instructions that the compiler itself generated. So by default, asm
accesses are not instrumented.

Thanks

> 
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> 
>> ---
>> v2: Add read-side annotations to both polling locations in
>> kvm_ppc_read_one_intr().
>> ---
>> arch/powerpc/include/asm/kvm_ppc.h   | 4 ++--
>> arch/powerpc/kvm/book3s_hv_builtin.c | 4 ++--
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
>> index bc57d058ad5b..d701df006c08 100644
>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>> @@ -548,12 +548,12 @@ static inline void kvmppc_set_host_ipi(int cpu)
>> * pairs with the barrier in kvmppc_clear_host_ipi()
>> */
>> smp_mb();
>> - paca_ptrs[cpu]->kvm_hstate.host_ipi = 1;
>> + WRITE_ONCE(paca_ptrs[cpu]->kvm_hstate.host_ipi, 1);
>> }
>> 
>> static inline void kvmppc_clear_host_ipi(int cpu)
>> {
>> - paca_ptrs[cpu]->kvm_hstate.host_ipi = 0;
>> + WRITE_ONCE(paca_ptrs[cpu]->kvm_hstate.host_ipi, 0);
>> /*
>> * order clearing of host_ipi flag vs. processing of IPI messages
>> *
>> diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
>> index da85f046377a..0f5b021fa559 100644
>> --- a/arch/powerpc/kvm/book3s_hv_builtin.c
>> +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
>> @@ -406,7 +406,7 @@ static long kvmppc_read_one_intr(bool *again)
>> return 1;
>> 
>> /* see if a host IPI is pending */
>> - host_ipi = local_paca->kvm_hstate.host_ipi;
>> + host_ipi = READ_ONCE(local_paca->kvm_hstate.host_ipi);
>> if (host_ipi)
>> return 1;
>> 
>> @@ -466,7 +466,7 @@ static long kvmppc_read_one_intr(bool *again)
>> * meantime. If it's clear, we bounce the interrupt to the
>> * guest
>> */
>> - host_ipi = local_paca->kvm_hstate.host_ipi;
>> + host_ipi = READ_ONCE(local_paca->kvm_hstate.host_ipi);
>> if (unlikely(host_ipi != 0)) {
>> /* We raced with the host,
>> * we need to resend that IPI, bummer
>> -- 
>> 2.37.2
>
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index bc57d058ad5b..d701df006c08 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -548,12 +548,12 @@  static inline void kvmppc_set_host_ipi(int cpu)
 	 * pairs with the barrier in kvmppc_clear_host_ipi()
 	 */
 	smp_mb();
-	paca_ptrs[cpu]->kvm_hstate.host_ipi = 1;
+	WRITE_ONCE(paca_ptrs[cpu]->kvm_hstate.host_ipi, 1);
 }
 
 static inline void kvmppc_clear_host_ipi(int cpu)
 {
-	paca_ptrs[cpu]->kvm_hstate.host_ipi = 0;
+	WRITE_ONCE(paca_ptrs[cpu]->kvm_hstate.host_ipi, 0);
 	/*
 	 * order clearing of host_ipi flag vs. processing of IPI messages
 	 *
diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
index da85f046377a..0f5b021fa559 100644
--- a/arch/powerpc/kvm/book3s_hv_builtin.c
+++ b/arch/powerpc/kvm/book3s_hv_builtin.c
@@ -406,7 +406,7 @@  static long kvmppc_read_one_intr(bool *again)
 		return 1;
 
 	/* see if a host IPI is pending */
-	host_ipi = local_paca->kvm_hstate.host_ipi;
+	host_ipi = READ_ONCE(local_paca->kvm_hstate.host_ipi);
 	if (host_ipi)
 		return 1;
 
@@ -466,7 +466,7 @@  static long kvmppc_read_one_intr(bool *again)
 		 * meantime. If it's clear, we bounce the interrupt to the
 		 * guest
 		 */
-		host_ipi = local_paca->kvm_hstate.host_ipi;
+		host_ipi = READ_ONCE(local_paca->kvm_hstate.host_ipi);
 		if (unlikely(host_ipi != 0)) {
 			/* We raced with the host,
 			 * we need to resend that IPI, bummer