| Message ID | 1368475464-3116-1-git-send-email-green@moxielogic.com |
|---|---|
| State | New |
| Headers | show |
On 05/13/2013 01:33 PM, Max Filippov wrote: > memory access at points (1) and (2) can abort the instruction (it did so > b/o the bug, but it may do so legitimately when you add MMU support), > but it has modified REG(1) at those points, which will not be restored. > It's probably worth carrying register modifications in some temporary > until after the point (2). ... Which you'll recall that I pointed out during initial review, AG. r~
This patch still needs to be applied. There was some follow-up discussion on this patch back in May, but none of it negates the fact that this patch needs to be applied. Thanks! AG Anthony Green <green@moxielogic.com> writes: > Fix a simple bug in tlb_fill for moxie. The port was mostly working > before, which is why I only really noticed it recently. Thanks to > @jcmvbkbc for tracking it down. > > Signed-off-by: Anthony Green <green@moxielogic.com> > --- > target-moxie/helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target-moxie/helper.c b/target-moxie/helper.c > index 6e0ac2a..6c36c49 100644 > --- a/target-moxie/helper.c > +++ b/target-moxie/helper.c > @@ -55,8 +55,8 @@ void tlb_fill(CPUMoxieState *env, target_ulong addr, int is_write, int mmu_idx, > if (retaddr) { > cpu_restore_state(env, retaddr); > } > + cpu_loop_exit(env); > } > - cpu_loop_exit(env); > } > > void helper_raise_exception(CPUMoxieState *env, int ex)
Hi, Am 15.12.2013 05:10, schrieb Anthony Green: > > This patch still needs to be applied. There was some follow-up > discussion on this patch back in May, but none of it negates the fact > that this patch needs to be applied. It introduces a tab, please fix. Apart from that I believe I reported some inconsistencies between targets in that function, so "all targets except moxie do it conditional to FOO" may be a convincing explanation independent of the bug you were discussing that may or may not be otherwise workaroundable. And since this is purely in target-moxie I would suggest to simply send a pull as target maintainer once you have someone trustworthy's Reviewed-by and it doesn't break `make check`, similar to how it's done for OpenRISC. And as usual the warning that I have a patch lying around that changes the cpu_loop_exit() argument to CPUState, but I may not get to rebasing that until the holidays, so no imminent conflict. Regards, Andreas
On 15 December 2013 18:51, Andreas Färber <afaerber@suse.de> wrote: > Am 15.12.2013 05:10, schrieb Anthony Green: >> This patch still needs to be applied. There was some follow-up >> discussion on this patch back in May, but none of it negates the fact >> that this patch needs to be applied. > > It introduces a tab, please fix. > > Apart from that I believe I reported some inconsistencies between > targets in that function, so "all targets except moxie do it conditional > to FOO" may be a convincing explanation independent of the bug you were > discussing that may or may not be otherwise workaroundable. I dug out the thread where you did that (which turns out to be private mail and not qemu-devel). In follow up to that RTH and I agreed that it definitely is a bug and this patch is the correct fix. > And since this is purely in target-moxie I would suggest to simply send > a pull as target maintainer once you have someone trustworthy's > Reviewed-by and it doesn't break `make check`, similar to how it's done > for OpenRISC. Yes, this makes sense to me, especially since Anthony has a number of other moxie patches on list at this point. thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 15 December 2013 18:51, Andreas Färber <afaerber@suse.de> wrote: >> And since this is purely in target-moxie I would suggest to simply send >> a pull as target maintainer once you have someone trustworthy's >> Reviewed-by and it doesn't break `make check`, similar to how it's done >> for OpenRISC. > > Yes, this makes sense to me, especially since Anthony has a number > of other moxie patches on list at this point. According to this page... http://wiki.qemu.org/Contribute/SubmitAPullRequest ...I need to get a key signed by a member of the QEMU community in person. Are there any QEMU hackers in the Toronto area? AG
On Mon, Dec 16, 2013 at 5:26 AM, Anthony Green <green@moxielogic.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 15 December 2013 18:51, Andreas Färber <afaerber@suse.de> wrote: >>> And since this is purely in target-moxie I would suggest to simply send >>> a pull as target maintainer once you have someone trustworthy's >>> Reviewed-by and it doesn't break `make check`, similar to how it's done >>> for OpenRISC. >> >> Yes, this makes sense to me, especially since Anthony has a number >> of other moxie patches on list at this point. > > According to this page... > > http://wiki.qemu.org/Contribute/SubmitAPullRequest > > ...I need to get a key signed by a member of the QEMU community in > person. Are there any QEMU hackers in the Toronto area? > While this question has air, anyone in and around Brisbane Australia who can sign me? (although that's probably a long shot). I can get a key signed by Edgar if that is enough. Regards, Peter > AG >
diff --git a/target-moxie/helper.c b/target-moxie/helper.c index 6e0ac2a..6c36c49 100644 --- a/target-moxie/helper.c +++ b/target-moxie/helper.c @@ -55,8 +55,8 @@ void tlb_fill(CPUMoxieState *env, target_ulong addr, int is_write, int mmu_idx, if (retaddr) { cpu_restore_state(env, retaddr); } + cpu_loop_exit(env); } - cpu_loop_exit(env); } void helper_raise_exception(CPUMoxieState *env, int ex)
Fix a simple bug in tlb_fill for moxie. The port was mostly working before, which is why I only really noticed it recently. Thanks to @jcmvbkbc for tracking it down. Signed-off-by: Anthony Green <green@moxielogic.com> --- target-moxie/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)