From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v4 07/15] VMX: add VMFUNC leaf 0 (EPTP switching) to emulator. Date: Mon, 13 Jul 2015 08:13:34 +0100 Message-ID: <55A3813E020000780008FF02@mail.emea.novell.com> References: <1436489553-6300-1-git-send-email-edmund.h.white@intel.com> <1436489553-6300-8-git-send-email-edmund.h.white@intel.com> <559FACE0020000780008F53F@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ravi Sahita Cc: Tim Deegan , Wei Liu , George Dunlap , Andrew Cooper , Ian Jackson , Edmund H White , "xen-devel@lists.xen.org" , "tlengyel@novetta.com" , Daniel De Graaf List-Id: xen-devel@lists.xenproject.org >>> On 11.07.15 at 22:01, wrote: >> From: Jan Beulich [mailto:JBeulich@suse.com] >>Sent: Friday, July 10, 2015 2:31 AM >> >>>>> On 10.07.15 at 02:52, wrote: >>> @@ -3234,6 +3256,13 @@ void vmx_vmexit_handler(struct cpu_user_regs >>*regs) >>> update_guest_eip(); >>> break; >>> >>> + case EXIT_REASON_VMFUNC: >>> + if ( vmx_vmfunc_intercept(regs) == X86EMUL_EXCEPTION ) >>> + hvm_inject_hw_exception(TRAP_invalid_op, >>HVM_DELIVER_NO_ERROR_CODE); >>> + else >>> + update_guest_eip(); >>> + break; >> >>How about X86EMUL_UNHANDLEABLE and X86EMUL_RETRY? As said before, >>either get this right, or simply fold the relatively pointless helper into > here. > > Sure I can add the other error conditions but note that they will be handled > as EXCEPTION. The reason for this would need to go into ... > Let me explain the point of the relatively pointless :-) helper > was to have the interface complete so that if someone in the future wanted to > handle VMFUNC exits (perhaps for lazily managing EPTP list for nesting > scenarios) they could do that by extending the vmx_vmfunc_intercept. I can > also add a comment there - Will that be sufficient? (I'm trying to avoid > another revision after I revise it to add the other exception conditions as > stated) ... such a comment. And yes, I'd be as fine with just a comment as with the wrapper being folded in. >>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >>> @@ -3816,8 +3816,9 @@ x86_emulate( >>> struct segment_register reg; >>> unsigned long base, limit, cr0, cr0w; >>> >>> - if ( modrm == 0xdf ) /* invlpga */ >>> + switch( modrm ) >>> { >>> + case 0xdf: /* invlpga AMD */ >>> generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1); >>> generate_exception_if(!mode_ring0(), EXC_GP, 0); >>> fail_if(ops->invlpg == NULL); >> >>The diff now looks much better. Yet I don't see why you added "AMD" >>to the comment - we don't elsewhere note that certain instructions are >>vendor specific (and really which ones are also changes over time, see RDTSCP >>for a prominent example). >> > > I thought it would be better to specify instructions that are unique to a > specific CPU. > But I can remove it. Yes please. Jan