From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH 1/6] KVM: x86: DR7.GD should be cleared upon any #DB exception Date: Wed, 1 Oct 2014 21:22:31 +0200 Message-ID: <20141001192230.GE12083@potion.brq.redhat.com> References: <1412099359-5316-1-git-send-email-namit@cs.technion.ac.il> <1412099359-5316-2-git-send-email-namit@cs.technion.ac.il> <20141001152446.GA12085@potion.brq.redhat.com> <05CDC095-E68B-4BB0-94DE-1BC3202C4217@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Nadav Amit , pbonzini@redhat.com, kvm@vger.kernel.org To: Nadav Amit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:15756 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751142AbaJATWj (ORCPT ); Wed, 1 Oct 2014 15:22:39 -0400 Content-Disposition: inline In-Reply-To: <05CDC095-E68B-4BB0-94DE-1BC3202C4217@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: 2014-10-01 21:22+0300, Nadav Amit: >=20 > On Oct 1, 2014, at 6:24 PM, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: >=20 > > 2014-09-30 20:49+0300, Nadav Amit: > >> Intel SDM 17.2.4 (Debug Control Register (DR7)) says: "The process= or clears the > >> GD flag upon entering to the debug exception handler." This senten= ce may be > >> misunderstood as if it happens only on #DB due to debug-register p= rotection, > >> but it happens regardless to the cause of the #DB. > >=20 > > All real hardware behaves that way? > I have no way of knowing. :) > I know Intel=E2=80=99s phrasing is misleading, so I verified this beh= aviour in two ways: > 1. I changed KVM not to trap #DB. I then changed kvm-unit-tests/debug= =2Ec to set DR7.GD prior to the watchpoint test, and printed once I ent= ered the handler, before any DR was accessed by the handler. > The result: we entered the handler once (afterwards I printed DR7 and= saw GD is indeed clear). If #DB due to watchpoint did not clear GD, we= would enter the handler twice. Thanks. > 2. I looked at bochs: https://github.com/larsr/bochs-svn/blob/master/= cpu/exception.cc : >=20 > if (vector =3D=3D BX_DB_EXCEPTION) { > // Commit debug events to DR6: preserve DR5.BS and DR6.BD values, > // only software can clear them > BX_CPU_THIS_PTR dr6.val32 =3D (BX_CPU_THIS_PTR dr6.val32 & 0xffff= 6ff0) | > (BX_CPU_THIS_PTR debug_trap & 0x0000e00f); >=20 > // clear GD flag in the DR7 prior entering debug exception handle= r > BX_CPU_THIS_PTR dr7.set_GD(0); > } (Ok.) > The behaviour seems reasonable to me. Otherwise the CPU would re-ente= r the handler when the handler inspects DR6. It usually is sufficient just to read it, but yeah, re-entry for the general usage isn't nice either. To the patch itself: We could use kvm_x86_ops->set_dr7() directly, but maybe we are going to do something with on GD bit, so Reviewed-by: Radim Kr=C4=8Dm=C3=A1=C5=99