On Tue, Apr 20, 2021 at 11:34 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Fri, Apr 16, 2021, Aaron Lewis wrote:
...
> > + vcpu->run->emulation_failure.suberror = KVM_INTERNAL_ERROR_EMULATION;
> > + vcpu->run->emulation_failure.ndata = 0;
> > + if (kvm->arch.exit_on_emulation_error && insn_size > 0) {
>
> I definitely think this should not be conditioned on exit_on_emulation_error.
>
> No need for "> 0", it's an unsigned value.
Unsigned doesn't imply non-zero. If insn_size is 0, then something is wrong.
...
> > + KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES;
> > + vcpu->run->emulation_failure.insn_size = insn_size;
> > + memcpy(vcpu->run->emulation_failure.insn_bytes,
> > + ctxt->fetch.data, sizeof(ctxt->fetch.data));
>
> Doesn't truly matter, but I think it's less confusing to copy over insn_size
> bytes.
Are you convinced that insn_size is always less than or equal to
sizeof(ctxt->fetch.data)? I'm not. It shouldn't be, of course, but
perhaps we should confirm that before copying insn_size bytes.
