From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] x86/HVM: correct notion of new CPL in task switch emulation Date: Thu, 01 Jun 2017 06:11:36 -0600 Message-ID: <59302098020000780015EB40@prv-mh.provo.novell.com> References: <59302098020000780015EB40@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part6D550968.2__=" Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dGOx2-00085O-BA for xen-devel@lists.xenproject.org; Thu, 01 Jun 2017 12:11:40 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: xen-devel Cc: Andrew Cooper , Julien Grall List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part6D550968.2__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Commit aac1df3d03 ("x86/HVM: introduce hvm_get_cpl() and respective hook") went too far in one aspect: When emulating a task switch we really shouldn't be looking at what hvm_get_cpl() returns, as we're switching all segment registers. However, instead of reverting the relevant parts of that commit, have the caller tell the segment loading function what the new CPL is. This at once fixes ES being loaded before CS so far having had its checks done against the old CPL. Reported-by: Andrew Cooper Signed-off-by: Jan Beulich --- An alternative to adding yet another parameter to the function would be to have "cpl" have a special case value (e.g. negative) to indicate VM86 mode. That would allow replacing the current "eflags" parameter. --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2616,11 +2616,11 @@ static void hvm_unmap_entry(void *p) } =20 static int hvm_load_segment_selector( - enum x86_segment seg, uint16_t sel, unsigned int eflags) + enum x86_segment seg, uint16_t sel, unsigned int cpl, unsigned int = eflags) { struct segment_register desctab, segr; struct desc_struct *pdesc, desc; - u8 dpl, rpl, cpl; + u8 dpl, rpl; bool_t writable; int fault_type =3D TRAP_invalid_tss; struct vcpu *v =3D current; @@ -2674,7 +2674,6 @@ static int hvm_load_segment_selector( =20 dpl =3D (desc.b >> 13) & 3; rpl =3D sel & 3; - cpl =3D hvm_get_cpl(v); =20 switch ( seg ) { @@ -2804,7 +2803,7 @@ void hvm_task_switch( struct segment_register gdt, tr, prev_tr, segr; struct desc_struct *optss_desc =3D NULL, *nptss_desc =3D NULL, = tss_desc; bool_t otd_writable, ntd_writable; - unsigned int eflags; + unsigned int eflags, new_cpl; pagefault_info_t pfinfo; int exn_raised, rc; struct tss32 tss; @@ -2918,7 +2917,9 @@ void hvm_task_switch( if ( rc !=3D HVMCOPY_okay ) goto out; =20 - if ( hvm_load_segment_selector(x86_seg_ldtr, tss.ldt, 0) ) + new_cpl =3D tss.eflags & X86_EFLAGS_VM ? 3 : tss.cs & 3; + + if ( hvm_load_segment_selector(x86_seg_ldtr, tss.ldt, new_cpl, 0) ) goto out; =20 rc =3D hvm_set_cr3(tss.cr3, 1); @@ -2939,12 +2940,12 @@ void hvm_task_switch( regs->rdi =3D tss.edi; =20 exn_raised =3D 0; - if ( hvm_load_segment_selector(x86_seg_es, tss.es, tss.eflags) || - hvm_load_segment_selector(x86_seg_cs, tss.cs, tss.eflags) || - hvm_load_segment_selector(x86_seg_ss, tss.ss, tss.eflags) || - hvm_load_segment_selector(x86_seg_ds, tss.ds, tss.eflags) || - hvm_load_segment_selector(x86_seg_fs, tss.fs, tss.eflags) || - hvm_load_segment_selector(x86_seg_gs, tss.gs, tss.eflags) ) + if ( hvm_load_segment_selector(x86_seg_es, tss.es, new_cpl, tss.eflags= ) || + hvm_load_segment_selector(x86_seg_cs, tss.cs, new_cpl, tss.eflags= ) || + hvm_load_segment_selector(x86_seg_ss, tss.ss, new_cpl, tss.eflags= ) || + hvm_load_segment_selector(x86_seg_ds, tss.ds, new_cpl, tss.eflags= ) || + hvm_load_segment_selector(x86_seg_fs, tss.fs, new_cpl, tss.eflags= ) || + hvm_load_segment_selector(x86_seg_gs, tss.gs, new_cpl, tss.eflags= ) ) exn_raised =3D 1; =20 if ( taskswitch_reason =3D=3D TSW_call_or_int ) --=__Part6D550968.2__= Content-Type: text/plain; name="x86-HVM-task-switch-CPL.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86-HVM-task-switch-CPL.patch" x86/HVM: correct notion of new CPL in task switch emulation=0A=0ACommit = aac1df3d03 ("x86/HVM: introduce hvm_get_cpl() and respective=0Ahook") went = too far in one aspect: When emulating a task switch we=0Areally shouldn't = be looking at what hvm_get_cpl() returns, as we're=0Aswitching all segment = registers.=0A=0AHowever, instead of reverting the relevant parts of that = commit, have=0Athe caller tell the segment loading function what the new = CPL is. This=0Aat once fixes ES being loaded before CS so far having had = its checks=0Adone against the old CPL.=0A=0AReported-by: Andrew Cooper = =0ASigned-off-by: Jan Beulich =0A---=0AAn alternative to adding yet another parameter to the function = would=0Abe to have "cpl" have a special case value (e.g. negative) to = indicate=0AVM86 mode. That would allow replacing the current "eflags" = parameter.=0A=0A--- a/xen/arch/x86/hvm/hvm.c=0A+++ b/xen/arch/x86/hvm/hvm.c= =0A@@ -2616,11 +2616,11 @@ static void hvm_unmap_entry(void *p)=0A }=0A = =0A static int hvm_load_segment_selector(=0A- enum x86_segment seg, = uint16_t sel, unsigned int eflags)=0A+ enum x86_segment seg, uint16_t = sel, unsigned int cpl, unsigned int eflags)=0A {=0A struct segment_regi= ster desctab, segr;=0A struct desc_struct *pdesc, desc;=0A- u8 dpl, = rpl, cpl;=0A+ u8 dpl, rpl;=0A bool_t writable;=0A int fault_type= =3D TRAP_invalid_tss;=0A struct vcpu *v =3D current;=0A@@ -2674,7 = +2674,6 @@ static int hvm_load_segment_selector(=0A =0A dpl =3D = (desc.b >> 13) & 3;=0A rpl =3D sel & 3;=0A- cpl =3D = hvm_get_cpl(v);=0A =0A switch ( seg )=0A {=0A@@ -2804,7 = +2803,7 @@ void hvm_task_switch(=0A struct segment_register gdt, tr, = prev_tr, segr;=0A struct desc_struct *optss_desc =3D NULL, *nptss_desc = =3D NULL, tss_desc;=0A bool_t otd_writable, ntd_writable;=0A- = unsigned int eflags;=0A+ unsigned int eflags, new_cpl;=0A pagefault_= info_t pfinfo;=0A int exn_raised, rc;=0A struct tss32 tss;=0A@@ = -2918,7 +2917,9 @@ void hvm_task_switch(=0A if ( rc !=3D HVMCOPY_okay = )=0A goto out;=0A =0A- if ( hvm_load_segment_selector(x86_seg_ld= tr, tss.ldt, 0) )=0A+ new_cpl =3D tss.eflags & X86_EFLAGS_VM ? 3 : = tss.cs & 3;=0A+=0A+ if ( hvm_load_segment_selector(x86_seg_ldtr, = tss.ldt, new_cpl, 0) )=0A goto out;=0A =0A rc =3D hvm_set_cr3(t= ss.cr3, 1);=0A@@ -2939,12 +2940,12 @@ void hvm_task_switch(=0A = regs->rdi =3D tss.edi;=0A =0A exn_raised =3D 0;=0A- if ( = hvm_load_segment_selector(x86_seg_es, tss.es, tss.eflags) ||=0A- = hvm_load_segment_selector(x86_seg_cs, tss.cs, tss.eflags) ||=0A- = hvm_load_segment_selector(x86_seg_ss, tss.ss, tss.eflags) ||=0A- = hvm_load_segment_selector(x86_seg_ds, tss.ds, tss.eflags) ||=0A- = hvm_load_segment_selector(x86_seg_fs, tss.fs, tss.eflags) ||=0A- = hvm_load_segment_selector(x86_seg_gs, tss.gs, tss.eflags) )=0A+ if ( = hvm_load_segment_selector(x86_seg_es, tss.es, new_cpl, tss.eflags) ||=0A+ = hvm_load_segment_selector(x86_seg_cs, tss.cs, new_cpl, tss.eflags) = ||=0A+ hvm_load_segment_selector(x86_seg_ss, tss.ss, new_cpl, = tss.eflags) ||=0A+ hvm_load_segment_selector(x86_seg_ds, tss.ds, = new_cpl, tss.eflags) ||=0A+ hvm_load_segment_selector(x86_seg_fs, = tss.fs, new_cpl, tss.eflags) ||=0A+ hvm_load_segment_selector(x86_s= eg_gs, tss.gs, new_cpl, tss.eflags) )=0A exn_raised =3D 1;=0A =0A = if ( taskswitch_reason =3D=3D TSW_call_or_int )=0A --=__Part6D550968.2__= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --=__Part6D550968.2__=--