Re: [Xen-devel] [PATCH 2/2] x86/VMX: don't needlessly install VMFUNC emulation hook

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

> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Thursday, December 22, 2016 11:37 PM
> 
> >>> On 22.12.16 at 16:14, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 22/12/16 14:58, Jan Beulich wrote:
> >>>>> On 22.12.16 at 15:31, <JBeulich@xxxxxxxx> wrote:
> >>>>>> On 22.12.16 at 14:47, <andrew.cooper3@xxxxxxxxxx> wrote:
> >>>> On 22/12/16 08:37, Jan Beulich wrote:
> >>>>> Instead of checking cpu_has_vmx_vmfunc inside the hook, use it to
> >>>>> determine whether to install the hook in the first place.
> >>>>>
> >>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >>>> I am not so sure about this.
> >>>>
> >>>> vmfunc is reachable in the instruction emulator on hardware which
> >>>> doesn't support vmfunc, and there is explicit provision for using vmfunc
> >>>> 0 via hypercall on hardware lacking vmfunc support.
> >>>>
> >>>> Given that the #VE part of altp2m is always emulated architecturally, I
> >>>> think there is an argument to be made for also emulating EPTP switching
> >>>> architecturally as well.
> >>> I don't understand this argumentation: Without the patch, the
> >>> hook function checks !cpu_has_vmx_vmfunc (and fails otherwise);
> >>> with the patch the hook isn't being put in place when
> >>> !cpu_has_vmx_vmfunc, and failure occurs in hvmemul_vmfunc().
> >>> I admit there's the difference in error codes, but we could
> >>> certainly make hvmemul_vmfunc() return EXCEPTION when
> >>> there's no hook.
> >> And btw., installing altp2m_vcpu_update_vmfunc_ve is as pointless
> >> in the opposite case, do it bailing early when !cpu_has_vmx_vmfunc.
> >> I guess I'll do both changes for a v2.
> >
> > My argument is that, instead of excluding the hook, the behaviour of the
> > emulation path should be made to function sensibly even on hardware
> > without vmfunc.
> >
> > i.e. drop the cpu_has_vmx_vmfunc check and do nothing else.
> 
> Ah, I see. I guess I'll leave that to someone having an environment
> to test this. The patch's goal was to not change observable behavior.
> 

If later we'll have to again get hook always installed (for hardware w/o 
vmfunc), I'm hesitant to ack this version...

Thanks
Kevin

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