LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] powerpc: Add cpu family documentation
From: Scott Wood @ 2014-04-30 18:26 UTC (permalink / raw)
  To: Tom Musta; +Cc: Stephen Rothwell, linuxppc-dev
In-Reply-To: <53613D7B.8090001@gmail.com>

On Wed, 2014-04-30 at 13:14 -0500, Tom Musta wrote:
> On 4/30/2014 1:45 AM, Michael Ellerman wrote:
> >> > Are 40x considered booke?
> > You tell me.
> >  
> 
> The original 401, 403 and 405 cores predate the actual existence of what we now call Book E.
> But they most certainly contained features that would eventually become Book E (different timers,
> software managed TLB, etc.)  For the sake of this diagram, I would say "yes".

CONFIG_BOOKE doesn't get set on 40x builds...

-Scott

^ permalink raw reply

* Re: [PATCH v2] powerpc: Add cpu family documentation
From: Scott Wood @ 2014-04-30 18:29 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Stephen Rothwell
In-Reply-To: <1398840308.5722.5.camel@concordia>

On Wed, 2014-04-30 at 16:45 +1000, Michael Ellerman wrote:
> On Tue, 2014-02-04 at 16:43 -0600, Scott Wood wrote: 
> > Missing e300 (603 derivative) and e600 (7448 derivative).
> 
> Happy to add them, where do they hang off?

e300 hangs off 603 and e600 hangs off 7448. :-)

> > > +Motorola/Freescale 8xx
> > > +----------------------
> > > +
> > > + - Software loaded with hardware assist.
> > > + - All 32 bit
> > > +
> > > +   +--------------+
> > > +   |     8xx      |
> > > +   +--------------+
> > > +          |
> > > +          |
> > > +          v
> > > +   +--------------+
> > > +   |     850      |
> > > +   +--------------+
> > 
> > Is the core of MPC850 different from other MPC8xx?
> 
> Dunno, maybe someone who works at Freescale knows ;)

I think they're the same -- I was just wondering if you had some
difference in mind that led you to single it out.

-Scott

^ permalink raw reply

* Re: [PATCH v2] powerpc: Add cpu family documentation
From: Arnd Bergmann @ 2014-04-30 18:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Scott Wood, Stephen Rothwell, linuxppc-dev
In-Reply-To: <1398840308.5722.5.camel@concordia>

On Wednesday 30 April 2014 16:45:08 Michael Ellerman wrote:
> > > +          v                                              v
> > > +   +--------------+                  +-------+   +----------------+
> > > +   |   POWER4+    | ---------------> |  970  |   |      7447      |
> > > +   +--------------+                  +-------+   +----------------+
> > > +          |                              |               |
> > > +          |                              |               |
> > > +          v                              v               v
> > > +   +--------------+     +-------+    +-------+   +----------------+
> > > +   |    POWER5    | --> | Cell  |    | 970FX |   |      7448      |
> > > +   +--------------+     +-------+    +-------+   +----------------+
> > > +          |                              |
> > > +          |                              |
> > > +          v                              v
> > > +   +--------------+                  +-------+
> > > +   |   POWER5+    |                  | 970MP |
> > > +   +--------------+                  +-------+
> > > +          |
> > > +          |
> > > +          v
> > > +   +--------------+
> > > +   |   POWER5++   |
> > > +   +--------------+
> > > +          |
> > > +          |
> > > +          v
> > > +   +--------------+
> > > +   |    POWER6    |
> > > +   +--------------+
> > > +          |
> > > +          |
> > > +          v

I think Cell and POWER6 are somewhat misrepresented here. Cell (together with
Xenon, which is the same core) is basically an independent development unrelated
to POWER5. POWER6 shares a lot of concepts with Cell, and IIRC is much closer
related to Cell than to POWER5, though I don't remember which one was designed
first. Probably code went both ways, and POWER6 got some enhancements that didn't
make it into the Cell PPU. They both contain VMX (Altivec) and have an in-order
pipeline.

POWER6 is also closely related to z10, while POWER7 then includes aspects of
both POWER5 and POWER6 but no longer shares much with z196, which in turn is
derived from z10.

	Arnd

^ permalink raw reply

