>> The other motivation is that KVM can opportunistically start dumping extra info >> for old VMMs, though this patch does not do that; feedback imminent. :-) > > It's nothing more than that the interface ends up feeling a little > strange. With several flags added and some of the earlier flags unused, > ndata ends up indicating the largest extent of the flag-indicated data, > but the earlier elements of the structure are unused. Hence the question > about how many flags we anticipate using simultaneously. > > (I'm not really arguing that we should be packing the stuff in and > having to decode it, as that is also unpleasant.) It's hard to say how many flags will be used, but I suspect it will be a small set (I mentioned previously a possibility of 2 more), so I like Sean's suggestion to keep it simple and just use the flags to indicate which fields are used, instead of trying to pack everything in tightly. On Wed, Apr 21, 2021 at 1:39 AM David Edmondson <dme@xxxxxxx> wrote: > > On Tuesday, 2021-04-20 at 18:34:48 UTC, Sean Christopherson wrote: > > > On Fri, Apr 16, 2021, Aaron Lewis wrote: > >> + 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. > > And zero out the rest? > I left the memcpy alone as this seems like a more concervative / safer approach. I've had and have seen too many memcpy fails that my preference is to be conservative here. I could add a comment to clarify any confusion that this may bring by not using insn_size if that will help. Cheers, Aaron
