Why attempt to emulate these instructions at all, if we're not going
to handle a data access to emulated/special memory?
It seems that one of the following three cases must hold:
1) The data accessed by the instruction is emulated/special memory.
2) The instruction was fetched from emulated/special memory.
3) The instruction has been modified since the VM-exit.
The proposed patch is incorrect for case (1). Case (2) violates the
emulator's assumptions outlined in kvm_emulate.h. Case (3) seems best
handled by simply re-entering VMX non-root mode.
On Thu, Jan 12, 2017 at 5:40 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
>
> ----- Original Message -----
>> From: "Steve Rutherford" <srutherford@xxxxxxxxxx>
>> To: kvm@xxxxxxxxxxxxxxx
>> Cc: pbonzini@xxxxxxxxxx, dvyukov@xxxxxxxxxx, rkrcmar@xxxxxxxxxx, ppandit@xxxxxxxxxx, kernellwp@xxxxxxxxx
>> Sent: Thursday, January 12, 2017 3:28:29 AM
>> Subject: [PATCH] KVM: x86: Introduce segmented_write_std
>>
>> Introduces segemented_write_std.
>>
>> Switches from emulated reads/writes to standard read/writes in fxsave,
>> fxrstor, sgdt, and sidt.
>>
>> Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
>> Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> Signed-off-by: Steve Rutherford <srutherford@xxxxxxxxxx>
>> ---
>> arch/x86/kvm/emulate.c | 22 ++++++++++++++++++----
>> 1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index 2b8349a2b14b..ad258aa0b302 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -819,6 +819,20 @@ static int segmented_read_std(struct x86_emulate_ctxt
>> *ctxt,
>> return ctxt->ops->read_std(ctxt, linear, data, size, &ctxt->exception);
>> }
>>
>> +static int segmented_write_std(struct x86_emulate_ctxt *ctxt,
>> + struct segmented_address addr,
>> + void *data,
>> + unsigned int size)
>> +{
>> + int rc;
>> + ulong linear;
>> +
>> + rc = linearize(ctxt, addr, size, true, &linear);
>> + if (rc != X86EMUL_CONTINUE)
>> + return rc;
>> + return ctxt->ops->write_std(ctxt, linear, data, size, &ctxt->exception);
>> +}
>> +
>> /*
>> * Prefetch the remaining bytes of the instruction without crossing page
>> * boundary if they are not in fetch_cache yet.
>> @@ -3686,8 +3700,8 @@ static int emulate_store_desc_ptr(struct
>> x86_emulate_ctxt *ctxt,
>> }
>> /* Disable writeback. */
>> ctxt->dst.type = OP_NONE;
>> - return segmented_write(ctxt, ctxt->dst.addr.mem,
>> - &desc_ptr, 2 + ctxt->op_bytes);
>> + return segmented_write_std(ctxt, ctxt->dst.addr.mem,
>> + &desc_ptr, 2 + ctxt->op_bytes);
>> }
>>
>> static int em_sgdt(struct x86_emulate_ctxt *ctxt)
>> @@ -3933,7 +3947,7 @@ static int em_fxsave(struct x86_emulate_ctxt *ctxt)
>> else
>> size = offsetof(struct fxregs_state, xmm_space[0]);
>>
>> - return segmented_write(ctxt, ctxt->memop.addr.mem, &fx_state, size);
>> + return segmented_write_std(ctxt, ctxt->memop.addr.mem, &fx_state, size);
>> }
>>
>> static int fxrstor_fixup(struct x86_emulate_ctxt *ctxt,
>> @@ -3975,7 +3989,7 @@ static int em_fxrstor(struct x86_emulate_ctxt *ctxt)
>> if (rc != X86EMUL_CONTINUE)
>> return rc;
>>
>> - rc = segmented_read(ctxt, ctxt->memop.addr.mem, &fx_state, 512);
>> + rc = segmented_read_std(ctxt, ctxt->memop.addr.mem, &fx_state, 512);
>> if (rc != X86EMUL_CONTINUE)
>> return rc;
>>
>> --
>> 2.11.0.390.gc69c2f50cf-goog
>>
>>
>
> Queued for 4.10, thanks. At least fxsave/fxrstor is not in any
> released version, but that was close. I owe Dmitry some (more) beer.
>
> Paolo
> --
> 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
--
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