* Re: [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled
From: Ram Pai @ 2014-04-30 18:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Brian W Hart
In-Reply-To: <1398834894.31586.14.camel@pasglop>

On Wed, Apr 30, 2014 at 03:14:54PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2014-04-29 at 10:38 -0500, Brian W Hart wrote:
> 
> > >  CHECKFLAGS	+= -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
> > >  
> > > +ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
> > >  KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > > +endif
> > > +
> > >  
> > >  # No AltiVec or VSX instructions when building kernel
> > >  KBUILD_CFLAGS += $(call cc-option,-mno-altivec)
> > 
> > I didn't try building a kernel or in-tree modules, but I confirmed
> > that it allows building of out-of-tree modules when crtsavres.o is
> > not present (e.g. as for a distro install where the kernel headers
> > are provided by package, rather than being manually prepared from
> > the sources).
> > 
> > Tested-by: Brian W Hart <hartb@linux.vnet.ibm.com>
> 
> I still don't like it. What guarantee do we have that gcc will never
> call into this with other optimisation settings ? It might decide
> one day that calling out for saving a large pile of registers
> is still more efficient than unrolling the whole lot, including
> for speed.

This patch operates on the assumption that arch/powerpc/lib/crtsavres.o 
is needed only if the code is compiled with -Os.  Are you saying
this assumption is wrong?

> 
> Besides that doesn't fix the root problem. We want to be able to
> build the kernel with CONFIG_CC_OPTIMIZE_FOR_SIZE and still have
> modules.

And this patch will not stop you from doing that. You can compile your
kernel with CONFIG_CC_OPTIMIZE_FOR_SIZE and modules will be built
because arch/powerpc/lib/crtsavres.o will be linked with the module.
Now, if the arch/powerpc/lib/crtsavres.o file does not exist, that
is a different problem and has to be fixed by the distros for 
out-of-tree modules.

> 
> So a better solution needs to be found. I don't know what that
> solution is (we might want to look at what other archs are doing
> maybe ?), could be to include crtsaveres.S in the build of every
> module (we really don't want to EXPORT_SYMBOL these guys), but
> that would mean having it installed somewhere with the kernel
> headers for out-of-tree modules...

Currently crtsaveres.o is expected to be in the build during
the linking stage of the module. You suggest instead have 
crtsaveres.S and get it compiled and linked?

> 
> If necessary, involve lkml, Rusty etc... but this patch is crap.

I dont see other archs having this problem. Possibly because there
linker have inbuilt capabilities to satisfy the missing symbols?

Alan Modra did mention that the ppc linker will soon have the
capability to handle -Os compiled code, without the help
of arch/powerpc/lib/crtsavres.o.

However this patch is not about having crtsavres.o or crtsaveres.S

Its about not needing crtsavres.o if the code is not compiled for
space optimization using -Os. If you say that the assumption
is wrong, than yes the code is crap :)


RP

^ permalink raw reply

* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall
From: Alexander Graf @ 2014-04-30 19:56 UTC (permalink / raw)
  To: Stuart Yoder, benh, scottwood; +Cc: linuxppc-dev
In-Reply-To: <1398887641-20705-1-git-send-email-stuart.yoder@freescale.com>


On 30.04.14 21:54, Stuart Yoder wrote:
> From: Stuart Yoder <stuart.yoder@freescale.com>
>
> some restructuring of epapr paravirt init resulted in
> ppc_md.power_save being set, and then overwritten to
> NULL during machine_init.  This patch splits the
> initialization of ppc_md.power_save out into a postcore
> init call.
>
> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> ---
>   arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
> index 6300c13..c49b69c 100644
> --- a/arch/powerpc/kernel/epapr_paravirt.c
> +++ b/arch/powerpc/kernel/epapr_paravirt.c
> @@ -52,11 +52,6 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
>   #endif
>   	}
>   
> -#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> -	if (of_get_flat_dt_prop(node, "has-idle", NULL))
> -		ppc_md.power_save = epapr_ev_idle;
> -#endif
> -
>   	epapr_paravirt_enabled = true;
>   
>   	return 1;
> @@ -69,3 +64,23 @@ int __init epapr_paravirt_early_init(void)
>   	return 0;
>   }
>   
> +static int __init epapr_idle_init_dt_scan(unsigned long node,
> +					   const char *uname,
> +					   int depth, void *data)
> +{
> +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> +	if (of_get_flat_dt_prop(node, "has-idle", NULL))
> +		ppc_md.power_save = epapr_ev_idle;
> +#endif
> +	return 0;
> +}
> +
> +static int __init epapr_idle_init(void)
> +{
> +	if (epapr_paravirt_enabled)
> +		of_scan_flat_dt(epapr_idle_init_dt_scan, NULL);

Doesn't this scan all nodes? We only want to match on 
/hypervisor/has-idle, no?


Alex

^ permalink raw reply

* RE: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall
From: Stuart Yoder @ 2014-04-30 20:03 UTC (permalink / raw)
  To: Alexander Graf, benh@kernel.crashing.org, Scott Wood
  Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <5361556D.7080805@suse.de>



> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Wednesday, April 30, 2014 2:56 PM
> To: Yoder Stuart-B08248; benh@kernel.crashing.org; Wood Scott-B07421
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH] powerpc: move epapr paravirt init of power_save to
> an initcall
>=20
>=20
> On 30.04.14 21:54, Stuart Yoder wrote:
> > From: Stuart Yoder <stuart.yoder@freescale.com>
> >
> > some restructuring of epapr paravirt init resulted in
> > ppc_md.power_save being set, and then overwritten to
> > NULL during machine_init.  This patch splits the
> > initialization of ppc_md.power_save out into a postcore
> > init call.
> >
> > Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> > ---
> >   arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
> >   1 file changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/epapr_paravirt.c
> b/arch/powerpc/kernel/epapr_paravirt.c
> > index 6300c13..c49b69c 100644
> > --- a/arch/powerpc/kernel/epapr_paravirt.c
> > +++ b/arch/powerpc/kernel/epapr_paravirt.c
> > @@ -52,11 +52,6 @@ static int __init early_init_dt_scan_epapr(unsigned
> long node,
> >   #endif
> >   	}
> >
> > -#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> > -	if (of_get_flat_dt_prop(node, "has-idle", NULL))
> > -		ppc_md.power_save =3D epapr_ev_idle;
> > -#endif
> > -
> >   	epapr_paravirt_enabled =3D true;
> >
> >   	return 1;
> > @@ -69,3 +64,23 @@ int __init epapr_paravirt_early_init(void)
> >   	return 0;
> >   }
> >
> > +static int __init epapr_idle_init_dt_scan(unsigned long node,
> > +					   const char *uname,
> > +					   int depth, void *data)
> > +{
> > +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> > +	if (of_get_flat_dt_prop(node, "has-idle", NULL))
> > +		ppc_md.power_save =3D epapr_ev_idle;
> > +#endif
> > +	return 0;
> > +}
> > +
> > +static int __init epapr_idle_init(void)
> > +{
> > +	if (epapr_paravirt_enabled)
> > +		of_scan_flat_dt(epapr_idle_init_dt_scan, NULL);
>=20
> Doesn't this scan all nodes? We only want to match on
> /hypervisor/has-idle, no?

I cut/pasted from  the approach the existing code in that file
took, but yes you're right we just need the one property.
Let me respin that to look at the hypervisor node only.

Stuart

^ permalink raw reply

* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall
From: Alexander Graf @ 2014-04-30 20:06 UTC (permalink / raw)
  To: Stuart Yoder, benh@kernel.crashing.org, Scott Wood
  Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <33939d8249c34c8fb694da3e94196211@DM2PR03MB352.namprd03.prod.outlook.com>


On 30.04.14 22:03, Stuart Yoder wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Wednesday, April 30, 2014 2:56 PM
>> To: Yoder Stuart-B08248; benh@kernel.crashing.org; Wood Scott-B07421
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH] powerpc: move epapr paravirt init of power_save to
>> an initcall
>>
>>
>> On 30.04.14 21:54, Stuart Yoder wrote:
>>> From: Stuart Yoder <stuart.yoder@freescale.com>
>>>
>>> some restructuring of epapr paravirt init resulted in
>>> ppc_md.power_save being set, and then overwritten to
>>> NULL during machine_init.  This patch splits the
>>> initialization of ppc_md.power_save out into a postcore
>>> init call.
>>>
>>> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
>>> ---
>>>    arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
>>>    1 file changed, 20 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/epapr_paravirt.c
>> b/arch/powerpc/kernel/epapr_paravirt.c
>>> index 6300c13..c49b69c 100644
>>> --- a/arch/powerpc/kernel/epapr_paravirt.c
>>> +++ b/arch/powerpc/kernel/epapr_paravirt.c
>>> @@ -52,11 +52,6 @@ static int __init early_init_dt_scan_epapr(unsigned
>> long node,
>>>    #endif
>>>    	}
>>>
>>> -#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
>>> -	if (of_get_flat_dt_prop(node, "has-idle", NULL))
>>> -		ppc_md.power_save = epapr_ev_idle;
>>> -#endif
>>> -
>>>    	epapr_paravirt_enabled = true;
>>>
>>>    	return 1;
>>> @@ -69,3 +64,23 @@ int __init epapr_paravirt_early_init(void)
>>>    	return 0;
>>>    }
>>>
>>> +static int __init epapr_idle_init_dt_scan(unsigned long node,
>>> +					   const char *uname,
>>> +					   int depth, void *data)
>>> +{
>>> +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
>>> +	if (of_get_flat_dt_prop(node, "has-idle", NULL))
>>> +		ppc_md.power_save = epapr_ev_idle;
>>> +#endif
>>> +	return 0;
>>> +}
>>> +
>>> +static int __init epapr_idle_init(void)
>>> +{
>>> +	if (epapr_paravirt_enabled)
>>> +		of_scan_flat_dt(epapr_idle_init_dt_scan, NULL);
>> Doesn't this scan all nodes? We only want to match on
>> /hypervisor/has-idle, no?
> I cut/pasted from  the approach the existing code in that file
> took, but yes you're right we just need the one property.
> Let me respin that to look at the hypervisor node only.

The other function aborts early if it doesn't find a 
"hcall-instructions" property. I'm still surprised we don't limit the 
scope to /hypervisor at all though. Is this on purpose maybe?


Alex

^ permalink raw reply

* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall
From: Alexander Graf @ 2014-04-30 20:09 UTC (permalink / raw)
  To: Stuart Yoder, benh@kernel.crashing.org, Scott Wood
  Cc: Laurentiu.Tudor, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <33939d8249c34c8fb694da3e94196211@DM2PR03MB352.namprd03.prod.outlook.com>


On 30.04.14 22:03, Stuart Yoder wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Wednesday, April 30, 2014 2:56 PM
>> To: Yoder Stuart-B08248; benh@kernel.crashing.org; Wood Scott-B07421
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH] powerpc: move epapr paravirt init of power_save to
>> an initcall
>>
>>
>> On 30.04.14 21:54, Stuart Yoder wrote:
>>> From: Stuart Yoder <stuart.yoder@freescale.com>
>>>
>>> some restructuring of epapr paravirt init resulted in
>>> ppc_md.power_save being set, and then overwritten to
>>> NULL during machine_init.  This patch splits the
>>> initialization of ppc_md.power_save out into a postcore
>>> init call.
>>>
>>> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
>>> ---
>>>    arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
>>>    1 file changed, 20 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/epapr_paravirt.c
>> b/arch/powerpc/kernel/epapr_paravirt.c
>>> index 6300c13..c49b69c 100644
>>> --- a/arch/powerpc/kernel/epapr_paravirt.c
>>> +++ b/arch/powerpc/kernel/epapr_paravirt.c
>>> @@ -52,11 +52,6 @@ static int __init early_init_dt_scan_epapr(unsigned
>> long node,
>>>    #endif
>>>    	}
>>>
>>> -#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
>>> -	if (of_get_flat_dt_prop(node, "has-idle", NULL))
>>> -		ppc_md.power_save = epapr_ev_idle;
>>> -#endif
>>> -
>>>    	epapr_paravirt_enabled = true;
>>>
>>>    	return 1;
>>> @@ -69,3 +64,23 @@ int __init epapr_paravirt_early_init(void)
>>>    	return 0;
>>>    }
>>>
>>> +static int __init epapr_idle_init_dt_scan(unsigned long node,
>>> +					   const char *uname,
>>> +					   int depth, void *data)
>>> +{
>>> +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
>>> +	if (of_get_flat_dt_prop(node, "has-idle", NULL))
>>> +		ppc_md.power_save = epapr_ev_idle;
>>> +#endif
>>> +	return 0;
>>> +}
>>> +
>>> +static int __init epapr_idle_init(void)
>>> +{
>>> +	if (epapr_paravirt_enabled)
>>> +		of_scan_flat_dt(epapr_idle_init_dt_scan, NULL);
>> Doesn't this scan all nodes? We only want to match on
>> /hypervisor/has-idle, no?
> I cut/pasted from  the approach the existing code in that file
> took, but yes you're right we just need the one property.
> Let me respin that to look at the hypervisor node only.

Yeah, the same commit that introduced the breakage on has-idle also 
removed the explicit check for /hypervisor.

Laurentiu, was this change on purpose?



commit 4e21b94c9c644c43223878f4c848e852743e789c
Author: Laurentiu TUDOR <Laurentiu.Tudor@freescale.com>
Date:   Wed Jul 3 17:13:15 2013 +0300

     powerpc/85xx: Move ePAPR paravirt initialization earlier

     At console init, when the kernel tries to flush the log buffer
     the ePAPR byte-channel based console write fails silently,
     losing the buffered messages.
     This happens because The ePAPR para-virtualization init isn't
     done early enough so that the hcall instruction to be set,
     causing the byte-channel write hcall to be a nop.
     To fix, change the ePAPR para-virt init to use early device
     tree functions and move it in early init.

     Signed-off-by: Laurentiu Tudor <Laurentiu.Tudor@freescale.com>
     Signed-off-by: Scott Wood <scottwood@freescale.com>
[...]
diff --git a/arch/powerpc/kernel/epapr_paravirt.c 
b/arch/powerpc/kernel/epapr_paravirt.c
index d44a571..6300c13 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -30,22 +30,20 @@ extern u32 epapr_ev_idle_start[];

  bool epapr_paravirt_enabled;

-static int __init epapr_paravirt_init(void)
+static int __init early_init_dt_scan_epapr(unsigned long node,
+                                          const char *uname,
+                                          int depth, void *data)
  {
-       struct device_node *hyper_node;
         const u32 *insts;
-       int len, i;
+       unsigned long len;
+       int i;

-       hyper_node = of_find_node_by_path("/hypervisor");
-       if (!hyper_node)
-               return -ENODEV;
-
-       insts = of_get_property(hyper_node, "hcall-instructions", &len);
+       insts = of_get_flat_dt_prop(node, "hcall-instructions", &len);
         if (!insts)
-               return -ENODEV;
+               return 0;

         if (len % 4 || len > (4 * 4))
-               return -ENODEV;
+               return -1;
[...]


Alex

^ permalink raw reply related

* [PATCH] powerpc: move epapr paravirt init of power_save to an initcall
From: Stuart Yoder @ 2014-04-30 19:54 UTC (permalink / raw)
  To: benh, scottwood; +Cc: linuxppc-dev, agraf, Stuart Yoder

From: Stuart Yoder <stuart.yoder@freescale.com>

some restructuring of epapr paravirt init resulted in
ppc_md.power_save being set, and then overwritten to
NULL during machine_init.  This patch splits the
initialization of ppc_md.power_save out into a postcore
init call.

Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---
 arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
index 6300c13..c49b69c 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -52,11 +52,6 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
 #endif
 	}
 
-#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
-	if (of_get_flat_dt_prop(node, "has-idle", NULL))
-		ppc_md.power_save = epapr_ev_idle;
-#endif
-
 	epapr_paravirt_enabled = true;
 
 	return 1;
@@ -69,3 +64,23 @@ int __init epapr_paravirt_early_init(void)
 	return 0;
 }
 
