| Message ID | 1242062986-29383-4-git-send-email-eak@us.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
Avi Kivity wrote: >>> 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? > Also, we might migrate between a host with pit-in-kernel and a host with pit-in-userspace, so this is should be handled at the pit level, not kvm.
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"); + } +} + static uint32_t timer_int_route(struct HPETTimer *timer) { uint32_t route; @@ -475,9 +498,9 @@ static void hpet_ram_writel(void *opaque, target_phys_addr_t addr, } /* i8254 and RTC are disabled when HPET is in legacy mode */ if (activating_bit(old_val, new_val, HPET_CFG_LEGACY)) { - hpet_pit_disable(); + hpet_legacy_enable(); } else if (deactivating_bit(old_val, new_val, HPET_CFG_LEGACY)) { - hpet_pit_enable(); + hpet_legacy_disable(); } break; case HPET_CFG + 4: @@ -560,7 +583,7 @@ static void hpet_reset(void *opaque) { * hpet_reset is called due to system reset. At this point control must * be returned to pit until SW reenables hpet. */ - hpet_pit_enable(); + hpet_legacy_disable(); count = 1; } diff --git a/qemu-kvm.c b/qemu-kvm.c index f55cee8..1bb853b 100644 --- a/qemu-kvm.c +++ b/qemu-kvm.c @@ -463,6 +463,25 @@ void kvm_init_vcpu(CPUState *env) qemu_cond_wait(&qemu_vcpu_cond); } +void kvm_kpit_enable(void) +{ + struct kvm_pit_state ps; + if (qemu_kvm_pit_in_kernel()) { + kvm_get_pit(kvm_context, &ps); + kvm_set_pit(kvm_context, &ps); + } +} + +void kvm_kpit_disable(void) +{ + struct kvm_pit_state ps; + if (qemu_kvm_pit_in_kernel()) { + kvm_get_pit(kvm_context, &ps); + ps.channels[0].mode = 0xff; + kvm_set_pit(kvm_context, &ps); + } +} + int kvm_init_ap(void) { #ifdef TARGET_I386 diff --git a/qemu-kvm.h b/qemu-kvm.h index 6a1968a..13353ec 100644 --- a/qemu-kvm.h +++ b/qemu-kvm.h @@ -31,6 +31,8 @@ int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap); int kvm_qemu_init_env(CPUState *env); int kvm_qemu_check_extension(int ext); void kvm_apic_init(CPUState *env); +void kvm_kpit_enable(void); +void kvm_kpit_disable(void); int kvm_set_irq(int irq, int level, int *status); int kvm_physical_memory_set_dirty_tracking(int enable); diff --git a/vl.c b/vl.c index 0bffc82..8f120c5 100644 --- a/vl.c +++ b/vl.c @@ -6132,10 +6132,15 @@ int main(int argc, char **argv, char **envp) } if (kvm_enabled()) { - kvm_init_ap(); + kvm_init_ap(); #ifdef USE_KVM if (kvm_irqchip && !qemu_kvm_has_gsi_routing()) { irq0override = 0; + /* if kernel can't do irq routing, interrupt source + * override 0->2 can not be set up as required by hpet, + * so disable hpet. + */ + no_hpet=1; } #endif }
Signed-off-by: Beth Kon <eak@us.ibm.com> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html