From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] x86emul: don't unconditionally clear segment bases upon null selector loads Date: Tue, 20 Dec 2016 01:18:23 -0700 Message-ID: <5858F75F020000780012ACDB@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__PartDEE7D35F.1__=" 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 1cJFcz-0005Zb-FU for xen-devel@lists.xenproject.org; Tue, 20 Dec 2016 08:18:29 +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 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. --=__PartDEE7D35F.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline AMD explicitly documents that namely FS and GS don't have their bases cleared in that case, and I see no reason why guests may not rely on that behavior. To facilitate this a new input field (the CPU vendor) is being added. Signed-off-by: Jan Beulich --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -119,6 +119,7 @@ int main(int argc, char **argv) =20 ctxt.regs =3D ®s; ctxt.force_writeback =3D 0; + ctxt.vendor =3D X86_VENDOR_UNKNOWN; ctxt.addr_size =3D 8 * sizeof(void *); ctxt.sp_size =3D 8 * sizeof(void *); =20 --- a/tools/tests/x86_emulator/x86_emulate.h +++ b/tools/tests/x86_emulator/x86_emulate.h @@ -38,6 +38,11 @@ =20 #define is_canonical_address(x) (((int64_t)(x) >> 47) =3D=3D ((int64_t)(x)= >> 63)) =20 +/* There's no strict need for these to be in sync with processor.h. */ +#define X86_VENDOR_INTEL 0 +#define X86_VENDOR_AMD 2 +#define X86_VENDOR_UNKNOWN 0xff + #define MMAP_SZ 16384 bool emul_test_make_stack_executable(void); =20 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -1897,6 +1897,7 @@ void hvm_emulate_init_once( hvmemul_get_seg_reg(x86_seg_ss, hvmemul_ctxt); =20 hvmemul_ctxt->ctxt.regs =3D regs; + hvmemul_ctxt->ctxt.vendor =3D current->domain->arch.x86_vendor; hvmemul_ctxt->ctxt.force_writeback =3D true; =20 if ( cpu_has_vmx ) --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -5358,6 +5358,7 @@ int ptwr_do_page_fault(struct vcpu *v, u struct ptwr_emulate_ctxt ptwr_ctxt =3D { .ctxt =3D { .regs =3D regs, + .vendor =3D d->arch.x86_vendor, .addr_size =3D is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG, .sp_size =3D is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG, .swint_emulate =3D x86_swint_emulate_none, @@ -5513,6 +5514,7 @@ int mmio_ro_do_page_fault(struct vcpu *v struct mmio_ro_emulate_ctxt mmio_ro_ctxt =3D { .cr2 =3D addr }; struct x86_emulate_ctxt ctxt =3D { .regs =3D regs, + .vendor =3D v->domain->arch.x86_vendor, .addr_size =3D addr_size, .sp_size =3D addr_size, .swint_emulate =3D x86_swint_emulate_none, --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -330,6 +330,7 @@ const struct x86_emulate_ops *shadow_ini memset(sh_ctxt, 0, sizeof(*sh_ctxt)); =20 sh_ctxt->ctxt.regs =3D regs; + sh_ctxt->ctxt.vendor =3D v->domain->arch.x86_vendor; sh_ctxt->ctxt.swint_emulate =3D x86_swint_emulate_none; =20 /* Segment cache initialisation. Primed with CS. */ --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -3352,7 +3352,10 @@ static int emulate_privileged_op(struct { struct vcpu *curr =3D current; struct domain *currd =3D curr->domain; - struct priv_op_ctxt ctxt =3D { .ctxt.regs =3D regs }; + struct priv_op_ctxt ctxt =3D { + .ctxt.regs =3D regs, + .ctxt.vendor =3D currd->arch.x86_vendor, + }; int rc; unsigned int eflags, ar; =20 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -1395,7 +1395,11 @@ protmode_load_seg( case x86_seg_tr: goto raise_exn; } - memset(sreg, 0, sizeof(*sreg)); + if ( ctxt->vendor !=3D X86_VENDOR_AMD || !ops->read_segment || + ops->read_segment(seg, sreg, ctxt) !=3D X86EMUL_OKAY ) + memset(sreg, 0, sizeof(*sreg)); + else + sreg->attr.bytes =3D 0; sreg->sel =3D sel; return X86EMUL_OKAY; } --- a/xen/arch/x86/x86_emulate/x86_emulate.h +++ b/xen/arch/x86/x86_emulate/x86_emulate.h @@ -462,6 +462,9 @@ struct x86_emulate_ctxt /* Software event injection support. */ enum x86_swint_emulation swint_emulate; =20 + /* CPU vendor (X86_VENDOR_UNKNOWN for "don't care") */ + unsigned char vendor; + /* Set this if writes may have side effects. */ bool force_writeback; =20 --=__PartDEE7D35F.1__= Content-Type: text/plain; name="x86emul-null-sel-AMD.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86emul-null-sel-AMD.patch" x86emul: don't unconditionally clear segment bases upon null selector = loads=0A=0AAMD explicitly documents that namely FS and GS don't have their = bases=0Acleared in that case, and I see no reason why guests may not rely = on=0Athat behavior. To facilitate this a new input field (the CPU vendor) = is=0Abeing added.=0A=0ASigned-off-by: Jan Beulich =0A=0A= --- a/tools/tests/x86_emulator/test_x86_emulator.c=0A+++ b/tools/tests/x86_= emulator/test_x86_emulator.c=0A@@ -119,6 +119,7 @@ int main(int argc, char = **argv)=0A =0A ctxt.regs =3D ®s;=0A ctxt.force_writeback =3D = 0;=0A+ ctxt.vendor =3D X86_VENDOR_UNKNOWN;=0A ctxt.addr_size =3D = 8 * sizeof(void *);=0A ctxt.sp_size =3D 8 * sizeof(void *);=0A = =0A--- a/tools/tests/x86_emulator/x86_emulate.h=0A+++ b/tools/tests/x86_emu= lator/x86_emulate.h=0A@@ -38,6 +38,11 @@=0A =0A #define is_canonical_addres= s(x) (((int64_t)(x) >> 47) =3D=3D ((int64_t)(x) >> 63))=0A =0A+/* There's = no strict need for these to be in sync with processor.h. */=0A+#define = X86_VENDOR_INTEL 0=0A+#define X86_VENDOR_AMD 2=0A+#define X86_VENDOR_= UNKNOWN 0xff=0A+=0A #define MMAP_SZ 16384=0A bool emul_test_make_stack_exec= utable(void);=0A =0A--- a/xen/arch/x86/hvm/emulate.c=0A+++ b/xen/arch/x86/h= vm/emulate.c=0A@@ -1897,6 +1897,7 @@ void hvm_emulate_init_once(=0A = hvmemul_get_seg_reg(x86_seg_ss, hvmemul_ctxt);=0A =0A hvmemul_ctxt->ctx= t.regs =3D regs;=0A+ hvmemul_ctxt->ctxt.vendor =3D current->domain->arch= .x86_vendor;=0A hvmemul_ctxt->ctxt.force_writeback =3D true;=0A =0A = if ( cpu_has_vmx )=0A--- a/xen/arch/x86/mm.c=0A+++ b/xen/arch/x86/mm.c=0A@= @ -5358,6 +5358,7 @@ int ptwr_do_page_fault(struct vcpu *v, u=0A = struct ptwr_emulate_ctxt ptwr_ctxt =3D {=0A .ctxt =3D {=0A = .regs =3D regs,=0A+ .vendor =3D d->arch.x86_vendor,=0A = .addr_size =3D is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG,=0A = .sp_size =3D is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG,=0A = .swint_emulate =3D x86_swint_emulate_none,=0A@@ -5513,6 +5514,7 @@ = int mmio_ro_do_page_fault(struct vcpu *v=0A struct mmio_ro_emulate_ctxt= mmio_ro_ctxt =3D { .cr2 =3D addr };=0A struct x86_emulate_ctxt ctxt = =3D {=0A .regs =3D regs,=0A+ .vendor =3D v->domain->arch.x86= _vendor,=0A .addr_size =3D addr_size,=0A .sp_size =3D = addr_size,=0A .swint_emulate =3D x86_swint_emulate_none,=0A--- = a/xen/arch/x86/mm/shadow/common.c=0A+++ b/xen/arch/x86/mm/shadow/common.c= =0A@@ -330,6 +330,7 @@ const struct x86_emulate_ops *shadow_ini=0A = memset(sh_ctxt, 0, sizeof(*sh_ctxt));=0A =0A sh_ctxt->ctxt.regs =3D = regs;=0A+ sh_ctxt->ctxt.vendor =3D v->domain->arch.x86_vendor;=0A = sh_ctxt->ctxt.swint_emulate =3D x86_swint_emulate_none;=0A =0A /* = Segment cache initialisation. Primed with CS. */=0A--- a/xen/arch/x86/traps= .c=0A+++ b/xen/arch/x86/traps.c=0A@@ -3352,7 +3352,10 @@ static int = emulate_privileged_op(struct=0A {=0A struct vcpu *curr =3D current;=0A = struct domain *currd =3D curr->domain;=0A- struct priv_op_ctxt ctxt = =3D { .ctxt.regs =3D regs };=0A+ struct priv_op_ctxt ctxt =3D {=0A+ = .ctxt.regs =3D regs,=0A+ .ctxt.vendor =3D currd->arch.x86_vendor,= =0A+ };=0A int rc;=0A unsigned int eflags, ar;=0A =0A--- = a/xen/arch/x86/x86_emulate/x86_emulate.c=0A+++ b/xen/arch/x86/x86_emulate/x= 86_emulate.c=0A@@ -1395,7 +1395,11 @@ protmode_load_seg(=0A case = x86_seg_tr:=0A goto raise_exn;=0A }=0A- = memset(sreg, 0, sizeof(*sreg));=0A+ if ( ctxt->vendor !=3D = X86_VENDOR_AMD || !ops->read_segment ||=0A+ ops->read_segment(s= eg, sreg, ctxt) !=3D X86EMUL_OKAY )=0A+ memset(sreg, 0, = sizeof(*sreg));=0A+ else=0A+ sreg->attr.bytes =3D 0;=0A = sreg->sel =3D sel;=0A return X86EMUL_OKAY;=0A }=0A--- = a/xen/arch/x86/x86_emulate/x86_emulate.h=0A+++ b/xen/arch/x86/x86_emulate/x= 86_emulate.h=0A@@ -462,6 +462,9 @@ struct x86_emulate_ctxt=0A /* = Software event injection support. */=0A enum x86_swint_emulation = swint_emulate;=0A =0A+ /* CPU vendor (X86_VENDOR_UNKNOWN for "don't = care") */=0A+ unsigned char vendor;=0A+=0A /* Set this if writes = may have side effects. */=0A bool force_writeback;=0A =0A --=__PartDEE7D35F.1__= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --=__PartDEE7D35F.1__=--