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