This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH] x86: fix NULL function call in timer_softirq_act

To: NISHIGUCHI Naoki <nisiguti@xxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86: fix NULL function call in timer_softirq_action()
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Tue, 22 Apr 2008 11:29:06 +0100
Delivery-date: Tue, 22 Apr 2008 03:29:52 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <480D508A.6000403@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcikY7EZ718thhBWEd2NMQAX8io7RQ==
Thread-topic: [Xen-devel] [PATCH] x86: fix NULL function call in timer_softirq_action()
User-agent: Microsoft-Entourage/
On 22/4/08 03:42, "NISHIGUCHI Naoki" <nisiguti@xxxxxxxxxxxxxx> wrote:

> This panic's cause was find_first_bit() in vmx_dirq_assist().
> In find_first_bit(__find_first_bit) function, "repe; scas" instruction
> and "bsf" instruction refer addresses of a bitmap. If clear_bit() is
> called to clear a bit of the bitmap between above instructions, eax
> register's value is zero after execution of "bsf" instruction. As a
> result, the return value of find_first_bit() will be 0, 64, 128 or
> 192(on x86_64 arch).
> In this case, vmx_dirq_assist() calls set_timer() about the bit not to
> be set. If hvm_timer(timer structure) about the bit is not initialized,
> timer_softirq_action() will call zero address.

Good catch. Actually our usage of BSF is generally bad because Intel does
not guarantee the contents of the destination register when the source is
zero (we are currently assuming the destination is left intact in that
case). I will fix that up too.

Also I think vmx_dirq_assist() is broken because it assumes that it 'owns'
the bit it finds. But actually two VCPUs can race to clear_bit() and both
increment, for example, the mirq[irq].pending field. I think the loop body
should start with if ( !test_and_clear_bit(...) ) continue. I will make that
change also.

 -- Keir

Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>