Chris Wright wrote:
Thanks for the review! Comments inline.
+/* paravirt_ops.get_wallclock = vmi_get_wallclock */
Style nit, these pv_ops.foo = vmi_foo style comments aren't really useful.
Yeah, and easy to get out of sync. I'll drop them.
+ .rating = 1000,
Heh, no messing around ;-)
Yes, VMI has 1000 hps.
+ printk(KERN_WARNING "vmi: registering clock event %s. mult=%lu shift=%u\n",
+ evt->name, evt->mult, evt->shift);
Why is this a warning? ;-)
Debug info, I can remove it.
+void __init vmi_time_init(void)
+{
+ /* Disable PIT: BIOSes start PIT CH0 with 18.2hz peridic. */
+ outb_p(0x3a, PIT_MODE); /* binary, mode 5, LSB/MSB, ch 0 */
That shouldn't be necessary using clockevents.
Actually, I'm not so sure. If clockevents simply masks the PIT when
disabling it, we still have overhead of keeping the latch in sync, which
requires a timer at the PIT frequency. I can instrument to see how
exactly the PIT gets disabled.
+ vmi_time_init_clockevent();
+ setup_irq(0, &vmi_clock_action);
+}
+
+#ifdef CONFIG_X86_LOCAL_APIC
+void __devinit vmi_time_bsp_init(void)
+{
+ /*
+ * On APIC systems, we want local timers to fire on each cpu. We do
+ * this by programming LVTT to deliver timer events to the IRQ handler
+ * for IRQ-0, since we can't re-use the APIC local timer handler
+ * without interfering with that code.
+ */
+ clockevents_notify(CLOCK_EVT_NOTIFY_SUSPEND, NULL);
Why do you do this suspend...
We need to cancel all pending PIT timer events and restart then local
timer, which requires atomically taking over IRQ-0. We use the IDT gate
for IRQ-0 because it is already an exclusive interrupt, but we can't
re-use the LVTT IDT gate for local timer since that requires a custom
custom SMP interrupt in entry.S. So we must be absolutely sure when we
get an interrupt on IRQ-0 that it came from the VMI local (rather than
PIT) delivery path.
+ local_irq_disable();
+#ifdef CONFIG_X86_SMP
+ /*
+ * XXX handle_percpu_irq only defined for SMP; we need to switch over
+ * to using it, since this is a local interrupt, which each CPU must
+ * handle individually without locking out or dropping simultaneous
+ * local timers on other CPUs. We also don't want to trigger the
+ * quirk workaround code for interrupts which gets invoked from
+ * handle_percpu_irq via eoi, so we use our own IRQ chip.
+ */
+ set_irq_chip_and_handler_name(0, &vmi_chip, handle_percpu_irq, "lvtt");
+#else
+ set_irq_chip_and_handler_name(0, &vmi_chip, handle_edge_irq, "lvtt");
+#endif
+ vmi_wiring = VMI_ALARM_WIRED_LVTT;
+ apic_write(APIC_LVTT, vmi_get_timer_vector());
isn't this just your ->startup?
Which structure has a ->startup function we can use? Sorry if this
seems ignorant, I'm not quite sure what you mean.
+ local_irq_enable();
+ clockevents_notify(CLOCK_EVT_NOTIFY_RESUME, NULL);
...and resume? Instead of letting clockevents core handle all of that,
and just registering right here?
It wasn't clear that clockevents would issue a resume notify for us; if
so we could handle this setup in the callback, but it has to be done on
the correct CPU. I can try it and see if that works.
Thanks,
Zach
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
