From: Julien Grall <julien.grall@linaro.org>
To: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>,
xen-devel@lists.xen.org
Cc: stefano.stabellini@citrix.com, patches@apm.com,
ian.campbell@citrix.com, Anup Patel <anup.patel@linaro.org>,
patches@linaro.org
Subject: Re: [PATCH] xen: arm: arm64: Fix memory cloberring issues during VFP save restore.
Date: Fri, 07 Feb 2014 11:54:11 +0000 [thread overview]
Message-ID: <52F4C963.2030401@linaro.org> (raw)
In-Reply-To: <1391769538-9091-1-git-send-email-pranavkumar@linaro.org>
Hello,
Thanks for sending the patch quickly.
On 07/02/14 10:38, Pranavkumar Sawargaonkar wrote:
> This patch addresses memory cloberring issue mentioed by Julien Grall
clobbering mentioned.
> with my earlier patch -
> Ref:
> http://www.gossamer-threads.com/lists/xen/devel/316247
Can you add the commit id?
>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> ---
> xen/arch/arm/arm64/vfp.c | 70 ++++++++++++++++++++++++----------------------
> 1 file changed, 36 insertions(+), 34 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/vfp.c b/xen/arch/arm/arm64/vfp.c
> index c09cf0c..62f56a3 100644
> --- a/xen/arch/arm/arm64/vfp.c
> +++ b/xen/arch/arm/arm64/vfp.c
> @@ -8,23 +8,24 @@ void vfp_save_state(struct vcpu *v)
> if ( !cpu_has_fp )
> return;
>
> - asm volatile("stp q0, q1, [%0, #16 * 0]\n\t"
> - "stp q2, q3, [%0, #16 * 2]\n\t"
> - "stp q4, q5, [%0, #16 * 4]\n\t"
> - "stp q6, q7, [%0, #16 * 6]\n\t"
> - "stp q8, q9, [%0, #16 * 8]\n\t"
> - "stp q10, q11, [%0, #16 * 10]\n\t"
> - "stp q12, q13, [%0, #16 * 12]\n\t"
> - "stp q14, q15, [%0, #16 * 14]\n\t"
> - "stp q16, q17, [%0, #16 * 16]\n\t"
> - "stp q18, q19, [%0, #16 * 18]\n\t"
> - "stp q20, q21, [%0, #16 * 20]\n\t"
> - "stp q22, q23, [%0, #16 * 22]\n\t"
> - "stp q24, q25, [%0, #16 * 24]\n\t"
> - "stp q26, q27, [%0, #16 * 26]\n\t"
> - "stp q28, q29, [%0, #16 * 28]\n\t"
> - "stp q30, q31, [%0, #16 * 30]\n\t"
> - :: "r" ((char *)(&v->arch.vfp.fpregs)): "memory");
> + asm volatile("stp q0, q1, [%1, #16 * 0]\n\t"
> + "stp q2, q3, [%1, #16 * 2]\n\t"
> + "stp q4, q5, [%1, #16 * 4]\n\t"
> + "stp q6, q7, [%1, #16 * 6]\n\t"
> + "stp q8, q9, [%1, #16 * 8]\n\t"
> + "stp q10, q11, [%1, #16 * 10]\n\t"
> + "stp q12, q13, [%1, #16 * 12]\n\t"
> + "stp q14, q15, [%1, #16 * 14]\n\t"
> + "stp q16, q17, [%1, #16 * 16]\n\t"
> + "stp q18, q19, [%1, #16 * 18]\n\t"
> + "stp q20, q21, [%1, #16 * 20]\n\t"
> + "stp q22, q23, [%1, #16 * 22]\n\t"
> + "stp q24, q25, [%1, #16 * 24]\n\t"
> + "stp q26, q27, [%1, #16 * 26]\n\t"
> + "stp q28, q29, [%1, #16 * 28]\n\t"
> + "stp q30, q31, [%1, #16 * 30]\n\t"
> + :"=Q" (*v->arch.vfp.fpregs): "r" (v->arch.vfp.fpregs)
> + : "memory");
You don't need anymore to clobber the whole memory. "memory" can be removed.
>
> v->arch.vfp.fpsr = READ_SYSREG32(FPSR);
> v->arch.vfp.fpcr = READ_SYSREG32(FPCR);
> @@ -36,23 +37,24 @@ void vfp_restore_state(struct vcpu *v)
> if ( !cpu_has_fp )
> return;
>
> - asm volatile("ldp q0, q1, [%0, #16 * 0]\n\t"
> - "ldp q2, q3, [%0, #16 * 2]\n\t"
> - "ldp q4, q5, [%0, #16 * 4]\n\t"
> - "ldp q6, q7, [%0, #16 * 6]\n\t"
> - "ldp q8, q9, [%0, #16 * 8]\n\t"
> - "ldp q10, q11, [%0, #16 * 10]\n\t"
> - "ldp q12, q13, [%0, #16 * 12]\n\t"
> - "ldp q14, q15, [%0, #16 * 14]\n\t"
> - "ldp q16, q17, [%0, #16 * 16]\n\t"
> - "ldp q18, q19, [%0, #16 * 18]\n\t"
> - "ldp q20, q21, [%0, #16 * 20]\n\t"
> - "ldp q22, q23, [%0, #16 * 22]\n\t"
> - "ldp q24, q25, [%0, #16 * 24]\n\t"
> - "ldp q26, q27, [%0, #16 * 26]\n\t"
> - "ldp q28, q29, [%0, #16 * 28]\n\t"
> - "ldp q30, q31, [%0, #16 * 30]\n\t"
> - :: "r" ((char *)(&v->arch.vfp.fpregs)): "memory");
> + asm volatile("ldp q0, q1, [%1, #16 * 0]\n\t"
> + "ldp q2, q3, [%1, #16 * 2]\n\t"
> + "ldp q4, q5, [%1, #16 * 4]\n\t"
> + "ldp q6, q7, [%1, #16 * 6]\n\t"
> + "ldp q8, q9, [%1, #16 * 8]\n\t"
> + "ldp q10, q11, [%1, #16 * 10]\n\t"
> + "ldp q12, q13, [%1, #16 * 12]\n\t"
> + "ldp q14, q15, [%1, #16 * 14]\n\t"
> + "ldp q16, q17, [%1, #16 * 16]\n\t"
> + "ldp q18, q19, [%1, #16 * 18]\n\t"
> + "ldp q20, q21, [%1, #16 * 20]\n\t"
> + "ldp q22, q23, [%1, #16 * 22]\n\t"
> + "ldp q24, q25, [%1, #16 * 24]\n\t"
> + "ldp q26, q27, [%1, #16 * 26]\n\t"
> + "ldp q28, q29, [%1, #16 * 28]\n\t"
> + "ldp q30, q31, [%1, #16 * 30]\n\t"
> + :: "Q" (*v->arch.vfp.fpregs), "r" (v->arch.vfp.fpregs)
> + : "memory");
Same here.
Cheers,
--
Julien Grall
next prev parent reply other threads:[~2014-02-07 11:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-07 10:38 [PATCH] xen: arm: arm64: Fix memory cloberring issues during VFP save restore Pranavkumar Sawargaonkar
2014-02-07 11:39 ` Ian Campbell
2014-02-07 14:29 ` George Dunlap
2014-02-07 14:35 ` Ian Campbell
2014-02-07 11:54 ` Julien Grall [this message]
2014-02-07 12:47 ` Pranavkumar Sawargaonkar
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=52F4C963.2030401@linaro.org \
--to=julien.grall@linaro.org \
--cc=anup.patel@linaro.org \
--cc=ian.campbell@citrix.com \
--cc=patches@apm.com \
--cc=patches@linaro.org \
--cc=pranavkumar@linaro.org \
--cc=stefano.stabellini@citrix.com \
--cc=xen-devel@lists.xen.org \
/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.