+static int __init epapr_idle_init_dt_scan(unsigned long node,
+					   const char *uname,
+					   int depth, void *data)
+{
+#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
+	if (of_get_flat_dt_prop(node, "has-idle", NULL))
+		ppc_md.power_save = epapr_ev_idle;
+#endif
+	return 0;
+}
+
+static int __init epapr_idle_init(void)
+{
+	if (epapr_paravirt_enabled)
+		of_scan_flat_dt(epapr_idle_init_dt_scan, NULL);
+
+	return 0;
+}
+
+postcore_initcall(epapr_idle_init);
-- 
1.7.9.7

^ permalink raw reply related

* [PATCH][v2] powerpc: move epapr paravirt init of power_save to an initcall
From: Stuart Yoder @ 2014-04-30 20:20 UTC (permalink / raw)
  To: benh, scottwood; +Cc: linuxppc-dev, agraf, Stuart Yoder

From: Stuart Yoder <stuart.yoder@freescale.com>

some restructuring of epapr paravirt init resulted in
ppc_md.power_save being set, and then overwritten to
NULL during machine_init.  This patch splits the
initialization of ppc_md.power_save out into a postcore
init call.

Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---

-v2: don't iterate over the entire DT, just look at
 the hypervisor node

 arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
index 7898be9..a01df5e 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -53,11 +53,6 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
 #endif
 	}
 
-#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
-	if (of_get_flat_dt_prop(node, "has-idle", NULL))
-		ppc_md.power_save = epapr_ev_idle;
-#endif
-
 	epapr_paravirt_enabled = true;
 
 	return 1;
@@ -70,3 +65,23 @@ int __init epapr_paravirt_early_init(void)
 	return 0;
 }
 
+static int __init epapr_idle_init(void)
+{
+	struct device_node *node;
+
+	if (!epapr_paravirt_enabled)
+		return 0;
+
+	node = of_find_node_by_path("/hypervisor");
+	if (!node)
+		return -ENODEV;
+
+#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
+	if (of_get_property(node, "has-idle", NULL))
+		ppc_md.power_save = epapr_ev_idle;
+#endif
+
+	return 0;
+}
+
+postcore_initcall(epapr_idle_init);
-- 
1.7.9.7

^ permalink raw reply related

* Re: [PATCH][v2] powerpc: move epapr paravirt init of power_save to an initcall
From: Alexander Graf @ 2014-04-30 20:25 UTC (permalink / raw)
  To: Stuart Yoder, benh, scottwood; +Cc: linuxppc-dev
In-Reply-To: <1398889209-10350-1-git-send-email-stuart.yoder@freescale.com>


On 30.04.14 22:20, Stuart Yoder wrote:
> From: Stuart Yoder <stuart.yoder@freescale.com>
>
> some restructuring of epapr paravirt init resulted in
> ppc_md.power_save being set, and then overwritten to
> NULL during machine_init.  This patch splits the
> initialization of ppc_md.power_save out into a postcore
> init call.
>
> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>

Looks pretty good to me and IMHO should get a CC on stable when it gets 
into the tree. One minor nit below.

We still need to clarify whether the omitted check on 
early_init_dt_scan_epapr() is on purpose and if not reintroduce it, but 
that's a separate issue.

> ---
>
> -v2: don't iterate over the entire DT, just look at
>   the hypervisor node
>
>   arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
> index 7898be9..a01df5e 100644
> --- a/arch/powerpc/kernel/epapr_paravirt.c
> +++ b/arch/powerpc/kernel/epapr_paravirt.c
> @@ -53,11 +53,6 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
>   #endif
>   	}
>   
> -#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> -	if (of_get_flat_dt_prop(node, "has-idle", NULL))
> -		ppc_md.power_save = epapr_ev_idle;
> -#endif
> -
>   	epapr_paravirt_enabled = true;
>   
>   	return 1;
> @@ -70,3 +65,23 @@ int __init epapr_paravirt_early_init(void)
>   	return 0;
>   }
>   

v

> +static int __init epapr_idle_init(void)
> +{
> +	struct device_node *node;
> +
> +	if (!epapr_paravirt_enabled)
> +		return 0;
> +
> +	node = of_find_node_by_path("/hypervisor");
> +	if (!node)
> +		return -ENODEV;
> +
> +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)

Please move the #if scope from v to ^. That way we don't waste space / 
time / anything on systems that don't bother with the idle hcall.

> +	if (of_get_property(node, "has-idle", NULL))
> +		ppc_md.power_save = epapr_ev_idle;
> +#endif
> +
> +	return 0;
> +}
> +
> +postcore_initcall(epapr_idle_init);

^


Alex

^ permalink raw reply

* Re: [PATCH 2/2] PCI/MSI: Phase out pci_enable_msi_block()
From: Alexander Gordeev @ 2014-04-30 21:19 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-pci, linux-mips, x86, linuxppc-dev, linux-s390
In-Reply-To: <20140414132834.GA9164@dhcp-26-207.brq.redhat.com>

On Mon, Apr 14, 2014 at 03:28:34PM +0200, Alexander Gordeev wrote:
> There are no users of pci_enable_msi_block() function have
> left. Obsolete it in favor of pci_enable_msi_range() and
> pci_enable_msi_exact() functions.

Hi Bjorn,

How about this one?

Thanks!

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

^ permalink raw reply

* [PATCH] powerpc/powernv: Reduce pseries panic timeout from 180s to 10s
From: Anton Blanchard @ 2014-04-30 21:20 UTC (permalink / raw)
  To: benh, paulus, linuxppc-dev, Michael Ellerman

We've already dropped the default pseries timeout to 10s, do
the same for powernv.

Signed-off-by: Anton Blanchard <anton@samba.org>
---
 arch/powerpc/platforms/powernv/setup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 23aab41..a789307 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -39,6 +39,8 @@
 
 static void __init pnv_setup_arch(void)
 {
+	set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
+
 	/* Initialize SMP */
 	pnv_smp_init();
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] powerpc/powernv: Reduce pseries panic timeout from 180s to 10s
From: Paul Mackerras @ 2014-04-30 21:29 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: linuxppc-dev
In-Reply-To: <20140501072004.5b694f54@kryten>

On Thu, May 01, 2014 at 07:20:04AM +1000, Anton Blanchard wrote:
> We've already dropped the default pseries timeout to 10s, do
> the same for powernv.

Why is there "pseries" in the patch subject rather than "powernv"?

Paul.

^ permalink raw reply

* [PATCH] powerpc/powernv: Reduce panic timeout from 180s to 10s
From: Anton Blanchard @ 2014-04-30 22:07 UTC (permalink / raw)
  To: Paul Mackerras, benh, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20140430212942.GC9671@iris.ozlabs.ibm.com>

We've already dropped the default pseries timeout to 10s, do
the same for powernv.

Signed-off-by: Anton Blanchard <anton@samba.org>
---
 arch/powerpc/platforms/powernv/setup.c | 2 ++
 1 file changed, 2 insertions(+)

v2: fix the commit message as Paul pointed out

diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 23aab41..a789307 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -39,6 +39,8 @@
 
 static void __init pnv_setup_arch(void)
 {
+	set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
+
 	/* Initialize SMP */
 	pnv_smp_init();
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH][v2] powerpc: move epapr paravirt init of power_save to an initcall
From: Scott Wood @ 2014-04-30 22:48 UTC (permalink / raw)
  To: Stuart Yoder; +Cc: linuxppc-dev, agraf
In-Reply-To: <1398889209-10350-1-git-send-email-stuart.yoder@freescale.com>

On Wed, 2014-04-30 at 15:20 -0500, Stuart Yoder wrote:
> From: Stuart Yoder <stuart.yoder@freescale.com>
> 
> some restructuring of epapr paravirt init resulted in
> ppc_md.power_save being set, and then overwritten to
> NULL during machine_init.  This patch splits the
> initialization of ppc_md.power_save out into a postcore
> init call.
> 
> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> ---
> 
> -v2: don't iterate over the entire DT, just look at
>  the hypervisor node
> 
>  arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
> index 7898be9..a01df5e 100644
> --- a/arch/powerpc/kernel/epapr_paravirt.c
> +++ b/arch/powerpc/kernel/epapr_paravirt.c
> @@ -53,11 +53,6 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
>  #endif
>  	}
>  
> -#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> -	if (of_get_flat_dt_prop(node, "has-idle", NULL))
> -		ppc_md.power_save = epapr_ev_idle;
> -#endif
> -
>  	epapr_paravirt_enabled = true;
>  
>  	return 1;
> @@ -70,3 +65,23 @@ int __init epapr_paravirt_early_init(void)
>  	return 0;
>  }
>  
> +static int __init epapr_idle_init(void)
> +{
> +	struct device_node *node;
> +
> +	if (!epapr_paravirt_enabled)
> +		return 0;
> +
> +	node = of_find_node_by_path("/hypervisor");
> +	if (!node)
> +		return -ENODEV;
> +
> +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
> +	if (of_get_property(node, "has-idle", NULL))
> +		ppc_md.power_save = epapr_ev_idle;
> +#endif
> +
> +	return 0;
> +}

Why duplicate the search-for-hv-node code?  Just have the early init
func set a flag indicating whether has-idle was found, and have
epapr_idle_init act on that flag.

-Scott

^ permalink raw reply

