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

[Jan Beulich]

>>> "Tian, Kevin"  12/23/16 6:48 AM >>>
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Thursday, December 22, 2016 11:37 PM
>> >>> On 22.12.16 at 16:14,  wrote:
>> > 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...

While I can understand that response of yours, may I then ask that you
(the VMX maintainers) submit a patch to follow Andrew's outline (unless
he means to do so himself).

Jan


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

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

[Tian, Kevin]

> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Thursday, December 22, 2016 11:37 PM
> 
> >>> On 22.12.16 at 16:14,  wrote:
> > On 22/12/16 14:58, Jan Beulich wrote:
> > On 22.12.16 at 15:31,  wrote:
> >> On 22.12.16 at 14:47,  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 
>  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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Jan Beulich]

>>> On 22.12.16 at 16:14,  wrote:
> On 22/12/16 14:58, Jan Beulich wrote:
> On 22.12.16 at 15:31,  wrote:
>> On 22.12.16 at 14:47,  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 
 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.

Jan


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

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

[Andrew Cooper]

On 22/12/16 14:58, Jan Beulich wrote:

On 22.12.16 at 15:31,  wrote:

On 22.12.16 at 14:47,  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 

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.

~Andrew



Jan




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

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

[Jan Beulich]

>>> On 22.12.16 at 15:31,  wrote:
 On 22.12.16 at 14:47,  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 
>> 
>> 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.

Jan


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

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

[Jan Beulich]

>>> On 22.12.16 at 14:47,  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 
> 
> 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.

Jan


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

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

[Andrew Cooper]

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 


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.


~Andrew

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