* [PATCH] kvm: powerpc: book3s: Disable preemption while accessing paca xics_phys filed
@ 2017-03-27 8:26 Denis Kirjanov
2017-03-29 1:07 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: Denis Kirjanov @ 2017-03-27 8:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: benh, mpe, paulus, Denis Kirjanov
With CONFIG_DEBUG_PREEMPT enabled we see the following trace:
[ 129.314426] BUG: using smp_processor_id() in preemptible [00000000]
code: modprobe/5459
[ 129.314580] caller is .kvmppc_book3s_init_hv+0x184/0x350 [kvm_hv]
[ 129.314697] CPU: 11 PID: 5459 Comm: modprobe Not tainted
4.11.0-rc3-00022-gc7e790c #1
[ 129.314848] Call Trace:
[ 129.314915] [c0000007d624b810] [c0000000023eef10] .dump_stack+0xe4/0x150 (unreliable)
[ 129.315065] [c0000007d624b8a0] [c000000001f6ec04] .check_preemption_disabled+0x134/0x150
[ 129.315210] [c0000007d624b940] [d00000000a010274] .kvmppc_book3s_init_hv+0x184/0x350 [kvm_hv]
[ 129.315339] [c0000007d624ba00] [c00000000191d5cc] .do_one_initcall+0x5c/0x1c0
[ 129.315416] [c0000007d624bad0] [c0000000023e9494] .do_init_module+0x84/0x240
[ 129.315492] [c0000007d624bb70] [c000000001aade18] .load_module+0x1f68/0x2a10
[ 129.315568] [c0000007d624bd20] [c000000001aaeb30] .SyS_finit_module+0xc0/0xf0
[ 129.315645] [c0000007d624be30] [c00000000191baec] system_call+0x38/0xfc
Disable preemption while accessing paca struct to avoid inconsistent state
Fixes: f725758 ("KVM: PPC: Book3S HV: Use OPAL XICS emulation on POWER9")
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
arch/powerpc/kvm/book3s_hv.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 1ec86d9..8e96bab 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3913,6 +3913,10 @@ static int kvmppc_radix_possible(void)
static int kvmppc_book3s_init_hv(void)
{
int r;
+#ifdef CONFIG_SMP
+ unsigned long xics_phys;
+#endif
+
/*
* FIXME!! Do we need to check on all cpus ?
*/
@@ -3930,7 +3934,11 @@ static int kvmppc_book3s_init_hv(void)
* indirectly, via OPAL.
*/
#ifdef CONFIG_SMP
- if (!get_paca()->kvm_hstate.xics_phys) {
+ preempt_disable();
+ xics_phys = get_paca()->kvm_hstate.xics_phys;
+ preempt_enable();
+
+ if (!xics_phys) {
struct device_node *np;
np = of_find_compatible_node(NULL, NULL, "ibm,opal-intc");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] kvm: powerpc: book3s: Disable preemption while accessing paca xics_phys filed
2017-03-27 8:26 [PATCH] kvm: powerpc: book3s: Disable preemption while accessing paca xics_phys filed Denis Kirjanov
@ 2017-03-29 1:07 ` Michael Ellerman
2017-03-29 8:54 ` Denis Kirjanov
2017-03-29 9:37 ` Paul Mackerras
0 siblings, 2 replies; 6+ messages in thread
From: Michael Ellerman @ 2017-03-29 1:07 UTC (permalink / raw)
To: Denis Kirjanov, linuxppc-dev; +Cc: benh, paulus, Denis Kirjanov
Denis Kirjanov <kda@linux-powerpc.org> writes:
> With CONFIG_DEBUG_PREEMPT enabled we see the following trace:
>
> [ 129.314426] BUG: using smp_processor_id() in preemptible [00000000]
> code: modprobe/5459
> [ 129.314580] caller is .kvmppc_book3s_init_hv+0x184/0x350 [kvm_hv]
> [ 129.314697] CPU: 11 PID: 5459 Comm: modprobe Not tainted
> 4.11.0-rc3-00022-gc7e790c #1
> [ 129.314848] Call Trace:
> [ 129.314915] [c0000007d624b810] [c0000000023eef10] .dump_stack+0xe4/0x150 (unreliable)
> [ 129.315065] [c0000007d624b8a0] [c000000001f6ec04] .check_preemption_disabled+0x134/0x150
> [ 129.315210] [c0000007d624b940] [d00000000a010274] .kvmppc_book3s_init_hv+0x184/0x350 [kvm_hv]
> [ 129.315339] [c0000007d624ba00] [c00000000191d5cc] .do_one_initcall+0x5c/0x1c0
> [ 129.315416] [c0000007d624bad0] [c0000000023e9494] .do_init_module+0x84/0x240
> [ 129.315492] [c0000007d624bb70] [c000000001aade18] .load_module+0x1f68/0x2a10
> [ 129.315568] [c0000007d624bd20] [c000000001aaeb30] .SyS_finit_module+0xc0/0xf0
> [ 129.315645] [c0000007d624be30] [c00000000191baec] system_call+0x38/0xfc
>
> Disable preemption while accessing paca struct to avoid inconsistent state
>
> Fixes: f725758 ("KVM: PPC: Book3S HV: Use OPAL XICS emulation on POWER9")
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
> arch/powerpc/kvm/book3s_hv.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 1ec86d9..8e96bab 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3913,6 +3913,10 @@ static int kvmppc_radix_possible(void)
> static int kvmppc_book3s_init_hv(void)
> {
> int r;
> +#ifdef CONFIG_SMP
> + unsigned long xics_phys;
> +#endif
> +
> /*
> * FIXME!! Do we need to check on all cpus ?
> */
> @@ -3930,7 +3934,11 @@ static int kvmppc_book3s_init_hv(void)
> * indirectly, via OPAL.
> */
> #ifdef CONFIG_SMP
> - if (!get_paca()->kvm_hstate.xics_phys) {
> + preempt_disable();
> + xics_phys = get_paca()->kvm_hstate.xics_phys;
> + preempt_enable();
> +
> + if (!xics_phys) {
> struct device_node *np;
Can you tell me what actual problem (other than the warning) this is
fixing?
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kvm: powerpc: book3s: Disable preemption while accessing paca xics_phys filed
2017-03-29 1:07 ` Michael Ellerman
@ 2017-03-29 8:54 ` Denis Kirjanov
2017-03-29 9:37 ` Paul Mackerras
1 sibling, 0 replies; 6+ messages in thread
From: Denis Kirjanov @ 2017-03-29 8:54 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Denis Kirjanov, linuxppc-dev, Paul Mackerras
> Can you tell me what actual problem (other than the warning) this is
> fixing?
Since we're running with CONFIG_DEBUG_PREEMPT and CONFIG_SMP
get_paca defined as:
#define get_paca() ((void) debug_smp_processor_id(), local_paca)
instead of direct access to r13 so it's possible that we're preempted
on a different CPU.
> cheers
>
--=20
Regards / Mit besten Gr=C3=BC=C3=9Fen,
Denis
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm: powerpc: book3s: Disable preemption while accessing paca xics_phys filed
2017-03-29 1:07 ` Michael Ellerman
2017-03-29 8:54 ` Denis Kirjanov
@ 2017-03-29 9:37 ` Paul Mackerras
2017-03-29 9:45 ` Denis Kirjanov
1 sibling, 1 reply; 6+ messages in thread
From: Paul Mackerras @ 2017-03-29 9:37 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Denis Kirjanov, linuxppc-dev, benh
On Wed, Mar 29, 2017 at 12:07:03PM +1100, Michael Ellerman wrote:
> Denis Kirjanov <kda@linux-powerpc.org> writes:
>
[...]
> > @@ -3930,7 +3934,11 @@ static int kvmppc_book3s_init_hv(void)
> > * indirectly, via OPAL.
> > */
> > #ifdef CONFIG_SMP
> > - if (!get_paca()->kvm_hstate.xics_phys) {
> > + preempt_disable();
> > + xics_phys = get_paca()->kvm_hstate.xics_phys;
> > + preempt_enable();
> > +
> > + if (!xics_phys) {
> > struct device_node *np;
>
> Can you tell me what actual problem (other than the warning) this is
> fixing?
If paca->kvm_hstate.xics_phys is non-zero on one cpu, it will be
non-zero on them all. Therefore this is not fixing any actual
problem, just the warning.
By the way, the #ifdefs are ugly, and could be removed. (Yes I know
it was me that put the first #ifdef in that function.)
Paul.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kvm: powerpc: book3s: Disable preemption while accessing paca xics_phys filed
2017-03-29 9:37 ` Paul Mackerras
@ 2017-03-29 9:45 ` Denis Kirjanov
2017-03-29 10:46 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: Denis Kirjanov @ 2017-03-29 9:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Michael Ellerman, linuxppc-dev@ozlabs.org, Denis Kirjanov
[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]
On Wednesday, March 29, 2017, Paul Mackerras <paulus@ozlabs.org> wrote:
> On Wed, Mar 29, 2017 at 12:07:03PM +1100, Michael Ellerman wrote:
> > Denis Kirjanov <kda@linux-powerpc.org <javascript:;>> writes:
> >
> [...]
> > > @@ -3930,7 +3934,11 @@ static int kvmppc_book3s_init_hv(void)
> > > * indirectly, via OPAL.
> > > */
> > > #ifdef CONFIG_SMP
> > > - if (!get_paca()->kvm_hstate.xics_phys) {
> > > + preempt_disable();
> > > + xics_phys = get_paca()->kvm_hstate.xics_phys;
> > > + preempt_enable();
> > > +
> > > + if (!xics_phys) {
> > > struct device_node *np;
> >
> > Can you tell me what actual problem (other than the warning) this is
> > fixing?
>
> If paca->kvm_hstate.xics_phys is non-zero on one cpu, it will be
> non-zero on them all. Therefore this is not fixing any actual
> problem, just the warning.
>
> By the way, the #ifdefs are ugly, and could be removed. (Yes I know
> it was me that put the first #ifdef in that function.)
>
> Paul.
>
Then we can replace get_paca with local_paca to avoid the warning, right?
--
Regards / Mit besten Grüßen,
Denis
[-- Attachment #2: Type: text/html, Size: 1627 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kvm: powerpc: book3s: Disable preemption while accessing paca xics_phys filed
2017-03-29 9:45 ` Denis Kirjanov
@ 2017-03-29 10:46 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2017-03-29 10:46 UTC (permalink / raw)
To: Denis Kirjanov, Paul Mackerras; +Cc: linuxppc-dev@ozlabs.org, Denis Kirjanov
Denis Kirjanov <kirjanov@gmail.com> writes:
> On Wednesday, March 29, 2017, Paul Mackerras <paulus@ozlabs.org> wrote:
>
>> On Wed, Mar 29, 2017 at 12:07:03PM +1100, Michael Ellerman wrote:
>> > Denis Kirjanov <kda@linux-powerpc.org <javascript:;>> writes:
>> >
>> [...]
>> > > @@ -3930,7 +3934,11 @@ static int kvmppc_book3s_init_hv(void)
>> > > * indirectly, via OPAL.
>> > > */
>> > > #ifdef CONFIG_SMP
>> > > - if (!get_paca()->kvm_hstate.xics_phys) {
>> > > + preempt_disable();
>> > > + xics_phys = get_paca()->kvm_hstate.xics_phys;
>> > > + preempt_enable();
>> > > +
>> > > + if (!xics_phys) {
>> > > struct device_node *np;
>> >
>> > Can you tell me what actual problem (other than the warning) this is
>> > fixing?
>>
>> If paca->kvm_hstate.xics_phys is non-zero on one cpu, it will be
>> non-zero on them all. Therefore this is not fixing any actual
>> problem, just the warning.
>
> Then we can replace get_paca with local_paca to avoid the warning, right?
Yeah in this case I think that's fine. I dislike it somewhat because a
reader of that code now needs to know about what local_paca is but I
guess that's OK.
My point was that just sticking preempt_disable/enable() around every
get_paca() call is not the right way to deal with these warnings.
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-29 10:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-27 8:26 [PATCH] kvm: powerpc: book3s: Disable preemption while accessing paca xics_phys filed Denis Kirjanov
2017-03-29 1:07 ` Michael Ellerman
2017-03-29 8:54 ` Denis Kirjanov
2017-03-29 9:37 ` Paul Mackerras
2017-03-29 9:45 ` Denis Kirjanov
2017-03-29 10:46 ` 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).