* RE: [PATCH][v2] powerpc: move epapr paravirt init of power_save to an initcall
From: Stuart Yoder @ 2014-04-30 23:14 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev@lists.ozlabs.org, agraf@suse.de
In-Reply-To: <1398898118.24575.223.camel@snotra.buserror.net>

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogV2VkbmVzZGF5LCBBcHJpbCAzMCwgMjAxNCA1OjQ5IFBNDQo+IFRvOiBZb2Rl
ciBTdHVhcnQtQjA4MjQ4DQo+IENjOiBiZW5oQGtlcm5lbC5jcmFzaGluZy5vcmc7IGxpbnV4cHBj
LWRldkBsaXN0cy5vemxhYnMub3JnOw0KPiBhZ3JhZkBzdXNlLmRlDQo+IFN1YmplY3Q6IFJlOiBb
UEFUQ0hdW3YyXSBwb3dlcnBjOiBtb3ZlIGVwYXByIHBhcmF2aXJ0IGluaXQgb2YgcG93ZXJfc2F2
ZQ0KPiB0byBhbiBpbml0Y2FsbA0KPiANCj4gT24gV2VkLCAyMDE0LTA0LTMwIGF0IDE1OjIwIC0w
NTAwLCBTdHVhcnQgWW9kZXIgd3JvdGU6DQo+ID4gRnJvbTogU3R1YXJ0IFlvZGVyIDxzdHVhcnQu
eW9kZXJAZnJlZXNjYWxlLmNvbT4NCj4gPg0KPiA+IHNvbWUgcmVzdHJ1Y3R1cmluZyBvZiBlcGFw
ciBwYXJhdmlydCBpbml0IHJlc3VsdGVkIGluDQo+ID4gcHBjX21kLnBvd2VyX3NhdmUgYmVpbmcg
c2V0LCBhbmQgdGhlbiBvdmVyd3JpdHRlbiB0bw0KPiA+IE5VTEwgZHVyaW5nIG1hY2hpbmVfaW5p
dC4gIFRoaXMgcGF0Y2ggc3BsaXRzIHRoZQ0KPiA+IGluaXRpYWxpemF0aW9uIG9mIHBwY19tZC5w
b3dlcl9zYXZlIG91dCBpbnRvIGEgcG9zdGNvcmUNCj4gPiBpbml0IGNhbGwuDQo+ID4NCj4gPiBT
aWduZWQtb2ZmLWJ5OiBTdHVhcnQgWW9kZXIgPHN0dWFydC55b2RlckBmcmVlc2NhbGUuY29tPg0K
PiA+IC0tLQ0KPiA+DQo+ID4gLXYyOiBkb24ndCBpdGVyYXRlIG92ZXIgdGhlIGVudGlyZSBEVCwg
anVzdCBsb29rIGF0DQo+ID4gIHRoZSBoeXBlcnZpc29yIG5vZGUNCj4gPg0KPiA+ICBhcmNoL3Bv
d2VycGMva2VybmVsL2VwYXByX3BhcmF2aXJ0LmMgfCAgIDI1ICsrKysrKysrKysrKysrKysrKysr
LS0tLS0NCj4gPiAgMSBmaWxlIGNoYW5nZWQsIDIwIGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25z
KC0pDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2tlcm5lbC9lcGFwcl9wYXJh
dmlydC5jDQo+IGIvYXJjaC9wb3dlcnBjL2tlcm5lbC9lcGFwcl9wYXJhdmlydC5jDQo+ID4gaW5k
ZXggNzg5OGJlOS4uYTAxZGY1ZSAxMDA2NDQNCj4gPiAtLS0gYS9hcmNoL3Bvd2VycGMva2VybmVs
L2VwYXByX3BhcmF2aXJ0LmMNCj4gPiArKysgYi9hcmNoL3Bvd2VycGMva2VybmVsL2VwYXByX3Bh
cmF2aXJ0LmMNCj4gPiBAQCAtNTMsMTEgKzUzLDYgQEAgc3RhdGljIGludCBfX2luaXQgZWFybHlf
aW5pdF9kdF9zY2FuX2VwYXByKHVuc2lnbmVkDQo+IGxvbmcgbm9kZSwNCj4gPiAgI2VuZGlmDQo+
ID4gIAl9DQo+ID4NCj4gPiAtI2lmICFkZWZpbmVkKENPTkZJR182NEJJVCkgfHwgZGVmaW5lZChD
T05GSUdfUFBDX0JPT0szRV82NCkNCj4gPiAtCWlmIChvZl9nZXRfZmxhdF9kdF9wcm9wKG5vZGUs
ICJoYXMtaWRsZSIsIE5VTEwpKQ0KPiA+IC0JCXBwY19tZC5wb3dlcl9zYXZlID0gZXBhcHJfZXZf
aWRsZTsNCj4gPiAtI2VuZGlmDQo+ID4gLQ0KPiA+ICAJZXBhcHJfcGFyYXZpcnRfZW5hYmxlZCA9
IHRydWU7DQo+ID4NCj4gPiAgCXJldHVybiAxOw0KPiA+IEBAIC03MCwzICs2NSwyMyBAQCBpbnQg
X19pbml0IGVwYXByX3BhcmF2aXJ0X2Vhcmx5X2luaXQodm9pZCkNCj4gPiAgCXJldHVybiAwOw0K
PiA+ICB9DQo+ID4NCj4gPiArc3RhdGljIGludCBfX2luaXQgZXBhcHJfaWRsZV9pbml0KHZvaWQp
DQo+ID4gK3sNCj4gPiArCXN0cnVjdCBkZXZpY2Vfbm9kZSAqbm9kZTsNCj4gPiArDQo+ID4gKwlp
ZiAoIWVwYXByX3BhcmF2aXJ0X2VuYWJsZWQpDQo+ID4gKwkJcmV0dXJuIDA7DQo+ID4gKw0KPiA+
ICsJbm9kZSA9IG9mX2ZpbmRfbm9kZV9ieV9wYXRoKCIvaHlwZXJ2aXNvciIpOw0KPiA+ICsJaWYg
KCFub2RlKQ0KPiA+ICsJCXJldHVybiAtRU5PREVWOw0KPiA+ICsNCj4gPiArI2lmICFkZWZpbmVk
KENPTkZJR182NEJJVCkgfHwgZGVmaW5lZChDT05GSUdfUFBDX0JPT0szRV82NCkNCj4gPiArCWlm
IChvZl9nZXRfcHJvcGVydHkobm9kZSwgImhhcy1pZGxlIiwgTlVMTCkpDQo+ID4gKwkJcHBjX21k
LnBvd2VyX3NhdmUgPSBlcGFwcl9ldl9pZGxlOw0KPiA+ICsjZW5kaWYNCj4gPiArDQo+ID4gKwly
ZXR1cm4gMDsNCj4gPiArfQ0KPiANCj4gV2h5IGR1cGxpY2F0ZSB0aGUgc2VhcmNoLWZvci1odi1u
b2RlIGNvZGU/ICBKdXN0IGhhdmUgdGhlIGVhcmx5IGluaXQNCj4gZnVuYyBzZXQgYSBmbGFnIGlu
ZGljYXRpbmcgd2hldGhlciBoYXMtaWRsZSB3YXMgZm91bmQsIGFuZCBoYXZlDQo+IGVwYXByX2lk
bGVfaW5pdCBhY3Qgb24gdGhhdCBmbGFnLg0KDQpUaGF0J3MgYSBnb29kIGlkZWEsIGFuZCB0aGUg
cGF0Y2ggd291bGQgYmUgcXVpdGUgYSBiaXQgc21hbGxlci4gDQpJJ2xsIGRvIHRoYXQgYW5kIGxl
YXZlIHRoZSBzdHVmZiBpbiBlYXJseV9pbml0X2R0X3NjYW5fZXBhcHIoKSBhbG9uZQ0KbW9zdGx5
LiAgIFRoZW4gYXMgYSBzZXBhcmF0ZSBzdGVwIHdlIGNvdWxkIHB1dCBiYWNrIHRoZSBjb2RlIHRo
YXQNCmxvb2tzIGF0IG9ubHkgL2h5cGVydmlzb3IgKGJhc2VkIG9uIGZlZWRiYWNrIGZyb20gTGF1
cmVudGl1KS4NCg0KU3R1YXJ0DQo=

^ permalink raw reply

* [PATCH][v3] powerpc: move epapr paravirt init of power_save to an initcall
From: Stuart Yoder @ 2014-04-30 23:23 UTC (permalink / raw)
  To: benh, scottwood; +Cc: linuxppc-dev, agraf, Stuart Yoder

From: Stuart Yoder <stuart.yoder@freescale.com>

some restructuring of epapr paravirt init resulted in
ppc_md.power_save being set, and then overwritten to
NULL during machine_init.  This patch splits the
initialization of ppc_md.power_save out into a postcore
init call.

Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---
-v3
   -changed approach slightly, set flag in the dt scanning
    code and just look at that flag in the initcall


 arch/powerpc/kernel/epapr_paravirt.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
index 7898be9..6596cd7 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -30,6 +30,7 @@ extern u32 epapr_ev_idle_start[];
 #endif
 
 bool epapr_paravirt_enabled;
+bool epapr_has_idle;
 
 static int __init early_init_dt_scan_epapr(unsigned long node,
 					   const char *uname,
@@ -55,7 +56,7 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
 
 #if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
 	if (of_get_flat_dt_prop(node, "has-idle", NULL))
-		ppc_md.power_save = epapr_ev_idle;
+		epapr_has_idle = true;
 #endif
 
 	epapr_paravirt_enabled = true;
@@ -70,3 +71,12 @@ int __init epapr_paravirt_early_init(void)
 	return 0;
 }
 
+static int __init epapr_idle_init(void)
+{
+	if (epapr_has_idle)
+		ppc_md.power_save = epapr_ev_idle;
+
+	return 0;
+}
+
+postcore_initcall(epapr_idle_init);
-- 
1.7.9.7

^ permalink raw reply related

* Re: [PATCH][v3] powerpc: move epapr paravirt init of power_save to an initcall
From: Scott Wood @ 2014-04-30 23:24 UTC (permalink / raw)
  To: Stuart Yoder; +Cc: linuxppc-dev, agraf
In-Reply-To: <1398900192-2646-1-git-send-email-stuart.yoder@freescale.com>

On Wed, 2014-04-30 at 18:23 -0500, Stuart Yoder wrote:
> From: Stuart Yoder <stuart.yoder@freescale.com>
> 
> some restructuring of epapr paravirt init resulted in
> ppc_md.power_save being set, and then overwritten to
> NULL during machine_init.  This patch splits the
> initialization of ppc_md.power_save out into a postcore
> init call.
> 
> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> ---
> -v3
>    -changed approach slightly, set flag in the dt scanning
>     code and just look at that flag in the initcall
> 
> 
>  arch/powerpc/kernel/epapr_paravirt.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
> index 7898be9..6596cd7 100644
> --- a/arch/powerpc/kernel/epapr_paravirt.c
> +++ b/arch/powerpc/kernel/epapr_paravirt.c
> @@ -30,6 +30,7 @@ extern u32 epapr_ev_idle_start[];
>  #endif
>  
>  bool epapr_paravirt_enabled;
> +bool epapr_has_idle;

static

-Scott

^ permalink raw reply

