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

On 15 May 2023, at 3:53 pm, Nicholas Piggin [off-list ref] wrote:

On Wed May 10, 2023 at 1:31 PM AEST, Rohan McLure wrote:

quoted

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 <redacted>

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>

quoted

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