From owner-freebsd-virtualization@FreeBSD.ORG Wed Dec 11 07:19:15 2013 Return-Path: Delivered-To: virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C8E8C813; Wed, 11 Dec 2013 07:19:15 +0000 (UTC) Received: from mail-qe0-x230.google.com (mail-qe0-x230.google.com [IPv6:2607:f8b0:400d:c02::230]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 62FD911BD; Wed, 11 Dec 2013 07:19:15 +0000 (UTC) Received: by mail-qe0-f48.google.com with SMTP id gc15so5035782qeb.7 for ; Tue, 10 Dec 2013 23:19:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=qFJv3rFAZWCqpSVWKmwLIwL4QoJbznH+WluVS+3QIMw=; b=PrJEYH6AV4nnq2qCCO9sdcUtQDTvac9YcSA+QeiGO4FFh8MgRjUou3Ew/n7bx2zAcR 9s0jTtK5urPuwnmdlLcdO1izDovkw0V8s/H+tDAWVM+IeJvSD/hSsaQzWdD+/sTjDEen WikUt4YYTGgm5qyhZXkHn2ox9zoBnm2SS0y4b1xzGWkCOhXIu/q1M+jDhBlX5nxL24pm Re+asVK0SHwH9hUSonASgVtHn3ajBDJ329GKDJD0WykzbRMoqMTQQPNONEb/1rEN2SKe GAwTW3c7M9E8yP2x5IKxajxAz1qDTkc4zg3enyGC2J/eJjYswR5L+vvKbeiK2hgDP25T 8f4w== MIME-Version: 1.0 X-Received: by 10.224.112.68 with SMTP id v4mr51533538qap.90.1386746354584; Tue, 10 Dec 2013 23:19:14 -0800 (PST) Received: by 10.140.34.17 with HTTP; Tue, 10 Dec 2013 23:19:14 -0800 (PST) In-Reply-To: <201312101527.47234.jhb@freebsd.org> References: <201312101527.47234.jhb@freebsd.org> Date: Tue, 10 Dec 2013 23:19:14 -0800 Message-ID: Subject: Re: [PATCH] Additional lapic support for bhyve From: Neel Natu To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Cc: "freebsd-virtualization@freebsd.org" X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Dec 2013 07:19:16 -0000 Hi John, On Tue, Dec 10, 2013 at 12:27 PM, John Baldwin wrote: > This patch extends the local APIC emulation a bit by adding the following > things: > - Add an ioctl and a bhyvectl command to trigger local interrupts on a > local APIC. The "fixed" and "NMI" delivery modes are enabled. > - Add support for the CMCI LVT entry. > - Add the ability to report local APIC errors and trigger errors for > invalid vectors when sending IPIs or firing an external interrupt that > references an invalid IDT vector. > - Silently force all reserved fields in LVT entries to zero when they > are written (removes the need for clearing those bits when triggering > an LVT interrupt). > - Add entries to the MP Table and MADT to advertise the typical x86 LINT > configuration (ExtINT on LINT0 and NMI on LINT1). > - Add a bhyvectl command to inject an NMI on an arbitrary CPU (this latter > should probably be a separate patch) > > In particular, while bhyectl --inject-nmi can inject an NMI on a single vcpu, > the more traditional way of signalling a system-wide error (such as SERR# > or PERR#) is to assert the LINT1 pins on all CPUs. This can now be done via > 'bhyectl --vm=foo --cpu=-1 --assert-lapic-lvt=1' (cpu of -1 is a broadcast > to all CPUs for the LVT ioctl). The patch is at > people.freebsd.org/~jhb/patches/bhyve_lapic.patch I'm certainly open to > suggestions on ways to make this be more consistent with the style/design/flow > of the existing code. > This looks good. vlapic_fire_lvt() and vlapic_set_error() can call each other recursively if guest has programmed the ERROR_LVT vector incorrectly. This can probably be fixed by passing the LVT index to vlapic_fire_lvt() and treating LVT_ERROR specially. A couple of nits: - Could you add a /* fallthrough */ comment after setting the 'mask' for the LINT_LVT entries in vlapic_set_lvt()? - Any reason that vlapic_set_error() and vlapic_fire_cmci() are not static to vlapic.c? Do you anticipate calling them from outside this file in the future? best Neel > (I started on this because I want to add support for machine check injection > so I can use bhyve to test the machine check code, but for that I wanted CMCI > support and it kind of snowballed from there) > > -- > John Baldwin