Re: [Xen-devel] [PATCH v2 2/2] x86/vm_event: Allow overwriting Xen's i-cache used for emulation

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Mon, Sep 19, 2016 at 6:36 PM, Tian, Kevin <kevin.tian@xxxxxxxxx> wrote:
>> From: Tamas K Lengyel
>> Sent: Tuesday, September 20, 2016 12:40 AM
>>
>> >> --- a/xen/arch/x86/hvm/hvm.c
>> >> +++ b/xen/arch/x86/hvm/hvm.c
>> >> @@ -489,13 +489,16 @@ void hvm_do_resume(struct vcpu *v)
>> >>
>> >>              if ( v->arch.vm_event->emulate_flags &
>> >>                   VM_EVENT_FLAG_SET_EMUL_READ_DATA )
>> >> -                kind = EMUL_KIND_SET_CONTEXT;
>> >> +                kind = EMUL_KIND_SET_CONTEXT_DATA;
>> >>              else if ( v->arch.vm_event->emulate_flags &
>> >>                        VM_EVENT_FLAG_EMULATE_NOWRITE )
>> >>                  kind = EMUL_KIND_NOWRITE;
>> >> +            else if ( v->arch.vm_event->emulate_flags &
>> >> +                 VM_EVENT_FLAG_SET_EMUL_INSN_DATA )
>> >> +                kind = EMUL_KIND_SET_CONTEXT_INSN;
>> >
>> > The header talking about incompatibilities between these flags -
>> > wouldn't it be a good idea to actually -EINVAL such combinations?
>>
>> The header just points out that setting all these flags at the same
>> time won't have a "combined" effect - evident from the if-else
>> treatment above. There is really no point to do an error, the error
>> would never reach the user. Setting response flags through vm_event
>> doesn't have an error-path back.
>>
>
> Maybe you can have an explicit comment on priority of those flags,
> so consistent behavior can be expected moving forward. e.g. above
> code implies read_data>nowrite>insn_data. w/o a proper guidance
> here, future code changes by others may break that sequence...
>

Fair point, will do that.

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel