Patchwork ppc: initialize GPRs as per epapr

login
register
mail settings
Submitter Bharat Bhushan
Date April 26, 2013, 6:17 a.m.
Message ID <1366957045-21133-1-git-send-email-Bharat.Bhushan@freescale.com>
Download mbox | patch
Permalink /patch/239683/
State New
Headers show

Comments

Bharat Bhushan - April 26, 2013, 6:17 a.m.
ePAPR defines the initial values of cpu registers. This patch initialize
the GPRs as per ePAPR specification.

This resolves the issue of guest reboot/reset (guest hang on reboot).

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---
 hw/ppc/e500.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Alexander Graf - April 26, 2013, 6:21 a.m.
On 26.04.2013, at 08:17, Bharat Bhushan wrote:

> ePAPR defines the initial values of cpu registers. This patch initialize
> the GPRs as per ePAPR specification.
> 
> This resolves the issue of guest reboot/reset (guest hang on reboot).

Why does it hang only on reboot, not on initial bootup?

> 
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> ---
> hw/ppc/e500.c |    7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index c1bdb6b..a47f976 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -37,6 +37,7 @@
> #include "qemu/host-utils.h"
> #include "hw/pci-host/ppce500.h"
> 
> +#define EPAPR_MAGIC                (0x45504150)
> #define BINARY_DEVICE_TREE_FILE    "mpc8544ds.dtb"
> #define UIMAGE_LOAD_BASE           0
> #define DTC_LOAD_PAD               0x1800000
> @@ -444,6 +445,12 @@ static void ppce500_cpu_reset(void *opaque)

Does ePAPR mention anything wrt GPR state of secondary CPUs?

>     cs->halted = 0;
>     env->gpr[1] = (16<<20) - 8;
>     env->gpr[3] = bi->dt_base;
> +    env->gpr[4] = 0;
> +    env->gpr[5] = 0;
> +    env->gpr[6] = EPAPR_MAGIC;
> +    env->gpr[7] = (64 * 1024 * 1024);

What is this?


Alex

> +    env->gpr[8] = 0;
> +    env->gpr[9] = 0;
>     env->nip = bi->entry;
>     mmubooke_create_initial_mapping(env);
> }
> -- 
> 1.7.0.4
> 
>
Bharat Bhushan - April 26, 2013, 6:51 a.m.
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Friday, April 26, 2013 11:51 AM
> To: Bhushan Bharat-R65777
> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; Wood Scott-B07421; Bhushan
> Bharat-R65777; Yoder Stuart-B08248
> Subject: Re: [PATCH] ppc: initialize GPRs as per epapr
> 
> 
> On 26.04.2013, at 08:17, Bharat Bhushan wrote:
> 
> > ePAPR defines the initial values of cpu registers. This patch
> > initialize the GPRs as per ePAPR specification.
> >
> > This resolves the issue of guest reboot/reset (guest hang on reboot).
> 
> Why does it hang only on reboot, not on initial bootup?

may be memory pointed by env pointer are zero initialized initially.
Reboot also not always hangs. I have seen reboot mostly working on e500v2/e500mc and mostly hanging on e5500.

> 
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> > ---
> > hw/ppc/e500.c |    7 +++++++
> > 1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index c1bdb6b..a47f976
> > 100644
> > --- a/hw/ppc/e500.c
> > +++ b/hw/ppc/e500.c
> > @@ -37,6 +37,7 @@
> > #include "qemu/host-utils.h"
> > #include "hw/pci-host/ppce500.h"
> >
> > +#define EPAPR_MAGIC                (0x45504150)
> > #define BINARY_DEVICE_TREE_FILE    "mpc8544ds.dtb"
> > #define UIMAGE_LOAD_BASE           0
> > #define DTC_LOAD_PAD               0x1800000
> > @@ -444,6 +445,12 @@ static void ppce500_cpu_reset(void *opaque)
> 
> Does ePAPR mention anything wrt GPR state of secondary CPUs?

Yes, I think we handle this in hw/ppc/ppce500_spin.c

> 
> >     cs->halted = 0;
> >     env->gpr[1] = (16<<20) - 8;
> >     env->gpr[3] = bi->dt_base;
> > +    env->gpr[4] = 0;
> > +    env->gpr[5] = 0;
> > +    env->gpr[6] = EPAPR_MAGIC;
> > +    env->gpr[7] = (64 * 1024 * 1024);
> 
> What is this?

Size of initial TLB ( should be big enough to cover kernel handler). I do not see ePAPR defines any value, I set this to 64M.

-Bharat

> 
> 
> Alex
> 
> > +    env->gpr[8] = 0;
> > +    env->gpr[9] = 0;
> >     env->nip = bi->entry;
> >     mmubooke_create_initial_mapping(env);
> > }
> > --
> > 1.7.0.4
> >
> >
>
Alexander Graf - April 26, 2013, 6:59 a.m.
On 26.04.2013, at 08:51, Bhushan Bharat-R65777 wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Friday, April 26, 2013 11:51 AM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; Wood Scott-B07421; Bhushan
>> Bharat-R65777; Yoder Stuart-B08248
>> Subject: Re: [PATCH] ppc: initialize GPRs as per epapr
>> 
>> 
>> On 26.04.2013, at 08:17, Bharat Bhushan wrote:
>> 
>>> ePAPR defines the initial values of cpu registers. This patch
>>> initialize the GPRs as per ePAPR specification.
>>> 
>>> This resolves the issue of guest reboot/reset (guest hang on reboot).
>> 
>> Why does it hang only on reboot, not on initial bootup?
> 
> may be memory pointed by env pointer are zero initialized initially.
> Reboot also not always hangs. I have seen reboot mostly working on e500v2/e500mc and mostly hanging on e5500.

