From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Liguori <aliguori@us.ibm.com>
Subject: Re: [PATCH][QEMU] Use a separate device for in-kernel
 PIT
Date: Wed, 12 Mar 2008 17:13:48 -0500
Message-ID: <47D8559C.6060203@us.ibm.com>
References: <1205343520-11789-1-git-send-email-aliguori@us.ibm.com>
	<1205359450.3163.21.camel@localhost.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Cc: kvm-devel@lists.sourceforge.net, Avi Kivity <avi@qumranet.com>
To: dor.laor@qumranet.com
Return-path: <kvm-devel-bounces@lists.sourceforge.net>
In-Reply-To: <1205359450.3163.21.camel@localhost.localdomain>
List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/kvm-devel>,
	<mailto:kvm-devel-request@lists.sourceforge.net?subject=unsubscribe>
List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=kvm-devel>
List-Post: <mailto:kvm-devel@lists.sourceforge.net>
List-Help: <mailto:kvm-devel-request@lists.sourceforge.net?subject=help>
List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/kvm-devel>,
	<mailto:kvm-devel-request@lists.sourceforge.net?subject=subscribe>
Sender: kvm-devel-bounces@lists.sourceforge.net
Errors-To: kvm-devel-bounces@lists.sourceforge.net
List-Id: kvm.vger.kernel.org

Dor Laor wrote:
> On Wed, 2008-03-12 at 12:38 -0500, Anthony Liguori wrote:
>   
>> Part of the feedback we received from Fabrice about the KVM patches
>> for QEMU
>> is that we should create a separate device for the in-kernel APIC to
>> avoid
>> having lots of if (kvm_enabled()) within the APIC code that were
>> difficult to
>> understand why there were needed.
>>
>> This patch separates the in-kernel PIT into a separate device.  It
>> also
>> introduces some configure logic to only compile in support for the
>> in-kernel
>> PIT if it's available.
>>
>> The result of this is that we now only need a single if
>> (kvm_enabled()) to
>> determine which device to use.  Besides making it more upstream
>> friendly, I
>> think this makes the code much easier to understand.
>>
>>     
>
> Seems like a good idea.
>
> [snip]
> ..
>
>
>
>   
>> +static void pit_reset(void *opaque)
>> +{
>> +    PITState *pit = opaque;
>> +    PITChannelState *s;
>> +    int i;
>> +
>> +    for(i = 0;i < 3; i++) {
>> +        s = &pit->channels[i];
>> +        s->mode = 3;
>> +        s->gate = (i != 2);
>> +        pit_load_count(s, 0);
>> +    }
>> +}
>> +
>>     
>
> Seems like pit_reset won't change the in-kernel pit state.
>   

Yeah, that seemed suspicious to me too.  I didn't want to change the 
existing behavior though for fear of introducing regressions.  Perhaps 
Sheng can comment on whether it's necessary to even have a reset handler 
in userspace?

Regards,

Anthony Liguori

> Actually this should handled as a part of more general reset ioctl to
> all of kvm's in-kernel devices.
>
> Cheers,
> Dor
>
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/