From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] KVM: x86: Revert "check CS.DPL against RPL during task switch" Date: Sun, 17 Aug 2014 23:09:53 +0200 Message-ID: <53F11A21.2040406@redhat.com> References: <53F07745.9080200@redhat.com> <1408303946-2796-1-git-send-email-namit@cs.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Nadav Amit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:53140 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751404AbaHQVKB (ORCPT ); Sun, 17 Aug 2014 17:10:01 -0400 In-Reply-To: <1408303946-2796-1-git-send-email-namit@cs.technion.ac.il> Sender: kvm-owner@vger.kernel.org List-ID: Il 17/08/2014 21:32, Nadav Amit ha scritto: > This reverts commit 5045b468037dfe1c848827ce10e99d87f5669160. Although the > cs.dpl=cs.rpl check is mentioned in table 7-1 of the SDM as causing a #TSS > exception, it is not mentioned in table 6-6 that lists "invalid TSS conditions" > which cause #TSS exceptions. As it causes some tests, which pass on bare-metal, > to fail - it should be reverted. Right. However, I think reverting the patch is too big a hammer. We still need in_task_switch to raise TS_VECTOR instead of GP_VECTOR, so I propose instead something like: diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 56657b0bb3bb..cd230b035514 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1468,7 +1468,7 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt, return ret; err_code = selector & 0xfffc; - err_vec = GP_VECTOR; + err_vec = in_task_switch ? TS_VECTOR : GP_VECTOR; /* can't load system descriptor into segment selector */ if (seg <= VCPU_SREG_GS && !seg_desc.s) @@ -1491,9 +1491,6 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt, goto exception; break; case VCPU_SREG_CS: - if (in_task_switch && rpl != dpl) - goto exception; - if (!(seg_desc.type & 8)) goto exception; either in a single patch or as two separate patch. I'll test this against your test case and repost (not before Tuesday). Paolo