Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag
On Sat, Dec 22, 2012 at 05:51:08PM +0200, Avi Kivity wrote:
> On Sat, Dec 22, 2012 at 5:45 PM, Avi Kivity wrote:
> > On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov 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 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag
On Sat, Dec 22, 2012 at 5:45 PM, Avi Kivity wrote:
> On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov 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 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag
On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov 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 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag
On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote:
> On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag
On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag
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
> ---
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