* [PATCH][v4] powerpc: move epapr paravirt init of power_save to an initcall
From: Stuart Yoder @ 2014-04-30 23:34 UTC (permalink / raw)
  To: benh, scottwood; +Cc: linuxppc-dev, agraf, Stuart Yoder

From: Stuart Yoder <stuart.yoder@freescale.com>

some restructuring of epapr paravirt init resulted in
ppc_md.power_save being set, and then overwritten to
NULL during machine_init.  This patch splits the
initialization of ppc_md.power_save out into a postcore
init call.

Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---
-v3
   -changed approach slightly, set flag in the dt scanning
    code and just look at that flag in the initcall
-v4
   -made idle flag static

 arch/powerpc/kernel/epapr_paravirt.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
index 7898be9..8a7a62c 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -30,6 +30,7 @@ extern u32 epapr_ev_idle_start[];
 #endif
 
 bool epapr_paravirt_enabled;
+static bool epapr_has_idle;
 
 static int __init early_init_dt_scan_epapr(unsigned long node,
 					   const char *uname,
@@ -55,7 +56,7 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
 
 #if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
 	if (of_get_flat_dt_prop(node, "has-idle", NULL))
-		ppc_md.power_save = epapr_ev_idle;
+		epapr_has_idle = true;
 #endif
 
 	epapr_paravirt_enabled = true;
@@ -70,3 +71,12 @@ int __init epapr_paravirt_early_init(void)
 	return 0;
 }
 
+static int __init epapr_idle_init(void)
+{
+	if (epapr_has_idle)
+		ppc_md.power_save = epapr_ev_idle;
+
+	return 0;
+}
+
+postcore_initcall(epapr_idle_init);
-- 
1.7.9.7

^ permalink raw reply related

* Re: [PATCH 2/2] PCI/MSI: Phase out pci_enable_msi_block()
From: Bjorn Helgaas @ 2014-04-30 23:49 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-mips, linux-s390, linux-pci, x86, linux-kernel,
	linuxppc-dev
In-Reply-To: <20140414132834.GA9164@dhcp-26-207.brq.redhat.com>

On Mon, Apr 14, 2014 at 03:28:35PM +0200, Alexander Gordeev wrote:
> There are no users of pci_enable_msi_block() function have
> left. Obsolete it in favor of pci_enable_msi_range() and
> pci_enable_msi_exact() functions.

I mistakenly assumed this would have to wait because I thought there were
other pci_enable_msi_block() users that wouldn't be removed until the v3.16
merge window.  But I think I was wrong: I put your GenWQE patch in my tree,
and I think that was the last use, so I can just add this patch on top.

But I need a little help understanding the changelog:

> Up until now, when enabling MSI mode for a device a single
> successful call to arch_msi_check_device() was followed by
> a single call to arch_setup_msi_irqs() function.

I understand this part; the following two paths call
arch_msi_check_device() once and then arch_setup_msi_irqs() once:

  pci_enable_msi_block
    pci_msi_check_device
      arch_msi_check_device
    msi_capability_init
      arch_setup_msi_irqs

  pci_enable_msix
    pci_msi_check_device
      arch_msi_check_device
    msix_capability_init
      arch_setup_msi_irqs

> Yet, if arch_msi_check_device() returned success we should be
> able to call arch_setup_msi_irqs() multiple times - while it
> returns a number of MSI vectors that could have been allocated
> (a third state).

I don't know what you mean by "a third state."

Previously we only called arch_msi_check_device() once.  After your patch,
pci_enable_msi_range() can call it several times.  The only non-trivial
implementation of arch_msi_check_device() is in powerpc, and all the
ppc_md.msi_check_device() possibilities look safe to call multiple times.

After your patch, the pci_enable_msi_range() path can also call
arch_setup_msi_irqs() several times.  I don't see a problem with that --
even if the first call succeeds and allocates something, then a subsequent
call fails, I assume the allocations will be cleaned up when
msi_capability_init() calls free_msi_irqs().

> This update makes use of the assumption described above. It
> could have broke things had the architectures done any pre-
> allocations or switch to some state in a call to function
> arch_msi_check_device(). But because arch_msi_check_device()
> is expected stateless and MSI resources are allocated in a
> follow-up call to arch_setup_msi_irqs() we should be fine.

I guess you mean that your patch assumes arch_msi_check_device() is
stateless.  That looks like a safe assumption to me.

arch_setup_msi_irqs() is clearly not stateless, but I assume
free_msi_irqs() is enough to clean up any state if we fail.

Bjorn

> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: linux-mips@linux-mips.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: x86@kernel.org
> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/pci/msi.c   |   79 +++++++++++++++++++++-----------------------------
>  include/linux/pci.h |    5 +--
>  2 files changed, 34 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 955ab79..fc669ab 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -883,50 +883,6 @@ int pci_msi_vec_count(struct pci_dev *dev)
>  }
>  EXPORT_SYMBOL(pci_msi_vec_count);
>  
> -/**
> - * pci_enable_msi_block - configure device's MSI capability structure
> - * @dev: device to configure
> - * @nvec: number of interrupts to configure
> - *
> - * Allocate IRQs for a device with the MSI capability.
> - * This function returns a negative errno if an error occurs.  If it
> - * is unable to allocate the number of interrupts requested, it returns
> - * the number of interrupts it might be able to allocate.  If it successfully
> - * allocates at least the number of interrupts requested, it returns 0 and
> - * updates the @dev's irq member to the lowest new interrupt number; the
> - * other interrupt numbers allocated to this device are consecutive.
> - */
> -int pci_enable_msi_block(struct pci_dev *dev, int nvec)
> -{
> -	int status, maxvec;
> -
> -	if (dev->current_state != PCI_D0)
> -		return -EINVAL;
> -
> -	maxvec = pci_msi_vec_count(dev);
> -	if (maxvec < 0)
> -		return maxvec;
> -	if (nvec > maxvec)
> -		return maxvec;
> -
> -	status = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSI);
> -	if (status)
> -		return status;
> -
> -	WARN_ON(!!dev->msi_enabled);
> -
> -	/* Check whether driver already requested MSI-X irqs */
> -	if (dev->msix_enabled) {
> -		dev_info(&dev->dev, "can't enable MSI "
> -			 "(MSI-X already enabled)\n");
> -		return -EINVAL;
> -	}
> -
> -	status = msi_capability_init(dev, nvec);
> -	return status;
> -}
> -EXPORT_SYMBOL(pci_enable_msi_block);
> -
>  void pci_msi_shutdown(struct pci_dev *dev)
>  {
>  	struct msi_desc *desc;
> @@ -1132,14 +1088,45 @@ void pci_msi_init_pci_dev(struct pci_dev *dev)
>   **/
>  int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
>  {
> -	int nvec = maxvec;
> +	int nvec;
>  	int rc;
>  
> +	if (dev->current_state != PCI_D0)
> +		return -EINVAL;
> +
> +	WARN_ON(!!dev->msi_enabled);
> +
> +	/* Check whether driver already requested MSI-X irqs */
> +	if (dev->msix_enabled) {
> +		dev_info(&dev->dev,
> +			 "can't enable MSI (MSI-X already enabled)\n");
> +		return -EINVAL;
> +	}
> +
>  	if (maxvec < minvec)
>  		return -ERANGE;
>  
> +	nvec = pci_msi_vec_count(dev);
> +	if (nvec < 0)
> +		return nvec;
> +	else if (nvec < minvec)
> +		return -EINVAL;
> +	else if (nvec > maxvec)
> +		nvec = maxvec;
> +
> +	do {
> +		rc = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSI);
> +		if (rc < 0) {
> +			return rc;
> +		} else if (rc > 0) {
> +			if (rc < minvec)
> +				return -ENOSPC;
> +			nvec = rc;
> +		}
> +	} while (rc);
> +
>  	do {
> -		rc = pci_enable_msi_block(dev, nvec);
> +		rc = msi_capability_init(dev, nvec);
>  		if (rc < 0) {
>  			return rc;
>  		} else if (rc > 0) {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index aab57b4..499755e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1158,7 +1158,6 @@ struct msix_entry {
>  
>  #ifdef CONFIG_PCI_MSI
>  int pci_msi_vec_count(struct pci_dev *dev);
> -int pci_enable_msi_block(struct pci_dev *dev, int nvec);
>  void pci_msi_shutdown(struct pci_dev *dev);
>  void pci_disable_msi(struct pci_dev *dev);
>  int pci_msix_vec_count(struct pci_dev *dev);
> @@ -1188,8 +1187,6 @@ static inline int pci_enable_msix_exact(struct pci_dev *dev,
>  }
>  #else
>  static inline int pci_msi_vec_count(struct pci_dev *dev) { return -ENOSYS; }
> -static inline int pci_enable_msi_block(struct pci_dev *dev, int nvec)
> -{ return -ENOSYS; }
>  static inline void pci_msi_shutdown(struct pci_dev *dev) { }
>  static inline void pci_disable_msi(struct pci_dev *dev) { }
>  static inline int pci_msix_vec_count(struct pci_dev *dev) { return -ENOSYS; }
> @@ -1244,7 +1241,7 @@ static inline void pcie_set_ecrc_checking(struct pci_dev *dev) { }
>  static inline void pcie_ecrc_get_policy(char *str) { }
>  #endif
>  
> -#define pci_enable_msi(pdev)	pci_enable_msi_block(pdev, 1)
> +#define pci_enable_msi(pdev)	pci_enable_msi_exact(pdev, 1)
>  
>  #ifdef CONFIG_HT_IRQ
>  /* The functions a driver should call */
> -- 
> 1.7.7.6
> 
> -- 
> Regards,
> Alexander Gordeev
> agordeev@redhat.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2 0/4] KVM: PPC: Read guest instruction from kvmppc_get_last_inst()
From: Mihai Caraman @ 2014-05-01  0:45 UTC (permalink / raw)
  To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm

Read guest last instruction from kvmppc_get_last_inst() allowing the function
to fail in order to emulate again. On bookehv architecture search for
the physical address and kmap it, instead of using Load External PID (lwepx)
instruction. This fixes an infinite loop caused by lwepx's data TLB miss
exception handled in the host and the TODO for execute-but-not-read entries
and TLB eviction.

Mihai Caraman (4):
  KVM: PPC: e500mc: Revert "add load inst fixup"
  KVM: PPC: Book3e: Add TLBSEL/TSIZE defines for MAS0/1
  KVM: PPC: Alow kvmppc_get_last_inst() to fail
  KVM: PPC: Bookehv: Get vcpu's last instruction for emulation

 arch/powerpc/include/asm/kvm_book3s.h    | 26 ---------
 arch/powerpc/include/asm/kvm_booke.h     |  5 --
 arch/powerpc/include/asm/kvm_ppc.h       |  2 +
 arch/powerpc/include/asm/mmu-book3e.h    |  7 ++-
 arch/powerpc/kvm/book3s.c                | 32 +++++++++++
 arch/powerpc/kvm/book3s.h                |  1 +
 arch/powerpc/kvm/book3s_64_mmu_hv.c      | 16 ++----
 arch/powerpc/kvm/book3s_paired_singles.c | 42 ++++++++------
 arch/powerpc/kvm/book3s_pr.c             | 36 +++++++-----
 arch/powerpc/kvm/booke.c                 | 14 +++++
 arch/powerpc/kvm/booke.h                 |  3 +
 arch/powerpc/kvm/bookehv_interrupts.S    | 54 ++----------------
 arch/powerpc/kvm/e500_mmu_host.c         | 98 ++++++++++++++++++++++++++++++++
 arch/powerpc/kvm/emulate.c               | 18 ++++--
 arch/powerpc/kvm/powerpc.c               | 10 +++-
 15 files changed, 235 insertions(+), 129 deletions(-)

-- 
1.7.11.7

^ permalink raw reply

* [PATCH v2 2/4] KVM: PPC: Book3e: Add TLBSEL/TSIZE defines for MAS0/1
From: Mihai Caraman @ 2014-05-01  0:45 UTC (permalink / raw)
  To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
In-Reply-To: <1398905152-18091-1-git-send-email-mihai.caraman@freescale.com>

Add defines MAS0_GET_TLBSEL() and MAS1_GET_TSIZE() to Book3E.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
v2:
 - no change

 arch/powerpc/include/asm/mmu-book3e.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/mmu-book3e.h b/arch/powerpc/include/asm/mmu-book3e.h
index 901dac6..60a949a 100644
--- a/arch/powerpc/include/asm/mmu-book3e.h
+++ b/arch/powerpc/include/asm/mmu-book3e.h
@@ -40,7 +40,11 @@
 
 /* MAS registers bit definitions */
 
-#define MAS0_TLBSEL(x)		(((x) << 28) & 0x30000000)
+#define MAS0_TLBSEL_MASK	0x30000000
+#define MAS0_TLBSEL_SHIFT	28
+#define MAS0_TLBSEL(x)		(((x) << MAS0_TLBSEL_SHIFT) & MAS0_TLBSEL_MASK)
+#define MAS0_GET_TLBSEL(mas0)	(((mas0) & MAS0_TLBSEL_MASK) >> \
+			MAS0_TLBSEL_SHIFT)
 #define MAS0_ESEL_MASK		0x0FFF0000
 #define MAS0_ESEL_SHIFT		16
 #define MAS0_ESEL(x)		(((x) << MAS0_ESEL_SHIFT) & MAS0_ESEL_MASK)
@@ -58,6 +62,7 @@
 #define MAS1_TSIZE_MASK		0x00000f80
 #define MAS1_TSIZE_SHIFT	7
 #define MAS1_TSIZE(x)		(((x) << MAS1_TSIZE_SHIFT) & MAS1_TSIZE_MASK)
+#define MAS1_GET_TSIZE(mas1)	(((mas1) & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT)
 
 #define MAS2_EPN		(~0xFFFUL)
 #define MAS2_X0			0x00000040
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH v2 4/4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Mihai Caraman @ 2014-05-01  0:45 UTC (permalink / raw)
  To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
In-Reply-To: <1398905152-18091-1-git-send-email-mihai.caraman@freescale.com>

On bookehv vcpu's last instruction is read using load external pid
(lwepx) instruction. lwepx exceptions (DTLB_MISS, DSI and LRAT) need
to be handled by KVM. These exceptions originate from host state
(MSR[GS] = 0) which implies additional checks in DO_KVM macro (beside
the current MSR[GS] = 1) by looking at the Exception Syndrome Register
(ESR[EPID]) and the External PID Load Context Register (EPLC[EGS]).
Doing this on each Data TLB miss exception is obvious too intrusive
for the host.

Get rid of lwepx and acquire last instuction in kvmppc_get_last_inst()
by searching for the physical address and kmap it. This fixes an
infinite loop caused by lwepx's data TLB miss handled in the host
and the TODO for TLB eviction and execute-but-not-read entries.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
v2:
 - reworked patch description
 - used pr_* functions
 - addressed cosmetic feedback

 arch/powerpc/kvm/bookehv_interrupts.S | 37 ++++----------
 arch/powerpc/kvm/e500_mmu_host.c      | 93 ++++++++++++++++++++++++++++++++++-
 2 files changed, 101 insertions(+), 29 deletions(-)

diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
index 925da71..65eff4c 100644
--- a/arch/powerpc/kvm/bookehv_interrupts.S
+++ b/arch/powerpc/kvm/bookehv_interrupts.S
@@ -122,38 +122,14 @@
 1:
 
 	.if	\flags & NEED_EMU
-	/*
-	 * This assumes you have external PID support.
-	 * To support a bookehv CPU without external PID, you'll
-	 * need to look up the TLB entry and create a temporary mapping.
-	 *
-	 * FIXME: we don't currently handle if the lwepx faults.  PR-mode
-	 * booke doesn't handle it either.  Since Linux doesn't use
-	 * broadcast tlbivax anymore, the only way this should happen is
-	 * if the guest maps its memory execute-but-not-read, or if we
-	 * somehow take a TLB miss in the middle of this entry code and
-	 * evict the relevant entry.  On e500mc, all kernel lowmem is
-	 * bolted into TLB1 large page mappings, and we don't use
-	 * broadcast invalidates, so we should not take a TLB miss here.
-	 *
-	 * Later we'll need to deal with faults here.  Disallowing guest
-	 * mappings that are execute-but-not-read could be an option on
-	 * e500mc, but not on chips with an LRAT if it is used.
-	 */
-
-	mfspr	r3, SPRN_EPLC	/* will already have correct ELPID and EGS */
 	PPC_STL	r15, VCPU_GPR(R15)(r4)
 	PPC_STL	r16, VCPU_GPR(R16)(r4)
 	PPC_STL	r17, VCPU_GPR(R17)(r4)
 	PPC_STL	r18, VCPU_GPR(R18)(r4)
 	PPC_STL	r19, VCPU_GPR(R19)(r4)
-	mr	r8, r3
 	PPC_STL	r20, VCPU_GPR(R20)(r4)
-	rlwimi	r8, r6, EPC_EAS_SHIFT - MSR_IR_LG, EPC_EAS
 	PPC_STL	r21, VCPU_GPR(R21)(r4)
-	rlwimi	r8, r6, EPC_EPR_SHIFT - MSR_PR_LG, EPC_EPR
 	PPC_STL	r22, VCPU_GPR(R22)(r4)
-	rlwimi	r8, r10, EPC_EPID_SHIFT, EPC_EPID
 	PPC_STL	r23, VCPU_GPR(R23)(r4)
 	PPC_STL	r24, VCPU_GPR(R24)(r4)
 	PPC_STL	r25, VCPU_GPR(R25)(r4)
@@ -163,10 +139,15 @@
 	PPC_STL	r29, VCPU_GPR(R29)(r4)
 	PPC_STL	r30, VCPU_GPR(R30)(r4)
 	PPC_STL	r31, VCPU_GPR(R31)(r4)
-	mtspr	SPRN_EPLC, r8
-	isync
-	lwepx   r9, 0, r5
-	mtspr	SPRN_EPLC, r3
+
+	/*
+	 * We don't use external PID support. lwepx faults would need to be
+	 * handled by KVM and this implies aditional code in DO_KVM (for
+	 * DTB_MISS, DSI and LRAT) to check ESR[EPID] and EPLC[EGS] which
+	 * is too intrusive for the host. Get last instuction in
+	 * kvmppc_get_last_inst().
+	 */
+	li	r9, KVM_INST_FETCH_FAILED
 	stw	r9, VCPU_LAST_INST(r4)
 	.endif
 
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index fcccbb3..94b8be0 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -607,11 +607,102 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
 	}
 }
 
