From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v3 1/3] KVM: X86: Fix operand size during instruction decoding Date: Thu, 2 Nov 2017 18:45:42 +0100 Message-ID: References: <1509611499-9401-1-git-send-email-wanpeng.li@hotmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li , Nadav Amit , Pedro Fonseca To: Wanpeng Li , linux-kernel@vger.kernel.org, kvm@vger.kernel.org Return-path: In-Reply-To: <1509611499-9401-1-git-send-email-wanpeng.li@hotmail.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 02/11/2017 09:31, Wanpeng Li wrote: > From: Wanpeng Li > > Pedro reported: > During tests that we conducted on KVM, we noticed that executing a "PUSH %ES" > instruction under KVM produces different results on both memory and the SP > register depending on whether EPT support is enabled. With EPT the SP is > reduced by 4 bytes (and the written value is 0-padded) but without EPT support > it is only reduced by 2 bytes. The difference can be observed when the CS.DB > field is 1 (32-bit) but not when it's 0 (16-bit). > > The internal segment descriptor cache exist even in real/vm8096 mode. The CS.D > also should be respected instead of just default operand-size/66H prefix during > instruction decoding. This patch fixes it by also adjusting operand-size according > to CS.D. > > Reported-by: Pedro Fonseca > Tested-by: Pedro Fonseca > Cc: Paolo Bonzini > Cc: Radim Krčmář > Cc: Nadav Amit > Cc: Pedro Fonseca > Signed-off-by: Wanpeng Li > --- > v2 -> v3: > * cleanup the codes > v1 -> v2: > * respect cs.d for real/vm8096, other modes have already > been considered in init_emulate_ctxt(). > > arch/x86/kvm/emulate.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 8079d14..6ebc4cb 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -5000,6 +5000,8 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len) > bool op_prefix = false; > bool has_seg_override = false; > struct opcode opcode; > + u16 dummy; > + struct desc_struct desc; > > ctxt->memop.type = OP_NONE; > ctxt->memopp = NULL; > @@ -5020,6 +5022,11 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len) > case X86EMUL_MODE_VM86: > case X86EMUL_MODE_PROT16: > def_op_bytes = def_ad_bytes = 2; > + if (mode < X86EMUL_MODE_PROT16) { > + ctxt->ops->get_segment(ctxt, &dummy, &desc, NULL, VCPU_SREG_CS); > + if (desc.d) > + def_op_bytes = 4; def_ad_bytes must be changed to 4 as well. With that change, you should probably separate X86EMUL_MODE_PROT16 altogether from the others. Paolo > + } > break; > case X86EMUL_MODE_PROT32: > def_op_bytes = def_ad_bytes = 4; >