All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Leo Yan <leoy@marvell.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>,
	Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com>,
	Yu Tang <ytang5@marvell.com>, Will Deacon <Will.Deacon@arm.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	Russell King <linux@arm.linux.org.uk>,
	Nicolas Pitre <nico@linaro.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>, Loc Ho <lho@apm.com>,
	"ksankaran@apm.com" <ksankaran@apm.com>,
	Dave P Martin <Dave.Martin@arm.com>, Feng Kan <fkan@apm.com>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"graeme.gregory@linaro.org" <graeme.gregory@linaro.org>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Hanjun Guo <hanjun.guo@linaro.org>,
	Colin Cross <ccross@android.com>,
	Christoffer Dall <christoffer.dall@linaro.org>,
	Zhou
Subject: Re: [PATCH RFC v3 04/12] arm64: kernel: cpu_{suspend/resume} implementation
Date: Fri, 20 Dec 2013 11:57:35 +0000	[thread overview]
Message-ID: <20131220115735.GH25477@arm.com> (raw)
In-Reply-To: <52B42A48.5090202@marvell.com>

On Fri, Dec 20, 2013 at 11:30:16AM +0000, Leo Yan wrote:
> On 11/21/2013 07:24 PM, Lorenzo Pieralisi wrote:
> > +/*
> > + * x0 must contain the sctlr value retrieved from restored context
> > + */
> > +ENTRY(cpu_resume_mmu)
> > +	ldr	x3, =cpu_resume_after_mmu
> > +	msr	sctlr_el1, x0		// restore sctlr_el1
> > +	isb
> > +	br	x3			// global jump to virtual address
> > +ENDPROC(cpu_resume_mmu)
> > +cpu_resume_after_mmu:
> > +	mov	x0, #0			// return zero on success
> > +	ldp	x19, x20, [sp, #16]
> > +	ldp	x21, x22, [sp, #32]
> > +	ldp	x23, x24, [sp, #48]
> > +	ldp	x25, x26, [sp, #64]
> > +	ldp	x27, x28, [sp, #80]
> > +	ldp	x29, lr, [sp], #96
> > +	ret
> > +ENDPROC(cpu_resume_after_mmu)
> > +
> > +	.data
> > +ENTRY(cpu_resume)
> > +	bl	el2_setup		// if in EL2 drop to EL1 cleanly
> 
> Compare to v2's patch set, here remove the calculation fro the offset 
> b/t PHYS_OFFSET - PAGE_OFFSET; so when i verify the patch set, i saw x28 
> is zero and finally introduce the EL2's sync exception. Below are pasted 
> v2's code for reference.
> 
> do u want use firmware to set the x28 for the offset value? :-) IMHO, 
> v2's implementation is more reasonable and it's better keep the code.
> 
> ENTRY(cpu_resume)
>            adr     x4, sleep_save_sp
>            ldr     x5, =sleep_save_sp
>            sub     x28, x4, x5             // x28 = PHYS_OFFSET - 
> PAGE_OFFSET
>            /*
>             * make sure el2 is sane, el2_setup expects:
>             * x28 = PHYS_OFFSET - PAGE_OFFSET
>             */
>            bl      el2_setup               // if in EL2 drop to EL1 cleanly

With commit 85cc00eaa81d (arm64: kernel: add code to set cpu boot mode
to secondary_entry shim) we no longer compute the phys-page offset
before el2_setup.

Where does the fault happen?

-- 
Catalin

WARNING: multiple messages have this Message-ID (diff)
From: catalin.marinas@arm.com (Catalin Marinas)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RFC v3 04/12] arm64: kernel: cpu_{suspend/resume} implementation
Date: Fri, 20 Dec 2013 11:57:35 +0000	[thread overview]
Message-ID: <20131220115735.GH25477@arm.com> (raw)
In-Reply-To: <52B42A48.5090202@marvell.com>

On Fri, Dec 20, 2013 at 11:30:16AM +0000, Leo Yan wrote:
> On 11/21/2013 07:24 PM, Lorenzo Pieralisi wrote:
> > +/*
> > + * x0 must contain the sctlr value retrieved from restored context
> > + */
> > +ENTRY(cpu_resume_mmu)
> > +	ldr	x3, =cpu_resume_after_mmu
> > +	msr	sctlr_el1, x0		// restore sctlr_el1
> > +	isb
> > +	br	x3			// global jump to virtual address
> > +ENDPROC(cpu_resume_mmu)
> > +cpu_resume_after_mmu:
> > +	mov	x0, #0			// return zero on success
> > +	ldp	x19, x20, [sp, #16]
> > +	ldp	x21, x22, [sp, #32]
> > +	ldp	x23, x24, [sp, #48]
> > +	ldp	x25, x26, [sp, #64]
> > +	ldp	x27, x28, [sp, #80]
> > +	ldp	x29, lr, [sp], #96
> > +	ret
> > +ENDPROC(cpu_resume_after_mmu)
> > +
> > +	.data
> > +ENTRY(cpu_resume)
> > +	bl	el2_setup		// if in EL2 drop to EL1 cleanly
> 
> Compare to v2's patch set, here remove the calculation fro the offset 
> b/t PHYS_OFFSET - PAGE_OFFSET; so when i verify the patch set, i saw x28 
> is zero and finally introduce the EL2's sync exception. Below are pasted 
> v2's code for reference.
> 
> do u want use firmware to set the x28 for the offset value? :-) IMHO, 
> v2's implementation is more reasonable and it's better keep the code.
> 
> ENTRY(cpu_resume)
>            adr     x4, sleep_save_sp
>            ldr     x5, =sleep_save_sp
>            sub     x28, x4, x5             // x28 = PHYS_OFFSET - 
> PAGE_OFFSET
>            /*
>             * make sure el2 is sane, el2_setup expects:
>             * x28 = PHYS_OFFSET - PAGE_OFFSET
>             */
>            bl      el2_setup               // if in EL2 drop to EL1 cleanly

With commit 85cc00eaa81d (arm64: kernel: add code to set cpu boot mode
to secondary_entry shim) we no longer compute the phys-page offset
before el2_setup.

Where does the fault happen?

-- 
Catalin

  reply	other threads:[~2013-12-20 11:57 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-21 11:24 [PATCH RFC v3 00/12] arm64: suspend/resume implementation Lorenzo Pieralisi
2013-11-21 11:24 ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 01/12] arm64: kernel: add MPIDR_EL1 accessors macros Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 02/12] arm64: kernel: build MPIDR_EL1 hash function data structure Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 03/12] arm64: kernel: suspend/resume registers save/restore Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 04/12] arm64: kernel: cpu_{suspend/resume} implementation Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-12-20 11:30   ` Leo Yan
2013-12-20 11:30     ` Leo Yan
2013-12-20 11:57     ` Catalin Marinas [this message]
2013-12-20 11:57       ` Catalin Marinas
2013-12-23 14:04     ` Lorenzo Pieralisi
2013-12-23 14:04       ` Lorenzo Pieralisi
2013-12-24  6:18       ` Leo Yan
2013-12-24  6:18         ` Leo Yan
2013-11-21 11:24 ` [PATCH RFC v3 05/12] arm64: kernel: implement fpsimd CPU PM notifier Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 06/12] arm: kvm: implement " Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-27  3:02   ` Christoffer Dall
2013-11-27  3:02     ` Christoffer Dall
2013-11-21 11:24 ` [PATCH RFC v3 07/12] arm64: kernel: refactor code to install/uninstall breakpoints Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 08/12] arm64: kernel: implement HW breakpoints CPU PM notifier Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-12-20 17:29   ` Will Deacon
2013-12-20 17:29     ` Will Deacon
2013-12-23 13:50     ` Lorenzo Pieralisi
2013-12-23 13:50       ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 09/12] arm64: enable generic clockevent broadcast Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 10/12] arm64: kernel: add CPU idle call Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 11/12] arm64: kernel: add PM build infrastructure Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi
2013-11-21 11:24 ` [PATCH RFC v3 12/12] arm64: add CPU power management menu/entries Lorenzo Pieralisi
2013-11-21 11:24   ` Lorenzo Pieralisi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131220115735.GH25477@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=Dave.Martin@arm.com \
    --cc=Lorenzo.Pieralisi@arm.com \
    --cc=Marc.Zyngier@arm.com \
    --cc=Mark.Rutland@arm.com \
    --cc=Sudeep.KarkadaNagesha@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=ccross@android.com \
    --cc=christoffer.dall@linaro.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=fkan@apm.com \
    --cc=graeme.gregory@linaro.org \
    --cc=hanjun.guo@linaro.org \
    --cc=ksankaran@apm.com \
    --cc=leoy@marvell.com \
    --cc=lho@apm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=nico@linaro.org \
    --cc=santosh.shilimkar@ti.com \
    --cc=sboyd@codeaurora.org \
    --cc=ytang5@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.