From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60948) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VQZle-0004bb-AG for qemu-devel@nongnu.org; Mon, 30 Sep 2013 05:27:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VQZlY-0001W7-42 for qemu-devel@nongnu.org; Mon, 30 Sep 2013 05:27:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31073) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VQZlX-0001W0-Rf for qemu-devel@nongnu.org; Mon, 30 Sep 2013 05:27:44 -0400 Date: Mon, 30 Sep 2013 12:30:00 +0300 From: "Michael S. Tsirkin" Message-ID: <20130930093000.GE20291@redhat.com> References: <1378956318-23395-1-git-send-email-pingfank@linux.vnet.ibm.com> <1378956318-23395-3-git-send-email-pingfank@linux.vnet.ibm.com> <20130928195635.GC1051@redhat.com> <20130929041547.GA28242@redhat.com> <20130930090631.GC20291@redhat.com> <52493F28.5090303@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52493F28.5090303@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 2/5] hpet: entitle more irq pins for hpet List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Peter Maydell , Anthony Liguori , Jan Kiszka , liu ping fan , qemu-devel@nongnu.org, Stefan Hajnoczi , Andreas =?iso-8859-1?Q?F=E4rber?= On Mon, Sep 30, 2013 at 11:06:48AM +0200, Paolo Bonzini wrote: > Il 30/09/2013 11:06, Michael S. Tsirkin ha scritto: > > On Mon, Sep 30, 2013 at 04:02:29PM +0800, liu ping fan wrote: > >> On Sun, Sep 29, 2013 at 12:15 PM, Michael S. Tsirkin wrote: > >>> On Sun, Sep 29, 2013 at 11:49:41AM +0800, liu ping fan wrote: > >>>> On Sun, Sep 29, 2013 at 3:56 AM, Michael S. Tsirkin wrote: > >>>>> On Thu, Sep 12, 2013 at 11:25:15AM +0800, Liu Ping Fan wrote: > >>>>>> On PC, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23 > >>>>>> of ioapic can be dynamically assigned to hpet as guest chooses. > >>>>>> (Will enable them after introducing pc 1.6 compat) > >>>>>> > >>>>>> Signed-off-by: Liu Ping Fan > >>>>>> --- > >>>>>> hw/timer/hpet.c | 13 +++++++++++-- > >>>>>> 1 file changed, 11 insertions(+), 2 deletions(-) > >>>>>> > >>>>>> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c > >>>>>> index 8429eb3..46903b9 100644 > >>>>>> --- a/hw/timer/hpet.c > >>>>>> +++ b/hw/timer/hpet.c > >>>>>> @@ -25,6 +25,7 @@ > >>>>>> */ > >>>>>> > >>>>>> #include "hw/hw.h" > >>>>>> +#include "hw/boards.h" > >>>>>> #include "hw/i386/pc.h" > >>>>>> #include "ui/console.h" > >>>>>> #include "qemu/timer.h" > >>>>>> @@ -42,6 +43,12 @@ > >>>>>> > >>>>>> #define HPET_MSI_SUPPORT 0 > >>>>>> > >>>>>> +/* For bug compat, using only IRQ2. Soon it will be fixed as > >>>>>> + * 0xff0104ULL, i.e using IRQ16~23, IRQ8 and IRQ2 > >>>>> > >>>>> So users are expected to stick a bitmask of legal > >>>>> pins here? > >>>>> I think that's a bit too much rope to give to users. > >>>>> Don't you think? > >>>>> > >>>> Sorry, not understand your meaning exactly. But the scene will be: > >>>> guest kernel polls the ability bitmask, and pick up one pin which is > >>>> not occupied or can be shared with the level-trigger and low-active. > >>>> So is it rope? > >>> > >>> I merely say that it's better to make this a bool or bit property. > >>> UINT32 is too much flexibility imho. > >>> > >> The interrupt capability is export to guest by register > >> Tn_INT_ROUTE_CAP[63:32]. So it is useless to make them as a bit > >> property. Do you think so? > >> > >> Regards > >> Pingfan > > > > I think we merely need to support two modes: > > - qemu 1.6 and older compatible > > - compatible to actual hardware > > > > Why would we let users configure an arbitrary > > configuration which isn't compatible to either > > old qemu or real hardware? > > The actual setting depends on the chipset. For example, the "real" > PIIX4 is the same as the QEMU PIIX4, only Q35 uses the new value. > > If in the future we had a chipset with more than 24 GSIs, you would have > a third possibility. > > Paolo I was really only talking about q35 here. I thought it's ugly that users can control intcap directly. Can object_set_property be used after qdev_try_create? PIIX has another issue: the default value in hpet is really Q35 specific, that's also kind of ugly, isn't it? -- MST