+#ifdef CONFIG_KVM_BOOKE_HV
+int kvmppc_ld_inst(struct kvm_vcpu *vcpu, u32 *instr)
+{
+	gva_t geaddr;
+	hpa_t addr;
+	hfn_t pfn;
+	hva_t eaddr;
+	u32 mas0, mas1, mas2, mas3;
+	u64 mas7_mas3;
+	struct page *page;
+	unsigned int addr_space, psize_shift;
+	bool pr;
+	unsigned long flags;
+
+	/* Search TLB for guest pc to get the real address */
+	geaddr = kvmppc_get_pc(vcpu);
+	addr_space = (vcpu->arch.shared->msr & MSR_IS) >> MSR_IR_LG;
+
+	local_irq_save(flags);
+	mtspr(SPRN_MAS6, (vcpu->arch.pid << MAS6_SPID_SHIFT) | addr_space);
+	mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
+	asm volatile("tlbsx 0, %[geaddr]\n" : :
+		     [geaddr] "r" (geaddr));
+	mtspr(SPRN_MAS5, 0);
+	mtspr(SPRN_MAS8, 0);
+	mas0 = mfspr(SPRN_MAS0);
+	mas1 = mfspr(SPRN_MAS1);
+	mas2 = mfspr(SPRN_MAS2);
+	mas3 = mfspr(SPRN_MAS3);
+	mas7_mas3 = (((u64) mfspr(SPRN_MAS7)) << 32) | mas3;
+	local_irq_restore(flags);
+
+	/*
+	 * If the TLB entry for guest pc was evicted, return to the guest.
+	 * There are high chances to find a valid TLB entry next time.
+	 */
+	if (!(mas1 & MAS1_VALID))
+		return EMULATE_AGAIN;
 
+	/*
+	 * Another thread may rewrite the TLB entry in parallel, don't
+	 * execute from the address if the execute permission is not set
+	 */
+	pr = vcpu->arch.shared->msr & MSR_PR;
+	if ((pr && !(mas3 & MAS3_UX)) || (!pr && !(mas3 & MAS3_SX))) {
+		pr_debug("Instuction emulation from a guest page\n"
+				"withot execute permission\n");
+		kvmppc_core_queue_program(vcpu, 0);
+		return EMULATE_AGAIN;
+	}
+
+	/*
+	 * We will map the real address through a cacheable page, so we will
+	 * not support cache-inhibited guest pages. Fortunately emulated
+	 * instructions should not live there.
+	 */
+	if (mas2 & MAS2_I) {
+		pr_debug("Instuction emulation from cache-inhibited\n"
+				"guest pages is not supported\n");
+		return EMULATE_FAIL;
+	}
+
+	/* Get page size */
+	psize_shift = MAS1_GET_TSIZE(mas1) + 10;
+
+	/* Map a page and get guest's instruction */
+	addr = (mas7_mas3 & (~0ULL << psize_shift)) |
+	       (geaddr & ((1ULL << psize_shift) - 1ULL));
+	pfn = addr >> PAGE_SHIFT;
+
+	/* Guard us against emulation from devices area */
+	if (unlikely(!page_is_ram(pfn))) {
+		pr_debug("Instruction emulation from non-RAM host\n"
+				"pages is not supported\n");
+		return EMULATE_FAIL;
+	}
+
+	if (unlikely(!pfn_valid(pfn))) {
+		pr_debug("Invalid frame number\n");
+		return EMULATE_FAIL;
+	}
+
+	page = pfn_to_page(pfn);
+	eaddr = (unsigned long)kmap_atomic(page);
+	eaddr |= addr & ~PAGE_MASK;
+	*instr = *(u32 *)eaddr;
+	kunmap_atomic((u32 *)eaddr);
+
+	return EMULATE_DONE;
+}
+#else
 int kvmppc_ld_inst(struct kvm_vcpu *vcpu, u32 *instr)
 {
 	return EMULATE_FAIL;
-}
+};
+#endif
 
 /************* MMU Notifiers *************/
 
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH v2 3/4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: Mihai Caraman @ 2014-05-01  0:45 UTC (permalink / raw)
  To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
In-Reply-To: <1398905152-18091-1-git-send-email-mihai.caraman@freescale.com>

On book3e, guest last instruction was read on the exist path using load
external pid (lwepx) dedicated instruction. lwepx failures have to be
handled by KVM and this would require additional checks in DO_KVM hooks
(beside MSR[GS] = 1). However extra checks on host fast path are commonly
considered too intrusive.

This patch lay down the path for an altenative solution, by allowing
kvmppc_get_lat_inst() function to fail. Booke architectures may choose
to read guest instruction from kvmppc_ld_inst() and to instruct the
emulation layer either to fail or to emulate again.

emulation_result enum defintion is not accesible from book3 asm headers.
Move kvmppc_get_last_inst() definitions that require emulation_result
to source files.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
v2:
 - integrated kvmppc_get_last_inst() in book3s code and checked build
 - addressed cosmetic feedback
 - please validate book3s functionality!

 arch/powerpc/include/asm/kvm_book3s.h    | 26 --------------------
 arch/powerpc/include/asm/kvm_booke.h     |  5 ----
 arch/powerpc/include/asm/kvm_ppc.h       |  2 ++
 arch/powerpc/kvm/book3s.c                | 32 ++++++++++++++++++++++++
 arch/powerpc/kvm/book3s.h                |  1 +
 arch/powerpc/kvm/book3s_64_mmu_hv.c      | 16 +++---------
 arch/powerpc/kvm/book3s_paired_singles.c | 42 ++++++++++++++++++++------------
 arch/powerpc/kvm/book3s_pr.c             | 36 +++++++++++++++++----------
 arch/powerpc/kvm/booke.c                 | 14 +++++++++++
 arch/powerpc/kvm/booke.h                 |  3 +++
 arch/powerpc/kvm/e500_mmu_host.c         |  7 ++++++
 arch/powerpc/kvm/emulate.c               | 18 +++++++++-----
 arch/powerpc/kvm/powerpc.c               | 10 ++++++--
 13 files changed, 132 insertions(+), 80 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index bb1e38a..e2a89f3 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -273,32 +273,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
 	return (vcpu->arch.shared->msr & MSR_LE) != (MSR_KERNEL & MSR_LE);
 }
 
-static inline u32 kvmppc_get_last_inst_internal(struct kvm_vcpu *vcpu, ulong pc)
-{
-	/* Load the instruction manually if it failed to do so in the
-	 * exit path */
-	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
-		kvmppc_ld(vcpu, &pc, sizeof(u32), &vcpu->arch.last_inst, false);
-
-	return kvmppc_need_byteswap(vcpu) ? swab32(vcpu->arch.last_inst) :
-		vcpu->arch.last_inst;
-}
-
-static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
-{
-	return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu));
-}
-
-/*
- * Like kvmppc_get_last_inst(), but for fetching a sc instruction.
- * Because the sc instruction sets SRR0 to point to the following
- * instruction, we have to fetch from pc - 4.
- */
-static inline u32 kvmppc_get_last_sc(struct kvm_vcpu *vcpu)
-{
-	return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu) - 4);
-}
-
 static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)
 {
 	return vcpu->arch.fault_dar;
diff --git a/arch/powerpc/include/asm/kvm_booke.h b/arch/powerpc/include/asm/kvm_booke.h
index 80d46b5..6db1ca0 100644
--- a/arch/powerpc/include/asm/kvm_booke.h
+++ b/arch/powerpc/include/asm/kvm_booke.h
@@ -69,11 +69,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
 	return false;
 }
 
-static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
-{
-	return vcpu->arch.last_inst;
-}
-
 static inline void kvmppc_set_ctr(struct kvm_vcpu *vcpu, ulong val)
 {
 	vcpu->arch.ctr = val;
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 4096f16..6e7c358 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -72,6 +72,8 @@ extern int kvmppc_sanity_check(struct kvm_vcpu *vcpu);
 extern int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu);
 extern void kvmppc_subarch_vcpu_uninit(struct kvm_vcpu *vcpu);
 
+extern int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, u32 *inst);
+
 /* Core-specific hooks */
 
 extern void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 gvaddr, gpa_t gpaddr,
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 94e597e..3553735 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -463,6 +463,38 @@ mmio:
 }
 EXPORT_SYMBOL_GPL(kvmppc_ld);
 
+int kvmppc_get_last_inst_internal(struct kvm_vcpu *vcpu, ulong pc, u32 *inst)
+{
+	int ret = EMULATE_DONE;
+
+	/* Load the instruction manually if it failed to do so in the
+	 * exit path */
+	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
+		ret = kvmppc_ld(vcpu, &pc, sizeof(u32), &vcpu->arch.last_inst,
+			false);
+
+	*inst = kvmppc_need_byteswap(vcpu) ? swab32(vcpu->arch.last_inst) :
+		vcpu->arch.last_inst;
+
+	return ret;
+}
+
+int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, u32 *inst)
+{
+	return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu), inst);
+}
+
+/*
+ * Like kvmppc_get_last_inst(), but for fetching a sc instruction.
+ * Because the sc instruction sets SRR0 to point to the following
+ * instruction, we have to fetch from pc - 4.
+ */
+int kvmppc_get_last_sc(struct kvm_vcpu *vcpu, u32 *inst)
+{
+	return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu) - 4,
+		inst);
+}
+
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
 	return 0;
diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
index 4bf956c..d85839d 100644
--- a/arch/powerpc/kvm/book3s.h
+++ b/arch/powerpc/kvm/book3s.h
@@ -30,5 +30,6 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu,
 					int sprn, ulong *spr_val);
 extern int kvmppc_book3s_init_pr(void);
 extern void kvmppc_book3s_exit_pr(void);
+extern int kvmppc_get_last_sc(struct kvm_vcpu *vcpu, u32 *inst);
 
 #endif
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index fb25ebc..7ffcc24 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -541,21 +541,13 @@ static int instruction_is_store(unsigned int instr)
 static int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
 				  unsigned long gpa, gva_t ea, int is_store)
 {
-	int ret;
 	u32 last_inst;
-	unsigned long srr0 = kvmppc_get_pc(vcpu);
 
-	/* We try to load the last instruction.  We don't let
-	 * emulate_instruction do it as it doesn't check what
-	 * kvmppc_ld returns.
+	/*
 	 * If we fail, we just return to the guest and try executing it again.
 	 */
-	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED) {
-		ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
-		if (ret != EMULATE_DONE || last_inst == KVM_INST_FETCH_FAILED)
-			return RESUME_GUEST;
-		vcpu->arch.last_inst = last_inst;
-	}
+	if (kvmppc_get_last_inst(vcpu, &last_inst) != EMULATE_DONE)
+		return RESUME_GUEST;
 
 	/*
 	 * WARNING: We do not know for sure whether the instruction we just
@@ -569,7 +561,7 @@ static int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
 	 * we just return and retry the instruction.
 	 */
 
