| Message ID | 1459850484-28082-1-git-send-email-b18965@freescale.com |
|---|---|
| State | Rejected |
| Delegated to: | York Sun |
| Headers | show |
On 04/05/2016 03:10 AM, Alison Wang wrote: > For LS1021A Secure Boot, SPARE2 register is used and modified by the > IBR. To avoid the conflict, SPARE4 is used instead of SPARE2 to store > the entry point of kernel. This patch is to get the entry point of > kernel from SPARE4 instead of SPARE2. > > Signed-off-by: Alison Wang <alison.wang@nxp.com> > --- > board/freescale/common/arm_sleep.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/board/freescale/common/arm_sleep.c b/board/freescale/common/arm_sleep.c > index 71ed15e..6d967f0 100644 > --- a/board/freescale/common/arm_sleep.c > +++ b/board/freescale/common/arm_sleep.c > @@ -88,7 +88,7 @@ int fsl_dp_resume(void) > dp_resume_prepare(); > > /* Get the entry address and jump to kernel */ > - start_addr = in_le32(&scfg->sparecr[1]); > + start_addr = in_le32(&scfg->sparecr[3]); > debug("Entry address is 0x%08x\n", start_addr); > kernel_resume = (void (*)(void))start_addr; > secure_ram_addr(_do_nonsec_entry)(kernel_resume, 0, 0, 0); > Alison, Does this change need to be in sync with Kernel change? York
On 04/05/2016 05:11 AM, Alison Wang wrote: > For LS1021A Secure Boot, SPARE2 register is used and modified by the > IBR. To avoid the conflict, SPARE4 is used instead of SPARE2 to store > the entry point of kernel. This patch is to get the entry point of > kernel from SPARE4 instead of SPARE2. > > Signed-off-by: Alison Wang <alison.wang@nxp.com> > --- > board/freescale/common/arm_sleep.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/board/freescale/common/arm_sleep.c b/board/freescale/common/arm_sleep.c > index 71ed15e..6d967f0 100644 > --- a/board/freescale/common/arm_sleep.c > +++ b/board/freescale/common/arm_sleep.c > @@ -88,7 +88,7 @@ int fsl_dp_resume(void) > dp_resume_prepare(); > > /* Get the entry address and jump to kernel */ > - start_addr = in_le32(&scfg->sparecr[1]); > + start_addr = in_le32(&scfg->sparecr[3]); > debug("Entry address is 0x%08x\n", start_addr); > kernel_resume = (void (*)(void))start_addr; > secure_ram_addr(_do_nonsec_entry)(kernel_resume, 0, 0, 0); > Where does this get written? -Scott
Hi, York and Scott, > On 04/05/2016 05:11 AM, Alison Wang wrote: > > For LS1021A Secure Boot, SPARE2 register is used and modified by the > > IBR. To avoid the conflict, SPARE4 is used instead of SPARE2 to store > > the entry point of kernel. This patch is to get the entry point of > > kernel from SPARE4 instead of SPARE2. > > > > Signed-off-by: Alison Wang <alison.wang@nxp.com> > > --- > > board/freescale/common/arm_sleep.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/board/freescale/common/arm_sleep.c > > b/board/freescale/common/arm_sleep.c > > index 71ed15e..6d967f0 100644 > > --- a/board/freescale/common/arm_sleep.c > > +++ b/board/freescale/common/arm_sleep.c > > @@ -88,7 +88,7 @@ int fsl_dp_resume(void) > > dp_resume_prepare(); > > > > /* Get the entry address and jump to kernel */ > > - start_addr = in_le32(&scfg->sparecr[1]); > > + start_addr = in_le32(&scfg->sparecr[3]); > > debug("Entry address is 0x%08x\n", start_addr); > > kernel_resume = (void (*)(void))start_addr; > > secure_ram_addr(_do_nonsec_entry)(kernel_resume, 0, 0, 0); > > > Alison, > > Does this change need to be in sync with Kernel change? > > York > > Where does this get written? > > -Scott [Alison Wang] Thanks for your replies. Your concerns are right. SPARE4 register needs to be written in kernel. This is an issue about deep sleep in LS1021A Secure Boot. It is found in SDK2.0. The corresponding patch for kernel is sent in SDK2.0. Well, deep sleep uses an old way in SDK2.0. For upstream, deep sleep patches haven't been sent out as it will use PSCI and there are some issues about PSCI. So the corresponding patch for kernel can't be sent out now. Best Regards, Alison Wang
On 04/05/2016 09:16 PM, Huan Wang wrote: > Hi, York and Scott, > >> On 04/05/2016 05:11 AM, Alison Wang wrote: >>> For LS1021A Secure Boot, SPARE2 register is used and modified by the >>> IBR. To avoid the conflict, SPARE4 is used instead of SPARE2 to store >>> the entry point of kernel. This patch is to get the entry point of >>> kernel from SPARE4 instead of SPARE2. >>> >>> Signed-off-by: Alison Wang <alison.wang@nxp.com> >>> --- >>> board/freescale/common/arm_sleep.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/board/freescale/common/arm_sleep.c >>> b/board/freescale/common/arm_sleep.c >>> index 71ed15e..6d967f0 100644 >>> --- a/board/freescale/common/arm_sleep.c >>> +++ b/board/freescale/common/arm_sleep.c >>> @@ -88,7 +88,7 @@ int fsl_dp_resume(void) >>> dp_resume_prepare(); >>> >>> /* Get the entry address and jump to kernel */ >>> - start_addr = in_le32(&scfg->sparecr[1]); >>> + start_addr = in_le32(&scfg->sparecr[3]); >>> debug("Entry address is 0x%08x\n", start_addr); >>> kernel_resume = (void (*)(void))start_addr; >>> secure_ram_addr(_do_nonsec_entry)(kernel_resume, 0, 0, 0); >>> >> Alison, >> >> Does this change need to be in sync with Kernel change? >> >> York >> >> Where does this get written? >> >> -Scott > [Alison Wang] Thanks for your replies. Your concerns are right. > SPARE4 register needs to be written in kernel. > > This is an issue about deep sleep in LS1021A Secure Boot. It is found > in SDK2.0. The corresponding patch for kernel is sent in SDK2.0. > > Well, deep sleep uses an old way in SDK2.0. For upstream, deep sleep > patches haven't been sent out as it will use PSCI and there are some > issues about PSCI. So the corresponding patch for kernel can't be sent > out now. It's not about when the patch is sent. It's about managing compatibility. There needs to be some way to communicate what the expectations are between Linux and U-Boot, or to limit the change to chips where this feature has never worked before. We can't introduce regressions when the kernel is updated but not U-Boot, and regressions when U-Boot is updated but not the kernel are almost as bad. -Scott
Hi, Scott, > On 04/05/2016 09:16 PM, Huan Wang wrote: > > Hi, York and Scott, > > > >> On 04/05/2016 05:11 AM, Alison Wang wrote: > >>> For LS1021A Secure Boot, SPARE2 register is used and modified by the > >>> IBR. To avoid the conflict, SPARE4 is used instead of SPARE2 to > >>> store the entry point of kernel. This patch is to get the entry > >>> point of kernel from SPARE4 instead of SPARE2. > >>> > >>> Signed-off-by: Alison Wang <alison.wang@nxp.com> > >>> --- > >>> board/freescale/common/arm_sleep.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/board/freescale/common/arm_sleep.c > >>> b/board/freescale/common/arm_sleep.c > >>> index 71ed15e..6d967f0 100644 > >>> --- a/board/freescale/common/arm_sleep.c > >>> +++ b/board/freescale/common/arm_sleep.c > >>> @@ -88,7 +88,7 @@ int fsl_dp_resume(void) > >>> dp_resume_prepare(); > >>> > >>> /* Get the entry address and jump to kernel */ > >>> - start_addr = in_le32(&scfg->sparecr[1]); > >>> + start_addr = in_le32(&scfg->sparecr[3]); > >>> debug("Entry address is 0x%08x\n", start_addr); > >>> kernel_resume = (void (*)(void))start_addr; > >>> secure_ram_addr(_do_nonsec_entry)(kernel_resume, 0, 0, 0); > >>> > >> Alison, > >> > >> Does this change need to be in sync with Kernel change? > >> > >> York > >> > >> Where does this get written? > >> > >> -Scott > > [Alison Wang] Thanks for your replies. Your concerns are right. > > SPARE4 register needs to be written in kernel. > > > > This is an issue about deep sleep in LS1021A Secure Boot. It is found > > in SDK2.0. The corresponding patch for kernel is sent in SDK2.0. > > > > Well, deep sleep uses an old way in SDK2.0. For upstream, deep sleep > > patches haven't been sent out as it will use PSCI and there are some > > issues about PSCI. So the corresponding patch for kernel can't be sent > > out now. > > It's not about when the patch is sent. It's about managing > compatibility. There needs to be some way to communicate what the > expectations are between Linux and U-Boot, or to limit the change to > chips where this feature has never worked before. We can't introduce > regressions when the kernel is updated but not U-Boot, and regressions > when U-Boot is updated but not the kernel are almost as bad. > > -Scott [Alison Wang] Thanks for your advice. What you said is right. I will give up this patch in upstream now. Later, when the deep sleep patches for kernel is ready, I will fix the issue in U-Boot and kernel simultaneously. So there isn't any problem about the compatibility between U-Boot and kernel. Best Regards, Alison Wang
On 04/07/2016 04:11 AM, Huan Wang wrote: > Hi, Scott, > >> On 04/05/2016 09:16 PM, Huan Wang wrote: >>> Hi, York and Scott, >>> >>>> On 04/05/2016 05:11 AM, Alison Wang wrote: >>>>> For LS1021A Secure Boot, SPARE2 register is used and modified by the >>>>> IBR. To avoid the conflict, SPARE4 is used instead of SPARE2 to >>>>> store the entry point of kernel. This patch is to get the entry >>>>> point of kernel from SPARE4 instead of SPARE2. >>>>> >>>>> Signed-off-by: Alison Wang <alison.wang@nxp.com> >>>>> --- >>>>> board/freescale/common/arm_sleep.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/board/freescale/common/arm_sleep.c >>>>> b/board/freescale/common/arm_sleep.c >>>>> index 71ed15e..6d967f0 100644 >>>>> --- a/board/freescale/common/arm_sleep.c >>>>> +++ b/board/freescale/common/arm_sleep.c >>>>> @@ -88,7 +88,7 @@ int fsl_dp_resume(void) >>>>> dp_resume_prepare(); >>>>> >>>>> /* Get the entry address and jump to kernel */ >>>>> - start_addr = in_le32(&scfg->sparecr[1]); >>>>> + start_addr = in_le32(&scfg->sparecr[3]); >>>>> debug("Entry address is 0x%08x\n", start_addr); >>>>> kernel_resume = (void (*)(void))start_addr; >>>>> secure_ram_addr(_do_nonsec_entry)(kernel_resume, 0, 0, 0); >>>>> >>>> Alison, >>>> >>>> Does this change need to be in sync with Kernel change? >>>> >>>> York >>>> >>>> Where does this get written? >>>> >>>> -Scott >>> [Alison Wang] Thanks for your replies. Your concerns are right. >>> SPARE4 register needs to be written in kernel. >>> >>> This is an issue about deep sleep in LS1021A Secure Boot. It is found >>> in SDK2.0. The corresponding patch for kernel is sent in SDK2.0. >>> >>> Well, deep sleep uses an old way in SDK2.0. For upstream, deep sleep >>> patches haven't been sent out as it will use PSCI and there are some >>> issues about PSCI. So the corresponding patch for kernel can't be sent >>> out now. >> >> It's not about when the patch is sent. It's about managing >> compatibility. There needs to be some way to communicate what the >> expectations are between Linux and U-Boot, or to limit the change to >> chips where this feature has never worked before. We can't introduce >> regressions when the kernel is updated but not U-Boot, and regressions >> when U-Boot is updated but not the kernel are almost as bad. >> >> -Scott > [Alison Wang] Thanks for your advice. What you said is right. I will give up > this patch in upstream now. Later, when the deep sleep patches for kernel is > ready, I will fix the issue in U-Boot and kernel simultaneously. So there isn't > any problem about the compatibility between U-Boot and kernel. Please reread what I wrote. Fixing them simultaneously doesn't solve the problem because there's no guarantee that users update simultaneously. For a particularly bad example, consider someone bisecting the kernel. It would be terrible to require that the boot firmware be reflashed every time the kernel crosses this boundary. -Scott
diff --git a/board/freescale/common/arm_sleep.c b/board/freescale/common/arm_sleep.c index 71ed15e..6d967f0 100644 --- a/board/freescale/common/arm_sleep.c +++ b/board/freescale/common/arm_sleep.c @@ -88,7 +88,7 @@ int fsl_dp_resume(void) dp_resume_prepare(); /* Get the entry address and jump to kernel */ - start_addr = in_le32(&scfg->sparecr[1]); + start_addr = in_le32(&scfg->sparecr[3]); debug("Entry address is 0x%08x\n", start_addr); kernel_resume = (void (*)(void))start_addr; secure_ram_addr(_do_nonsec_entry)(kernel_resume, 0, 0, 0);
For LS1021A Secure Boot, SPARE2 register is used and modified by the IBR. To avoid the conflict, SPARE4 is used instead of SPARE2 to store the entry point of kernel. This patch is to get the entry point of kernel from SPARE4 instead of SPARE2. Signed-off-by: Alison Wang <alison.wang@nxp.com> --- board/freescale/common/arm_sleep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)