From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] x86: always supply .cpuid() handler to x86_emulate() Date: Thu, 10 Nov 2016 05:30:25 -0700 Message-ID: <58247671020000780011DA5A@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part0D347C71.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 1c4oUw-00007j-BM for xen-devel@lists.xenproject.org; Thu, 10 Nov 2016 12:30:30 +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 , Paul Durrant , Wei Liu , Tim Deegan 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. --=__Part0D347C71.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline With us incremementally adding proper CPUID checks to x86_emulate() (see commit de05bd965a ["x86emul: correct {,F}CMOV and F{,U}COMI{,P} emulation"]) it is no longer appropriate to invoke the function with that hook being NULL, as long as respective instructions may get used in that case. Signed-off-by: Jan Beulich --- I've noticed the problem first with a not yet submitted patch checking cx8 (PV 32-bit kernels occasionally use cmpxchg8b for page table accesses), and I think we don't strictly need the change here for 4.8, but then again it may be better to handle this properly right away. --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -1542,7 +1542,7 @@ static int hvmemul_wbinvd( return X86EMUL_OKAY; } =20 -static int hvmemul_cpuid( +int hvmemul_cpuid( unsigned int *eax, unsigned int *ebx, unsigned int *ecx, @@ -1892,11 +1892,13 @@ int hvm_emulate_one_mmio(unsigned long m .read =3D x86emul_unhandleable_rw, .insn_fetch =3D hvmemul_insn_fetch, .write =3D mmcfg_intercept_write, + .cpuid =3D hvmemul_cpuid, }; static const struct x86_emulate_ops hvm_ro_emulate_ops_mmio =3D { .read =3D x86emul_unhandleable_rw, .insn_fetch =3D hvmemul_insn_fetch, - .write =3D mmio_ro_emulated_write + .write =3D mmio_ro_emulated_write, + .cpuid =3D hvmemul_cpuid, }; struct mmio_ro_emulate_ctxt mmio_ro_ctxt =3D { .cr2 =3D gla }; struct hvm_emulate_ctxt ctxt; --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -5327,6 +5327,7 @@ static const struct x86_emulate_ops ptwr .insn_fetch =3D ptwr_emulated_read, .write =3D ptwr_emulated_write, .cmpxchg =3D ptwr_emulated_cmpxchg, + .cpuid =3D pv_emul_cpuid, }; =20 /* Write page fault handler: check if guest is trying to modify a PTE. */ @@ -5414,6 +5415,7 @@ static const struct x86_emulate_ops mmio .read =3D x86emul_unhandleable_rw, .insn_fetch =3D ptwr_emulated_read, .write =3D mmio_ro_emulated_write, + .cpuid =3D pv_emul_cpuid, }; =20 int mmcfg_intercept_write( @@ -5451,6 +5453,7 @@ static const struct x86_emulate_ops mmcf .read =3D x86emul_unhandleable_rw, .insn_fetch =3D ptwr_emulated_read, .write =3D mmcfg_intercept_write, + .cpuid =3D pv_emul_cpuid, }; =20 /* Check if guest is trying to modify a r/o MMIO page. */ --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -306,6 +306,7 @@ static const struct x86_emulate_ops hvm_ .insn_fetch =3D hvm_emulate_insn_fetch, .write =3D hvm_emulate_write, .cmpxchg =3D hvm_emulate_cmpxchg, + .cpuid =3D hvmemul_cpuid, }; =20 static int @@ -374,6 +375,7 @@ static const struct x86_emulate_ops pv_s .insn_fetch =3D pv_emulate_read, .write =3D pv_emulate_write, .cmpxchg =3D pv_emulate_cmpxchg, + .cpuid =3D pv_emul_cpuid, }; =20 const struct x86_emulate_ops *shadow_init_emulation( --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -2755,6 +2755,24 @@ static int priv_op_write_msr(unsigned in return X86EMUL_UNHANDLEABLE; } =20 +int pv_emul_cpuid(unsigned int *eax, unsigned int *ebx, unsigned int = *ecx, + unsigned int *edx, struct x86_emulate_ctxt *ctxt) +{ + struct cpu_user_regs regs =3D *ctxt->regs; + + regs._eax =3D *eax; + regs._ecx =3D *ecx; + + pv_cpuid(®s); + + *eax =3D regs._eax; + *ebx =3D regs._ebx; + *ecx =3D regs._ecx; + *edx =3D regs._edx; + + return X86EMUL_OKAY; +} + /* Instruction fetch with error handling. */ #define insn_fetch(type, base, eip, limit) = \ ({ unsigned long _rc, _ptr =3D (base) + (eip); = \ --- a/xen/include/asm-x86/hvm/emulate.h +++ b/xen/include/asm-x86/hvm/emulate.h @@ -60,6 +60,12 @@ void hvm_emulate_init( unsigned int insn_bytes); void hvm_emulate_writeback( struct hvm_emulate_ctxt *hvmemul_ctxt); +int hvmemul_cpuid( + unsigned int *eax, + unsigned int *ebx, + unsigned int *ecx, + unsigned int *edx, + struct x86_emulate_ctxt *ctxt); struct segment_register *hvmemul_get_seg_reg( enum x86_segment seg, struct hvm_emulate_ctxt *hvmemul_ctxt); --- a/xen/include/asm-x86/mm.h +++ b/xen/include/asm-x86/mm.h @@ -504,6 +504,8 @@ extern int mmcfg_intercept_write(enum x8 void *p_data, unsigned int bytes, struct x86_emulate_ctxt *ctxt); +int pv_emul_cpuid(unsigned int *eax, unsigned int *ebx, unsigned int = *ecx, + unsigned int *edx, struct x86_emulate_ctxt *ctxt); =20 int ptwr_do_page_fault(struct vcpu *, unsigned long, struct cpu_user_regs *); --=__Part0D347C71.1__= Content-Type: text/plain; name="x86-emul-ops-CPUID.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86-emul-ops-CPUID.patch" x86: always supply .cpuid() handler to x86_emulate()=0A=0AWith us = incremementally adding proper CPUID checks to x86_emulate()=0A(see commit = de05bd965a ["x86emul: correct {,F}CMOV and F{,U}COMI{,P}=0Aemulation"]) it = is no longer appropriate to invoke the function with=0Athat hook being = NULL, as long as respective instructions may get used=0Ain that case.=0A=0A= Signed-off-by: Jan Beulich =0A---=0AI've noticed the = problem first with a not yet submitted patch checking=0Acx8 (PV 32-bit = kernels occasionally use cmpxchg8b for page table=0Aaccesses), and I think = we don't strictly need the change here for 4.8,=0Abut then again it may be = better to handle this properly right away.=0A=0A--- a/xen/arch/x86/hvm/emul= ate.c=0A+++ b/xen/arch/x86/hvm/emulate.c=0A@@ -1542,7 +1542,7 @@ static = int hvmemul_wbinvd(=0A return X86EMUL_OKAY;=0A }=0A =0A-static int = hvmemul_cpuid(=0A+int hvmemul_cpuid(=0A unsigned int *eax,=0A = unsigned int *ebx,=0A unsigned int *ecx,=0A@@ -1892,11 +1892,13 @@ int = hvm_emulate_one_mmio(unsigned long m=0A .read =3D x86emul_unh= andleable_rw,=0A .insn_fetch =3D hvmemul_insn_fetch,=0A = .write =3D mmcfg_intercept_write,=0A+ .cpuid =3D = hvmemul_cpuid,=0A };=0A static const struct x86_emulate_ops = hvm_ro_emulate_ops_mmio =3D {=0A .read =3D x86emul_unhandleab= le_rw,=0A .insn_fetch =3D hvmemul_insn_fetch,=0A- .write = =3D mmio_ro_emulated_write=0A+ .write =3D mmio_ro_emulated_wr= ite,=0A+ .cpuid =3D hvmemul_cpuid,=0A };=0A struct = mmio_ro_emulate_ctxt mmio_ro_ctxt =3D { .cr2 =3D gla };=0A struct = hvm_emulate_ctxt ctxt;=0A--- a/xen/arch/x86/mm.c=0A+++ b/xen/arch/x86/mm.c= =0A@@ -5327,6 +5327,7 @@ static const struct x86_emulate_ops ptwr=0A = .insn_fetch =3D ptwr_emulated_read,=0A .write =3D ptwr_emulated_wr= ite,=0A .cmpxchg =3D ptwr_emulated_cmpxchg,=0A+ .cpuid =3D = pv_emul_cpuid,=0A };=0A =0A /* Write page fault handler: check if guest is = trying to modify a PTE. */=0A@@ -5414,6 +5415,7 @@ static const struct = x86_emulate_ops mmio=0A .read =3D x86emul_unhandleable_rw,=0A = .insn_fetch =3D ptwr_emulated_read,=0A .write =3D mmio_ro_emulate= d_write,=0A+ .cpuid =3D pv_emul_cpuid,=0A };=0A =0A int mmcfg_inter= cept_write(=0A@@ -5451,6 +5453,7 @@ static const struct x86_emulate_ops = mmcf=0A .read =3D x86emul_unhandleable_rw,=0A .insn_fetch = =3D ptwr_emulated_read,=0A .write =3D mmcfg_intercept_write,=0A+ = .cpuid =3D pv_emul_cpuid,=0A };=0A =0A /* Check if guest is trying = to modify a r/o MMIO page. */=0A--- a/xen/arch/x86/mm/shadow/common.c=0A+++= b/xen/arch/x86/mm/shadow/common.c=0A@@ -306,6 +306,7 @@ static const = struct x86_emulate_ops hvm_=0A .insn_fetch =3D hvm_emulate_insn_fetch,= =0A .write =3D hvm_emulate_write,=0A .cmpxchg =3D = hvm_emulate_cmpxchg,=0A+ .cpuid =3D hvmemul_cpuid,=0A };=0A =0A = static int=0A@@ -374,6 +375,7 @@ static const struct x86_emulate_ops = pv_s=0A .insn_fetch =3D pv_emulate_read,=0A .write =3D = pv_emulate_write,=0A .cmpxchg =3D pv_emulate_cmpxchg,=0A+ .cpuid = =3D pv_emul_cpuid,=0A };=0A =0A const struct x86_emulate_ops = *shadow_init_emulation(=0A--- a/xen/arch/x86/traps.c=0A+++ b/xen/arch/x86/t= raps.c=0A@@ -2755,6 +2755,24 @@ static int priv_op_write_msr(unsigned = in=0A return X86EMUL_UNHANDLEABLE;=0A }=0A =0A+int pv_emul_cpuid(unsign= ed int *eax, unsigned int *ebx, unsigned int *ecx,=0A+ = unsigned int *edx, struct x86_emulate_ctxt *ctxt)=0A+{=0A+ struct = cpu_user_regs regs =3D *ctxt->regs;=0A+=0A+ regs._eax =3D *eax;=0A+ = regs._ecx =3D *ecx;=0A+=0A+ pv_cpuid(®s);=0A+=0A+ *eax =3D = regs._eax;=0A+ *ebx =3D regs._ebx;=0A+ *ecx =3D regs._ecx;=0A+ = *edx =3D regs._edx;=0A+=0A+ return X86EMUL_OKAY;=0A+}=0A+=0A /* = Instruction fetch with error handling. */=0A #define insn_fetch(type, = base, eip, limit) \=0A ({ unsigned long = _rc, _ptr =3D (base) + (eip); \=0A--- = a/xen/include/asm-x86/hvm/emulate.h=0A+++ b/xen/include/asm-x86/hvm/emulate= .h=0A@@ -60,6 +60,12 @@ void hvm_emulate_init(=0A unsigned int = insn_bytes);=0A void hvm_emulate_writeback(=0A struct hvm_emulate_ctxt = *hvmemul_ctxt);=0A+int hvmemul_cpuid(=0A+ unsigned int *eax,=0A+ = unsigned int *ebx,=0A+ unsigned int *ecx,=0A+ unsigned int *edx,=0A+ = struct x86_emulate_ctxt *ctxt);=0A struct segment_register *hvmemul_get_= seg_reg(=0A enum x86_segment seg,=0A struct hvm_emulate_ctxt = *hvmemul_ctxt);=0A--- a/xen/include/asm-x86/mm.h=0A+++ b/xen/include/asm-x8= 6/mm.h=0A@@ -504,6 +504,8 @@ extern int mmcfg_intercept_write(enum x8=0A = void *p_data,=0A = unsigned int bytes,=0A struct = x86_emulate_ctxt *ctxt);=0A+int pv_emul_cpuid(unsigned int *eax, unsigned = int *ebx, unsigned int *ecx,=0A+ unsigned int *edx, = struct x86_emulate_ctxt *ctxt);=0A =0A int ptwr_do_page_fault(struct vcpu = *, unsigned long,=0A struct cpu_user_regs *);=0A --=__Part0D347C71.1__= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --=__Part0D347C71.1__=--