* [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary
@ 2011-04-24 14:47 오유연
2011-04-24 15:25 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: 오유연 @ 2011-04-24 14:47 UTC (permalink / raw)
To: qemu-devel
When consecutive memory locations are on page boundary, a base register may be
loaded before page fault occurs. After page fault handling, it losts the memory
location information. To solve this problem, loading a base register has to put back.
Signed-off-by: Yuyeon Oh <yuyeon.oh@samsung.com>
---
target-arm/translate.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index e1bda57..61eb4d5 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7984,11 +7984,16 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
continue;
if (insn & (1 << 20)) {
/* Load. */
- tmp = gen_ld32(addr, IS_USER(s));
- if (i == 15) {
- gen_bx(s, tmp);
- } else {
- store_reg(s, i, tmp);
+ if (i == rn) {
+ tmp2 = gen_ld32(addr, IS_USER(s));
+ }
+ else {
+ tmp = gen_ld32(addr, IS_USER(s));
+ if (i == 15) {
+ gen_bx(s, tmp);
+ } else {
+ store_reg(s, i, tmp);
+ }
}
} else {
/* Store. */
@@ -7997,6 +8002,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
}
tcg_gen_addi_i32(addr, addr, 4);
}
+ if ((insn & (1 << 20)) && (insn & (1 << rn))) {
+ store_reg(s, rn, tmp2);
+ }
if (insn & (1 << 21)) {
/* Base register writeback. */
if (insn & (1 << 24)) {
--
1.7.4.msysgit.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary
2011-04-24 14:47 [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary 오유연
@ 2011-04-24 15:25 ` Peter Maydell
0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2011-04-24 15:25 UTC (permalink / raw)
To: yuyeon.oh; +Cc: qemu-devel
2011/4/24 오유연 <yuyeon.oh@samsung.com>:
> When consecutive memory locations are on page boundary, a base register may be
> loaded before page fault occurs. After page fault handling, it losts the memory
> location information. To solve this problem, loading a base register has to put back.
Thanks for finding this. I agree the fix is required, but I think I'd
prefer it if the thumb code for this case handled it in the same way
the disas_arm_insn() code does:
loaded_base = 0;
TCGV_UNUSED(loaded_var);
[...]
tmp = gen_ld32(addr, IS_USER(s));
if (i == 15) {
gen_bx(s, tmp);
} else if (i == rn) {
loaded_var = tmp;
loaded_base = 1;
} else {
store_reg(s, i, tmp);
}
[...]
if (loaded_base) {
store_reg(s, rn, loaded_var);
}
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary
@ 2011-04-25 1:19 YuYeon Oh
0 siblings, 0 replies; 6+ messages in thread
From: YuYeon Oh @ 2011-04-25 1:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel@nongnu.org
2011/4/24 Peter Maydell
>Thanks for finding this. I agree the fix is required, but I think I'd
>prefer it if the thumb code for this case handled it in the same way
>the disas_arm_insn() code does:
>
> loaded_base = 0;
> TCGV_UNUSED(loaded_var);
> [...]
> tmp = gen_ld32(addr, IS_USER(s));
> if (i == 15) {
> gen_bx(s, tmp);
> } else if (i == rn) {
> loaded_var = tmp;
> loaded_base = 1;
> } else {
> store_reg(s, i, tmp);
> }
> [...]
> if (loaded_base) {
> store_reg(s, rn, loaded_var);
> }
>
>-- PMM
Thank you for your advice. I am going to send a new patch soon.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary
@ 2011-04-25 1:23 YuYeon Oh
2011-04-26 17:17 ` Peter Maydell
2011-04-27 18:18 ` Aurelien Jarno
0 siblings, 2 replies; 6+ messages in thread
From: YuYeon Oh @ 2011-04-25 1:23 UTC (permalink / raw)
To: qemu-devel@nongnu.org
target-arm: fix LDMIA bug on page boundary
When consecutive memory locations are on page boundary, a base register may be
loaded before page fault occurs. After page fault handling, it losts the memory
location information. To solve this problem, loading a base register has to put back.
Signed-off-by: Yuyeon Oh <yuyeon.oh@samsung.com>
---
target-arm/translate.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index e1bda57..410e7c4 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7967,7 +7967,8 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
}
}
} else {
- int i;
+ int i, loaded_base = 0;
+ TCGv loaded_var;
/* Load/store multiple. */
addr = load_reg(s, rn);
offset = 0;
@@ -7979,6 +7980,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
tcg_gen_addi_i32(addr, addr, -offset);
}
+ TCGV_UNUSED(loaded_var);
for (i = 0; i < 16; i++) {
if ((insn & (1 << i)) == 0)
continue;
@@ -7987,6 +7989,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
tmp = gen_ld32(addr, IS_USER(s));
if (i == 15) {
gen_bx(s, tmp);
+ } else if (i == rn) {
+ loaded_var = tmp;
+ loaded_base = 1;
} else {
store_reg(s, i, tmp);
}
@@ -7997,6 +8002,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
}
tcg_gen_addi_i32(addr, addr, 4);
}
+ if (loaded_base) {
+ store_reg(s, rn, loaded_var);
+ }
if (insn & (1 << 21)) {
/* Base register writeback. */
if (insn & (1 << 24)) {
--
1.7.4.msysgit.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary
2011-04-25 1:23 YuYeon Oh
@ 2011-04-26 17:17 ` Peter Maydell
2011-04-27 18:18 ` Aurelien Jarno
1 sibling, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2011-04-26 17:17 UTC (permalink / raw)
To: yuyeon.oh; +Cc: qemu-devel@nongnu.org
On 25 April 2011 02:23, YuYeon Oh <yuyeon.oh@samsung.com> wrote:
> target-arm: fix LDMIA bug on page boundary
(You don't need to repeat the Subject summary line in the body, it makes the
git changelog look a bit odd when the patch is applied with 'git am').
> When consecutive memory locations are on page boundary, a base register may be
> loaded before page fault occurs. After page fault handling, it losts the memory
> location information. To solve this problem, loading a base register has to put back.
>
> Signed-off-by: Yuyeon Oh <yuyeon.oh@samsung.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
I've tested this and confirmed that it fixes this bug for the
Thumb T2 encoding. However, the same problem still exists for the
T1 (16 bit) encoding; I'll send a patch for that in a moment.
(The ARM encoding did not have this bug.)
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary
2011-04-25 1:23 YuYeon Oh
2011-04-26 17:17 ` Peter Maydell
@ 2011-04-27 18:18 ` Aurelien Jarno
1 sibling, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2011-04-27 18:18 UTC (permalink / raw)
To: YuYeon Oh; +Cc: qemu-devel@nongnu.org
On Mon, Apr 25, 2011 at 01:23:58AM +0000, YuYeon Oh wrote:
> target-arm: fix LDMIA bug on page boundary
>
> When consecutive memory locations are on page boundary, a base register may be
> loaded before page fault occurs. After page fault handling, it losts the memory
> location information. To solve this problem, loading a base register has to put back.
>
> Signed-off-by: Yuyeon Oh <yuyeon.oh@samsung.com>
> ---
> target-arm/translate.c | 10 +++++++++-
> 1 files changed, 9 insertions(+), 1 deletions(-)
Thanks, applied.
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index e1bda57..410e7c4 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7967,7 +7967,8 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
> }
> }
> } else {
> - int i;
> + int i, loaded_base = 0;
> + TCGv loaded_var;
> /* Load/store multiple. */
> addr = load_reg(s, rn);
> offset = 0;
> @@ -7979,6 +7980,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
> tcg_gen_addi_i32(addr, addr, -offset);
> }
>
> + TCGV_UNUSED(loaded_var);
> for (i = 0; i < 16; i++) {
> if ((insn & (1 << i)) == 0)
> continue;
> @@ -7987,6 +7989,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
> tmp = gen_ld32(addr, IS_USER(s));
> if (i == 15) {
> gen_bx(s, tmp);
> + } else if (i == rn) {
> + loaded_var = tmp;
> + loaded_base = 1;
> } else {
> store_reg(s, i, tmp);
> }
> @@ -7997,6 +8002,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
> }
> tcg_gen_addi_i32(addr, addr, 4);
> }
> + if (loaded_base) {
> + store_reg(s, rn, loaded_var);
> + }
> if (insn & (1 << 21)) {
> /* Base register writeback. */
> if (insn & (1 << 24)) {
> --
> 1.7.4.msysgit.0
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-27 18:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-24 14:47 [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary 오유연
2011-04-24 15:25 ` Peter Maydell
-- strict thread matches above, loose matches on Subject: below --
2011-04-25 1:19 YuYeon Oh
2011-04-25 1:23 YuYeon Oh
2011-04-26 17:17 ` Peter Maydell
2011-04-27 18:18 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).