-	if (instruction_is_store(kvmppc_get_last_inst(vcpu)) != !!is_store)
+	if (instruction_is_store(last_inst) != !!is_store)
 		return RESUME_GUEST;
 
 	/*
diff --git a/arch/powerpc/kvm/book3s_paired_singles.c b/arch/powerpc/kvm/book3s_paired_singles.c
index c1abd95..9a6e565 100644
--- a/arch/powerpc/kvm/book3s_paired_singles.c
+++ b/arch/powerpc/kvm/book3s_paired_singles.c
@@ -637,26 +637,36 @@ static int kvmppc_ps_one_in(struct kvm_vcpu *vcpu, bool rc,
 
 int kvmppc_emulate_paired_single(struct kvm_run *run, struct kvm_vcpu *vcpu)
 {
-	u32 inst = kvmppc_get_last_inst(vcpu);
-	enum emulation_result emulated = EMULATE_DONE;
-
-	int ax_rd = inst_get_field(inst, 6, 10);
-	int ax_ra = inst_get_field(inst, 11, 15);
-	int ax_rb = inst_get_field(inst, 16, 20);
-	int ax_rc = inst_get_field(inst, 21, 25);
-	short full_d = inst_get_field(inst, 16, 31);
-
-	u64 *fpr_d = &VCPU_FPR(vcpu, ax_rd);
-	u64 *fpr_a = &VCPU_FPR(vcpu, ax_ra);
-	u64 *fpr_b = &VCPU_FPR(vcpu, ax_rb);
-	u64 *fpr_c = &VCPU_FPR(vcpu, ax_rc);
-
-	bool rcomp = (inst & 1) ? true : false;
-	u32 cr = kvmppc_get_cr(vcpu);
+	u32 inst;
+	enum emulation_result emulated;
+	int ax_rd, ax_ra, ax_rb, ax_rc;
+	short full_d;
+	u64 *fpr_d, *fpr_a, *fpr_b, *fpr_c;
+
+	bool rcomp;
+	u32 cr;
 #ifdef DEBUG
 	int i;
 #endif
 
+	emulated = kvmppc_get_last_inst(vcpu, &inst);
+	if (emulated != EMULATE_DONE)
+		return emulated;
+
+	ax_rd = inst_get_field(inst, 6, 10);
+	ax_ra = inst_get_field(inst, 11, 15);
+	ax_rb = inst_get_field(inst, 16, 20);
+	ax_rc = inst_get_field(inst, 21, 25);
+	full_d = inst_get_field(inst, 16, 31);
+
+	fpr_d = &VCPU_FPR(vcpu, ax_rd);
+	fpr_a = &VCPU_FPR(vcpu, ax_ra);
+	fpr_b = &VCPU_FPR(vcpu, ax_rb);
+	fpr_c = &VCPU_FPR(vcpu, ax_rc);
+
+	rcomp = (inst & 1) ? true : false;
+	cr = kvmppc_get_cr(vcpu);
+
 	if (!kvmppc_inst_is_paired_single(vcpu, inst))
 		return EMULATE_FAIL;
 
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index c5c052a..b7fffd1 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -608,12 +608,9 @@ void kvmppc_giveup_ext(struct kvm_vcpu *vcpu, ulong msr)
 
 static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
 {
-	ulong srr0 = kvmppc_get_pc(vcpu);
-	u32 last_inst = kvmppc_get_last_inst(vcpu);
-	int ret;
+	u32 last_inst;
 
-	ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
-	if (ret == -ENOENT) {
+	if (kvmppc_get_last_inst(vcpu, &last_inst) == -ENOENT) {
 		ulong msr = vcpu->arch.shared->msr;
 
 		msr = kvmppc_set_field(msr, 33, 33, 1);
@@ -867,15 +864,18 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
 	{
 		enum emulation_result er;
 		ulong flags;
+		u32 last_inst;
 
 program_interrupt:
 		flags = vcpu->arch.shadow_srr1 & 0x1f0000ull;
+		kvmppc_get_last_inst(vcpu, &last_inst);
 
 		if (vcpu->arch.shared->msr & MSR_PR) {
 #ifdef EXIT_DEBUG
-			printk(KERN_INFO "Userspace triggered 0x700 exception at 0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
+			pr_info("Userspace triggered 0x700 exception at\n"
+			    "0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), last_inst);
 #endif
-			if ((kvmppc_get_last_inst(vcpu) & 0xff0007ff) !=
+			if ((last_inst & 0xff0007ff) !=
 			    (INS_DCBZ & 0xfffffff7)) {
 				kvmppc_core_queue_program(vcpu, flags);
 				r = RESUME_GUEST;
@@ -894,7 +894,7 @@ program_interrupt:
 			break;
 		case EMULATE_FAIL:
 			printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n",
-			       __func__, kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
+			       __func__, kvmppc_get_pc(vcpu), last_inst);
 			kvmppc_core_queue_program(vcpu, flags);
 			r = RESUME_GUEST;
 			break;
@@ -911,8 +911,12 @@ program_interrupt:
 		break;
 	}
 	case BOOK3S_INTERRUPT_SYSCALL:
+	{
+		u32 last_sc;
+
+		kvmppc_get_last_sc(vcpu, &last_sc);
 		if (vcpu->arch.papr_enabled &&
-		    (kvmppc_get_last_sc(vcpu) == 0x44000022) &&
+		    (last_sc == 0x44000022) &&
 		    !(vcpu->arch.shared->msr & MSR_PR)) {
 			/* SC 1 papr hypercalls */
 			ulong cmd = kvmppc_get_gpr(vcpu, 3);
@@ -957,6 +961,7 @@ program_interrupt:
 			r = RESUME_GUEST;
 		}
 		break;
+	}
 	case BOOK3S_INTERRUPT_FP_UNAVAIL:
 	case BOOK3S_INTERRUPT_ALTIVEC:
 	case BOOK3S_INTERRUPT_VSX:
@@ -985,15 +990,20 @@ program_interrupt:
 		break;
 	}
 	case BOOK3S_INTERRUPT_ALIGNMENT:
+	{
+		u32 last_inst;
+
 		if (kvmppc_read_inst(vcpu) == EMULATE_DONE) {
-			vcpu->arch.shared->dsisr = kvmppc_alignment_dsisr(vcpu,
-				kvmppc_get_last_inst(vcpu));
-			vcpu->arch.shared->dar = kvmppc_alignment_dar(vcpu,
-				kvmppc_get_last_inst(vcpu));
+			kvmppc_get_last_inst(vcpu, &last_inst);
+			vcpu->arch.shared->dsisr =
+				kvmppc_alignment_dsisr(vcpu, last_inst);
+			vcpu->arch.shared->dar =
+				kvmppc_alignment_dar(vcpu, last_inst);
 			kvmppc_book3s_queue_irqprio(vcpu, exit_nr);
 		}
 		r = RESUME_GUEST;
 		break;
+	}
 	case BOOK3S_INTERRUPT_MACHINE_CHECK:
 	case BOOK3S_INTERRUPT_TRACE:
 		kvmppc_book3s_queue_irqprio(vcpu, exit_nr);
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index ab62109..df25db0 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -752,6 +752,9 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
 		 * they were actually modified by emulation. */
 		return RESUME_GUEST_NV;
 
+	case EMULATE_AGAIN:
+		return RESUME_GUEST;
+
 	case EMULATE_DO_DCR:
 		run->exit_reason = KVM_EXIT_DCR;
 		return RESUME_HOST;
@@ -1911,6 +1914,17 @@ void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu)
 	vcpu->kvm->arch.kvm_ops->vcpu_put(vcpu);
 }
 
+int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, u32 *inst)
+{
+	int result = EMULATE_DONE;
+
+	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
+		result = kvmppc_ld_inst(vcpu, &vcpu->arch.last_inst);
+	*inst = vcpu->arch.last_inst;
+
+	return result;
+}
+
 int __init kvmppc_booke_init(void)
 {
 #ifndef CONFIG_KVM_BOOKE_HV
diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h
index b632cd3..d07a154 100644
--- a/arch/powerpc/kvm/booke.h
+++ b/arch/powerpc/kvm/booke.h
@@ -80,6 +80,9 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val);
 int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val);
 
+
+extern int kvmppc_ld_inst(struct kvm_vcpu *vcpu, u32 *instr);
+
 /* low-level asm code to transfer guest state */
 void kvmppc_load_guest_spe(struct kvm_vcpu *vcpu);
 void kvmppc_save_guest_spe(struct kvm_vcpu *vcpu);
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index dd2cc03..fcccbb3 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -34,6 +34,7 @@
 #include "e500.h"
 #include "timing.h"
 #include "e500_mmu_host.h"
+#include "booke.h"
 
 #include "trace_booke.h"
 
@@ -606,6 +607,12 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
 	}
 }
 
+
+int kvmppc_ld_inst(struct kvm_vcpu *vcpu, u32 *instr)
+{
+	return EMULATE_FAIL;
+}
+
 /************* MMU Notifiers *************/
 
 int kvm_unmap_hva(struct kvm *kvm, unsigned long hva)
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index c2b887b..e281d32 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -224,19 +224,25 @@ static int kvmppc_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
  * from opcode tables in the future. */
 int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 {
-	u32 inst = kvmppc_get_last_inst(vcpu);
-	int ra = get_ra(inst);
-	int rs = get_rs(inst);
-	int rt = get_rt(inst);
-	int sprn = get_sprn(inst);
-	enum emulation_result emulated = EMULATE_DONE;
+	u32 inst;
+	int ra, rs, rt, sprn;
+	enum emulation_result emulated;
 	int advance = 1;
 
 	/* this default type might be overwritten by subcategories */
 	kvmppc_set_exit_type(vcpu, EMULATED_INST_EXITS);
 
+	emulated = kvmppc_get_last_inst(vcpu, &inst);
+	if (emulated != EMULATE_DONE)
+		return emulated;
+
 	pr_debug("Emulating opcode %d / %d\n", get_op(inst), get_xop(inst));
 
+	ra = get_ra(inst);
+	rs = get_rs(inst);
+	rt = get_rt(inst);
+	sprn = get_sprn(inst);
+
 	switch (get_op(inst)) {
 	case OP_TRAP:
 #ifdef CONFIG_PPC_BOOK3S
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 3cf541a..fdc1ee3 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -220,6 +220,9 @@ int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu)
 		 * actually modified. */
 		r = RESUME_GUEST_NV;
 		break;
+	case EMULATE_AGAIN:
+		r = RESUME_GUEST;
+		break;
 	case EMULATE_DO_MMIO:
 		run->exit_reason = KVM_EXIT_MMIO;
 		/* We must reload nonvolatiles because "update" load/store
@@ -229,11 +232,14 @@ int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu)
 		r = RESUME_HOST_NV;
 		break;
 	case EMULATE_FAIL:
+	{
+		u32 last_inst;
+		kvmppc_get_last_inst(vcpu, &last_inst);
 		/* XXX Deliver Program interrupt to guest. */
-		printk(KERN_EMERG "%s: emulation failed (%08x)\n", __func__,
-		       kvmppc_get_last_inst(vcpu));
+		pr_emerg("%s: emulation failed (%08x)\n", __func__, last_inst);
 		r = RESUME_HOST;
 		break;
+	}
 	default:
 		WARN_ON(1);
 		r = RESUME_GUEST;
-- 
1.7.11.7

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox