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

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> 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...

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