| Message ID | 4F8D5664.7000304@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 17 April 2012 12:39, Alexey Starikovskiy <aystarik@gmail.com> wrote: Patches should almost always have more than a single line commit message in my opinion. > Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com> > --- > target-arm/translate.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/target-arm/translate.c b/target-arm/translate.c > index 7a3c7d6..b35c85f 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -7415,6 +7415,12 @@ static void disas_arm_insn(CPUARMState * env, > DisasContext *s) > } > tcg_temp_free(addr); > } else { > + if (!(env->cp15.c1_sys & (1 << 10))) { This is wrong -- on CPUs without the Multiprocessing Extensions SCTLR.SW is always zero but the SWP instructions work. > + /* Check if SCTLR.SW is set. Any change to SCTLR > + * invalidates all translations, so we are safe. > + */ This isn't true -- SCTLR writes currently do a tlb_flush() but not a tb_flush(). -- PMM
On 04/17/2012 03:55 PM, Peter Maydell wrote: > On 17 April 2012 12:39, Alexey Starikovskiy<aystarik@gmail.com> wrote: > > Patches should almost always have more than a single line > commit message in my opinion. > Ok >> Signed-off-by: Alexey Starikovskiy<aystarik@gmail.com> >> --- >> target-arm/translate.c | 6 ++++++ >> 1 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/target-arm/translate.c b/target-arm/translate.c >> index 7a3c7d6..b35c85f 100644 >> --- a/target-arm/translate.c >> +++ b/target-arm/translate.c >> @@ -7415,6 +7415,12 @@ static void disas_arm_insn(CPUARMState * env, >> DisasContext *s) >> } >> tcg_temp_free(addr); >> } else { >> + if (!(env->cp15.c1_sys& (1<< 10))) { > This is wrong -- on CPUs without the Multiprocessing Extensions > SCTLR.SW is always zero but the SWP instructions work. You mean ARM_FEATURE_V7MP or something else? >> + /* Check if SCTLR.SW is set. Any change to SCTLR >> + * invalidates all translations, so we are safe. >> + */ > This isn't true -- SCTLR writes currently do a tlb_flush() but > not a tb_flush(). Right... > -- PMM
On 17 April 2012 13:03, Alexey Starikovskiy <aystarik@gmail.com> wrote: > On 04/17/2012 03:55 PM, Peter Maydell wrote: >> >> On 17 April 2012 12:39, Alexey Starikovskiy<aystarik@gmail.com> wrote: >> >> Patches should almost always have more than a single line >> commit message in my opinion. >> > Ok >>> >>> Signed-off-by: Alexey Starikovskiy<aystarik@gmail.com> >>> --- >>> target-arm/translate.c | 6 ++++++ >>> 1 files changed, 6 insertions(+), 0 deletions(-) >>> >>> diff --git a/target-arm/translate.c b/target-arm/translate.c >>> index 7a3c7d6..b35c85f 100644 >>> --- a/target-arm/translate.c >>> +++ b/target-arm/translate.c >>> @@ -7415,6 +7415,12 @@ static void disas_arm_insn(CPUARMState * env, >>> DisasContext *s) >>> } >>> tcg_temp_free(addr); >>> } else { >>> + if (!(env->cp15.c1_sys& (1<< 10))) { >> >> This is wrong -- on CPUs without the Multiprocessing Extensions >> SCTLR.SW is always zero but the SWP instructions work. > > You mean ARM_FEATURE_V7MP or something else? Yes, ARM_FEATURE_V7MP is the bit that tracks whether the CPU implements the v7 multiprocessing extensions. Also you'll need to watch out that you don't break handling of SWP in linux-user mode. Forcing SCTLR.SW to 1 on reset on v7MP CPUs when CONFIG_USER_ONLY is defined is probably the best thing, but you'll find that changes there clash with a patchset I currently have out on the list. -- PMM
diff --git a/target-arm/translate.c b/target-arm/translate.c index 7a3c7d6..b35c85f 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -7415,6 +7415,12 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) } tcg_temp_free(addr); } else { + if (!(env->cp15.c1_sys & (1 << 10))) { + /* Check if SCTLR.SW is set. Any change to SCTLR + * invalidates all translations, so we are safe. + */ + goto illegal_op; + } /* SWP instruction */ rm = (insn) & 0xf;
Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com> --- target-arm/translate.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)