On Wed, Aug 19, 2009 at 11:32 AM, Avi Kivity<avi@xxxxxxxxxx> wrote:
> On 08/19/2009 07:11 AM, Mohammed Gamal wrote:
>>
>> +static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
>> +{
>> + struct decode_cache *c =&ctxt->decode;
>> + struct kvm_segment segment;
>> +
>> + if (ctxt->mode == X86EMUL_MODE_PROT64&& (seg != VCPU_SREG_FS ||
>> + seg != VCPU_SREG_GS)) {
>> + kvm_queue_exception(ctxt->vcpu, UD_VECTOR);
>> + return;
>> + }
>>
>
> It's better to check at the callsite, in case the opcode is ever reused for
> a new instruction. Or even better, add a new decode flag No64 so we can do
> this during the decode stage.
Good idea, but I believe it'd be better to introduce it in a separate
patch so that we can update all instructions incompatible with long
mode in one go. I'll move the checks to the call site for the time
being.
>
>> +
>> +static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
>> + struct x86_emulate_ops *ops, int seg)
>> +{
>> + struct decode_cache *c =&ctxt->decode;
>> + struct kvm_segment segment;
>> + int rc;
>> +
>> + if (ctxt->mode == X86EMUL_MODE_PROT64&& (seg != VCPU_SREG_FS ||
>> + seg != VCPU_SREG_GS))
>> {
>> + kvm_queue_exception(ctxt->vcpu, UD_VECTOR);
>> + return -1;
>> + }
>> +
>> + kvm_x86_ops->get_segment(ctxt->vcpu,&segment, seg);
>> + rc = emulate_pop(ctxt, ops,&segment.selector, c->op_bytes);
>> + if (rc != 0)
>> + return rc;
>> +
>> + rc = kvm_load_segment_descriptor(ctxt->vcpu, segment.selector, 1,
>> seg);
>> + return rc;
>> +}
>>
>
> Why do the ->get_segment() at all? pop into a temporary variable, and call
> kvm_load_segment_descriptor() with that.
>
>
> --
> error compiling committee.c: too many arguments to function
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
