From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MaW7P-00089Y-4I for qemu-devel@nongnu.org; Mon, 10 Aug 2009 10:44:59 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MaW7K-00085q-I9 for qemu-devel@nongnu.org; Mon, 10 Aug 2009 10:44:58 -0400 Received: from [199.232.76.173] (port=50109 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MaW7K-000850-8U for qemu-devel@nongnu.org; Mon, 10 Aug 2009 10:44:54 -0400 Received: from qw-out-1920.google.com ([74.125.92.149]:46263) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MaW7J-0006v9-MA for qemu-devel@nongnu.org; Mon, 10 Aug 2009 10:44:53 -0400 Received: by qw-out-1920.google.com with SMTP id 5so1053681qwc.4 for ; Mon, 10 Aug 2009 07:44:49 -0700 (PDT) Message-ID: <4A80325E.9020505@codemonkey.ws> Date: Mon, 10 Aug 2009 09:44:46 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/2] Make HPET Legacy Only References: <1249049162-685-1-git-send-email-eak@us.ibm.com> In-Reply-To: <1249049162-685-1-git-send-email-eak@us.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Beth Kon Cc: qemu-devel@nongnu.org Beth Kon wrote: > Because of unavailability of IOAPIC interrupt lines, HPET currently > supports only legacy mode, where the legacy timer interrupt (inti2) is > used. This patch makes this explicit by reducing the number of timers > supported by HPET to 2 (the 2 legacy timers) and advertising no > available IOAPIC interrupt lines. > > This patch also adds a check, suggested by Andriy Gapon, to ensure that > the interrupt chosen by the guest is one of those advertised by the HPET > as available. This is currently a noop since HPET advertises none as > available, but in the event of future expansion and availability of > IOAPIC interrupt lines, or implementation of level-triggered interrupts > that can share interrupt lines, this code will be in place. > > This subset of HPET capability (legacy-only) is reasonable, since the > basic operation of linux and windows only uses the HPET in legacy mode, > at least that's what all my testing so far has shown. Only userspace > access to the hpet would use non-legacy timers. > > > > > diff --git a/hw/hpet.c b/hw/hpet.c > index 01b10aa..56fa27c 100644 > --- a/hw/hpet.c > +++ b/hw/hpet.c > @@ -47,12 +47,16 @@ uint32_t hpet_in_legacy_mode(void) > return 0; > } > > -static uint32_t timer_int_route(struct HPETTimer *timer) > -{ > - uint32_t route; > - route = (timer->config & HPET_TN_INT_ROUTE_MASK) >> HPET_TN_INT_ROUTE_SHIFT; > - return route; > -} > +/* future code for when non-legacy interrupts are supported > + * (when spare ioapic interrupts are available) > + * static uint32_t timer_int_route(struct HPETTimer *timer) > + * { > + * uint32_t route; > + * route = (timer->config & HPET_TN_INT_ROUTE_CNF_MASK) > + * >> HPET_TN_INT_ROUTE_CNF_SHIFT; > + * return route; > + *} > + */ > Please don't introduce dead code. It will just rot. A comment would suffice. Regards, Anthony Liguori