Re: [Qemu-devel] [PATCH] target-i386: Fix I/O bitmap checks for in/out
Am 24.01.2014 um 11:19 hat Kevin Wolf geschrieben: Commit 1b90d56e changed the implementation of in/out imm to not assign the accessed port number to cpu_T[0] as it appeared unnecessary. However, currently gen_check_io() makes use of cpu_T[0] to implement the I/O bitmap checks, so it's in fact still used and the change broke the check, leading to #GP in legitimate cases (and probably also allowing access to ports that shouldn't be allowed). This patch reintroduces the missing assignment for these cases. Signed-off-by: Kevin Wolf kw...@redhat.com Reviewed-by: Richard Henderson r...@twiddle.net Ping? /me considers sending a one-patch pull request for an area he's absolutely not maintaining, but if this is the only way to get patches applied to qemu... Kevin --- target-i386/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-i386/translate.c b/target-i386/translate.c index b0f2279..5dd2450 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -6284,6 +6284,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, case 0xe5: ot = mo_b_d32(b, dflag); val = cpu_ldub_code(env, s-pc++); +tcg_gen_movi_tl(cpu_T[0], val); gen_check_io(s, ot, pc_start - s-cs_base, SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes)); if (use_icount) @@ -6300,6 +6301,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, case 0xe7: ot = mo_b_d32(b, dflag); val = cpu_ldub_code(env, s-pc++); +tcg_gen_movi_tl(cpu_T[0], val); gen_check_io(s, ot, pc_start - s-cs_base, svm_is_rep(prefixes)); gen_op_mov_v_reg(ot, cpu_T[1], R_EAX); -- 1.8.1.4
Re: [Qemu-devel] [PATCH] target-i386: Fix I/O bitmap checks for in/out
On 17 February 2014 10:14, Kevin Wolf kw...@redhat.com wrote: Am 24.01.2014 um 11:19 hat Kevin Wolf geschrieben: Commit 1b90d56e changed the implementation of in/out imm to not assign the accessed port number to cpu_T[0] as it appeared unnecessary. However, currently gen_check_io() makes use of cpu_T[0] to implement the I/O bitmap checks, so it's in fact still used and the change broke the check, leading to #GP in legitimate cases (and probably also allowing access to ports that shouldn't be allowed). This patch reintroduces the missing assignment for these cases. Signed-off-by: Kevin Wolf kw...@redhat.com Reviewed-by: Richard Henderson r...@twiddle.net Ping? /me considers sending a one-patch pull request for an area he's absolutely not maintaining, but if this is the only way to get patches applied to qemu... I don't currently have a workflow for identifying and applying patches which aren't in pull requests (apart from obvious fixes build breakage patches, and even there it's depending on my happening to notice them). In this case I'd expect rth to put together a pull request, I guess. Suggestions for better workflows welcome; we have had issues with patches falling through the gaps between maintained subsystems for a long time. thanks -- PMM
Re: [Qemu-devel] [PATCH] target-i386: Fix I/O bitmap checks for in/out
Am 17.02.2014 um 11:47 hat Peter Maydell geschrieben: On 17 February 2014 10:14, Kevin Wolf kw...@redhat.com wrote: Am 24.01.2014 um 11:19 hat Kevin Wolf geschrieben: Commit 1b90d56e changed the implementation of in/out imm to not assign the accessed port number to cpu_T[0] as it appeared unnecessary. However, currently gen_check_io() makes use of cpu_T[0] to implement the I/O bitmap checks, so it's in fact still used and the change broke the check, leading to #GP in legitimate cases (and probably also allowing access to ports that shouldn't be allowed). This patch reintroduces the missing assignment for these cases. Signed-off-by: Kevin Wolf kw...@redhat.com Reviewed-by: Richard Henderson r...@twiddle.net Ping? /me considers sending a one-patch pull request for an area he's absolutely not maintaining, but if this is the only way to get patches applied to qemu... I don't currently have a workflow for identifying and applying patches which aren't in pull requests (apart from obvious fixes build breakage patches, and even there it's depending on my happening to notice them). In this case I'd expect rth to put together a pull request, I guess. The problem is the I guess part, especially if Richard guesses otherwise. target-i386 happens to be an officially unmaintained area. This is the get_maintainer.pl output: qemu-devel@nongnu.org (odd fixer:X86) Richard Henderson r...@twiddle.net (commit_signer:123/126=98%) Peter Maydell peter.mayd...@linaro.org (commit_signer:51/126=40%) Paolo Bonzini pbonz...@redhat.com (commit_signer:32/126=25%) Blue Swirl blauwir...@gmail.com (commit_signer:13/126=10%) Richard, would you be willing to take up official maintainership to solve at least this uncertainty? Suggestions for better workflows welcome; we have had issues with patches falling through the gaps between maintained subsystems for a long time. Yes, we have a lot of code that doesn't fall in any subsystem with a subtree maintainer. This is the really worrying part here. I'm pretty sure I would get this specific patch merged the one or the other way (after all, my pull requests are generally accepted), but if even I fail to get it in using the normal way, it probably also means that contributors outside of the core team have no chance at all getting any patches in. This is alarming and certainly can't be healthy. I think Anthony did try to apply such patches that don't belong to any submaintainer's area (even though often with considerable delays), but I'm not sure how much time it cost him and how he managed to filter them. Anthony, any hints? Kevin
[Qemu-devel] [PATCH] target-i386: Fix I/O bitmap checks for in/out
Commit 1b90d56e changed the implementation of in/out imm to not assign the accessed port number to cpu_T[0] as it appeared unnecessary. However, currently gen_check_io() makes use of cpu_T[0] to implement the I/O bitmap checks, so it's in fact still used and the change broke the check, leading to #GP in legitimate cases (and probably also allowing access to ports that shouldn't be allowed). This patch reintroduces the missing assignment for these cases. Signed-off-by: Kevin Wolf kw...@redhat.com Reviewed-by: Richard Henderson r...@twiddle.net --- target-i386/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-i386/translate.c b/target-i386/translate.c index b0f2279..5dd2450 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -6284,6 +6284,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, case 0xe5: ot = mo_b_d32(b, dflag); val = cpu_ldub_code(env, s-pc++); +tcg_gen_movi_tl(cpu_T[0], val); gen_check_io(s, ot, pc_start - s-cs_base, SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes)); if (use_icount) @@ -6300,6 +6301,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, case 0xe7: ot = mo_b_d32(b, dflag); val = cpu_ldub_code(env, s-pc++); +tcg_gen_movi_tl(cpu_T[0], val); gen_check_io(s, ot, pc_start - s-cs_base, svm_is_rep(prefixes)); gen_op_mov_v_reg(ot, cpu_T[1], R_EAX); -- 1.8.1.4
