From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 4/4] Userspace changes for KVM HPET (v3)
Date: Wed, 13 May 2009 10:48:40 +0300
Message-ID: <4A0A7B58.9080906@redhat.com>
References: <1242062986-29383-1-git-send-email-eak@us.ibm.com>   <1242062986-29383-4-git-send-email-eak@us.ibm.com>  <4A093B49.7010609@redhat.com> <4A0986CD.20601@us.ibm.com> <4A09A35B.5030908@us.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org
To: Beth Kon <eak@us.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:59401 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1756942AbZEMHtN (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 13 May 2009 03:49:13 -0400
In-Reply-To: <4A09A35B.5030908@us.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Beth Kon wrote:
> Beth Kon wrote:
>> Avi Kivity wrote:
>>> Beth Kon wrote:
>>>> Signed-off-by: Beth Kon <eak@us.ibm.com>
>>>>
>>>>
>>>> diff --git a/hw/hpet.c b/hw/hpet.c
>>>> index c7945ec..100abf5 100644
>>>> --- a/hw/hpet.c
>>>> +++ b/hw/hpet.c
>>>> @@ -30,6 +30,7 @@
>>>>  #include "console.h"
>>>>  #include "qemu-timer.h"
>>>>  #include "hpet_emul.h"
>>>> +#include "qemu-kvm.h"
>>>>  
>>>>  //#define HPET_DEBUG
>>>>  #ifdef HPET_DEBUG
>>>> @@ -48,6 +49,28 @@ uint32_t hpet_in_legacy_mode(void)
>>>>          return 0;
>>>>  }
>>>>  
>>>> +static void hpet_legacy_enable(void)
>>>> +{
>>>> +    if (qemu_kvm_pit_in_kernel()) {
>>>> +        kvm_kpit_disable();
>>>> +        dprintf("qemu: hpet disabled kernel pit\n");
>>>> +    } else {
>>>> +        hpet_pit_disable();
>>>> +        dprintf("qemu: hpet disabled userspace pit\n");
>>>> +    }
>>>> +}
>>>> +
>>>> +static void hpet_legacy_disable(void)
>>>> +{
>>>> +    if (qemu_kvm_pit_in_kernel()) {
>>>> +        kvm_kpit_enable();
>>>> +        dprintf("qemu: hpet enabled kernel pit\n");
>>>> +    } else {
>>>> +        hpet_pit_enable();
>>>> +        dprintf("qemu: hpet enabled userspace pit\n");
>>>> +    }
>>>> +}
>>>>   
>>> I think it's better to move these into hpet_pit_enable() and 
>>> hpet_pit_enable().  This avoids changing the calls below, and puts 
>>> pit stuff in i8254.c instead of hpet.c.
>>>
>>> Might also need to be called from hpet_load(); probably a problem in 
>>> upstream as well.
>>>
>> My assumption about hpet_load was that the correct pit state would be 
>> established via pit_load (since all saves/loads are done together).  
>> But when I wrote this, I was thinking only about the userspace pit 
>> (for qemu). I'm not sure how the "load" concept applies to kernel 
>> state.  Do I need to explicitly re-enable or disable the kernel pit 
>> during load?
> Looking further at the code, it looks like kvm_pit_load should take 
> care of this. Agree?
>

I doesn't save/load the "enabled" bit, does it?

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.