| Message ID | 1356179217-5526-4-git-send-email-avi.kivity@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Dec 22, 2012 at 02:26:53PM +0200, Avi Kivity wrote: > Instead of disabling writeback via OP_NONE, just specify NoWrite. > > Signed-off-by: Avi Kivity <avi.kivity@gmail.com> > --- > arch/x86/kvm/emulate.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 9859df6..9dfbd07 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -150,6 +150,7 @@ > #define Unaligned ((u64)1 << 42) /* Explicitly unaligned (e.g. MOVDQU) */ > #define Avx ((u64)1 << 43) /* Advanced Vector Extensions */ > #define Fastop ((u64)1 << 44) /* Use opcode::u.fastop */ > +#define NoWrite ((u64)1 << 45) /* No writeback */ > > #define X2(x...) x, x > #define X3(x...) X2(x), x > @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt) > { > int rc; > > + if (ctxt->d & NoWrite) > + return X86EMUL_CONTINUE; > + Why not make it Dst flag and set dst.type to OP_NONE during decoding instead of this special case? > switch (ctxt->dst.type) { > case OP_REG: > write_register_operand(&ctxt->dst); > -- > 1.7.11.7 -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote: >> #define X2(x...) x, x >> #define X3(x...) X2(x), x >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt) >> { >> int rc; >> >> + if (ctxt->d & NoWrite) >> + return X86EMUL_CONTINUE; >> + > Why not make it Dst flag and set dst.type to OP_NONE during decoding > instead of this special case? My thinking was to eventually convert everything to NoWrite, then it's not a special case. But your approach works too, I can change it if you like. We'll have an if () either way though. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote: > On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote: > >> #define X2(x...) x, x > >> #define X3(x...) X2(x), x > >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt) > >> { > >> int rc; > >> > >> + if (ctxt->d & NoWrite) > >> + return X86EMUL_CONTINUE; > >> + > > Why not make it Dst flag and set dst.type to OP_NONE during decoding > > instead of this special case? > > My thinking was to eventually convert everything to NoWrite, then it's > not a special case. But your approach works too, I can change it if > you like. We'll have an if () either way though. But meanwhile we will have two different ways to disable writeback. When all instructions will be marked with NoWrite we can switch it easily if we want. Also some instruction can disable writeback conditionally. cmov is one (and may be only) of them. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov <gleb@redhat.com> wrote: > On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote: >> On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote: >> >> #define X2(x...) x, x >> >> #define X3(x...) X2(x), x >> >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt) >> >> { >> >> int rc; >> >> >> >> + if (ctxt->d & NoWrite) >> >> + return X86EMUL_CONTINUE; >> >> + >> > Why not make it Dst flag and set dst.type to OP_NONE during decoding >> > instead of this special case? >> >> My thinking was to eventually convert everything to NoWrite, then it's >> not a special case. But your approach works too, I can change it if >> you like. We'll have an if () either way though. > But meanwhile we will have two different ways to disable writeback. When > all instructions will be marked with NoWrite we can switch it easily if > we want. Also some instruction can disable writeback conditionally. cmov is one > (and may be only) of them. Makes sense, will update for the next iteration. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 22, 2012 at 5:45 PM, Avi Kivity <avi.kivity@gmail.com> wrote: > On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov <gleb@redhat.com> wrote: >> On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote: >>> On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote: >>> >> #define X2(x...) x, x >>> >> #define X3(x...) X2(x), x >>> >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt) >>> >> { >>> >> int rc; >>> >> >>> >> + if (ctxt->d & NoWrite) >>> >> + return X86EMUL_CONTINUE; >>> >> + >>> > Why not make it Dst flag and set dst.type to OP_NONE during decoding >>> > instead of this special case? >>> >>> My thinking was to eventually convert everything to NoWrite, then it's >>> not a special case. But your approach works too, I can change it if >>> you like. We'll have an if () either way though. >> But meanwhile we will have two different ways to disable writeback. When >> all instructions will be marked with NoWrite we can switch it easily if >> we want. Also some instruction can disable writeback conditionally. cmov is one >> (and may be only) of them. > > Makes sense, will update for the next iteration. Actually that doesn't work. x86_emulate_insn() needs dst.type in order to fetch the operand. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 22, 2012 at 05:51:08PM +0200, Avi Kivity wrote: > On Sat, Dec 22, 2012 at 5:45 PM, Avi Kivity <avi.kivity@gmail.com> wrote: > > On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov <gleb@redhat.com> wrote: > >> On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote: > >>> On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote: > >>> >> #define X2(x...) x, x > >>> >> #define X3(x...) X2(x), x > >>> >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt) > >>> >> { > >>> >> int rc; > >>> >> > >>> >> + if (ctxt->d & NoWrite) > >>> >> + return X86EMUL_CONTINUE; > >>> >> + > >>> > Why not make it Dst flag and set dst.type to OP_NONE during decoding > >>> > instead of this special case? > >>> > >>> My thinking was to eventually convert everything to NoWrite, then it's > >>> not a special case. But your approach works too, I can change it if > >>> you like. We'll have an if () either way though. > >> But meanwhile we will have two different ways to disable writeback. When > >> all instructions will be marked with NoWrite we can switch it easily if > >> we want. Also some instruction can disable writeback conditionally. cmov is one > >> (and may be only) of them. > > > > Makes sense, will update for the next iteration. > > Actually that doesn't work. x86_emulate_insn() needs dst.type in order > to fetch the operand. True :(. Actually if we will move from OP_NONE to NoWrite we can drop saved_dst_type hack from x86_emulate_insn. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 9859df6..9dfbd07 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -150,6 +150,7 @@ #define Unaligned ((u64)1 << 42) /* Explicitly unaligned (e.g. MOVDQU) */ #define Avx ((u64)1 << 43) /* Advanced Vector Extensions */ #define Fastop ((u64)1 << 44) /* Use opcode::u.fastop */ +#define NoWrite ((u64)1 << 45) /* No writeback */ #define X2(x...) x, x #define X3(x...) X2(x), x @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt) { int rc; + if (ctxt->d & NoWrite) + return X86EMUL_CONTINUE; + switch (ctxt->dst.type) { case OP_REG: write_register_operand(&ctxt->dst);
Instead of disabling writeback via OP_NONE, just specify NoWrite. Signed-off-by: Avi Kivity <avi.kivity@gmail.com> --- arch/x86/kvm/emulate.c | 4 ++++ 1 file changed, 4 insertions(+)