Yes, they're all zero initialized. I'm surprised that got things working.

> 
>> 
>>> 
>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
>>> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
>>> ---
>>> hw/ppc/e500.c |    7 +++++++
>>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index c1bdb6b..a47f976
>>> 100644
>>> --- a/hw/ppc/e500.c
>>> +++ b/hw/ppc/e500.c
>>> @@ -37,6 +37,7 @@
>>> #include "qemu/host-utils.h"
>>> #include "hw/pci-host/ppce500.h"
>>> 
>>> +#define EPAPR_MAGIC                (0x45504150)
>>> #define BINARY_DEVICE_TREE_FILE    "mpc8544ds.dtb"
>>> #define UIMAGE_LOAD_BASE           0
>>> #define DTC_LOAD_PAD               0x1800000
>>> @@ -444,6 +445,12 @@ static void ppce500_cpu_reset(void *opaque)
>> 
>> Does ePAPR mention anything wrt GPR state of secondary CPUs?
> 
> Yes, I think we handle this in hw/ppc/ppce500_spin.c
> 
>> 
>>>    cs->halted = 0;
>>>    env->gpr[1] = (16<<20) - 8;
>>>    env->gpr[3] = bi->dt_base;
>>> +    env->gpr[4] = 0;
>>> +    env->gpr[5] = 0;
>>> +    env->gpr[6] = EPAPR_MAGIC;
>>> +    env->gpr[7] = (64 * 1024 * 1024);
>> 
>> What is this?
> 
> Size of initial TLB ( should be big enough to cover kernel handler). I do not see ePAPR defines any value, I set this to 64M.

It's dynamic. Please set it to the actual size of the initial TLB mapping we create.


Alex
David Gibson - April 26, 2013, 11:58 a.m.
On Fri, Apr 26, 2013 at 08:21:24AM +0200, Alexander Graf wrote:
> 
> On 26.04.2013, at 08:17, Bharat Bhushan wrote:
> 
> > ePAPR defines the initial values of cpu registers. This patch initialize
> > the GPRs as per ePAPR specification.
> > 
> > This resolves the issue of guest reboot/reset (guest hang on reboot).
> 
> Why does it hang only on reboot, not on initial bootup?
> 
> > 
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> > ---
> > hw/ppc/e500.c |    7 +++++++
> > 1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> > index c1bdb6b..a47f976 100644
> > --- a/hw/ppc/e500.c
> > +++ b/hw/ppc/e500.c
> > @@ -37,6 +37,7 @@
> > #include "qemu/host-utils.h"
> > #include "hw/pci-host/ppce500.h"
> > 
> > +#define EPAPR_MAGIC                (0x45504150)
> > #define BINARY_DEVICE_TREE_FILE    "mpc8544ds.dtb"
> > #define UIMAGE_LOAD_BASE           0
> > #define DTC_LOAD_PAD               0x1800000
> > @@ -444,6 +445,12 @@ static void ppce500_cpu_reset(void *opaque)
> 
> Does ePAPR mention anything wrt GPR state of secondary CPUs?

Yes and no.  The entry point state for secondary CPUs depends on the
"enable-method" used to start the CPU.  The spin-table enable method
defined in ePAPR gives some information on GPR state, although the
constraints are much weaker than for the boot cpu.  Platform specific
enable-methods would have to define their own entry point requirements.
Scott Wood - April 26, 2013, 5:06 p.m.
On 04/26/2013 01:59:10 AM, Alexander Graf wrote:
> 
> On 26.04.2013, at 08:51, Bhushan Bharat-R65777 wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:agraf@suse.de]
> >> Sent: Friday, April 26, 2013 11:51 AM
> >> To: Bhushan Bharat-R65777
> >> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; Wood Scott-B07421;  
> Bhushan
> >> Bharat-R65777; Yoder Stuart-B08248
> >> Subject: Re: [PATCH] ppc: initialize GPRs as per epapr
> >>
> >>
> >> On 26.04.2013, at 08:17, Bharat Bhushan wrote:
> >>
> >>> ePAPR defines the initial values of cpu registers. This patch
> >>> initialize the GPRs as per ePAPR specification.
> >>>
> >>> This resolves the issue of guest reboot/reset (guest hang on  
> reboot).
> >>
> >> Why does it hang only on reboot, not on initial bootup?
> >
> > may be memory pointed by env pointer are zero initialized initially.
> > Reboot also not always hangs. I have seen reboot mostly working on  
> e500v2/e500mc and mostly hanging on e5500.
> 
> Yes, they're all zero initialized. I'm surprised that got things  
> working.

head_64.S assumes it's being booted from Open Firmware if r5 is  
non-zero -- this is why ePAPR requires r5 to be zero.

-Scott

Patch

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index c1bdb6b..a47f976 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -37,6 +37,7 @@ 
 #include "qemu/host-utils.h"
 #include "hw/pci-host/ppce500.h"
 
+#define EPAPR_MAGIC                (0x45504150)
 #define BINARY_DEVICE_TREE_FILE    "mpc8544ds.dtb"
 #define UIMAGE_LOAD_BASE           0
 #define DTC_LOAD_PAD               0x1800000
@@ -444,6 +445,12 @@  static void ppce500_cpu_reset(void *opaque)
     cs->halted = 0;
     env->gpr[1] = (16<<20) - 8;
     env->gpr[3] = bi->dt_base;
+    env->gpr[4] = 0;
+    env->gpr[5] = 0;
+    env->gpr[6] = EPAPR_MAGIC;
+    env->gpr[7] = (64 * 1024 * 1024);
+    env->gpr[8] = 0;
+    env->gpr[9] = 0;
     env->nip = bi->entry;
     mmubooke_create_initial_mapping(env);
 }