tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: pvclock (kvm_clock) support: where to attach



> Date: Sun, 31 Dec 2023 11:36:41 +0100
> From: Emile 'iMil' Heitor <imil%home.imil.net@localhost>
> 
> I ported pvclock / kvm_clock from OpenBSD in order for Firecracker to
> have an RTC. It's working but I'm not entirely sure where to attach it.
> 
> The device is x86 only so I added the source code in arch/x86/x86, and
> for now I have it attached at cpufeaturebus which felt the more natural.
> 
> Thoughts?
> 
> Here's the code: 
> https://github.com/NetBSD/src/commit/c09440be5aca7e16ce845c3ccbdfb47bac03fb63

Oops...  We already have a driver for what I think is essentially the
same thing in Xen:

https://nxr.netbsd.org/xref/src/sys/arch/xen/xen/xen_clock.c

Unless there's a compelling reason that the pvclock and xenclock
interfaces are different enough to warrant having multiple copies of
the logic in src, I think we should adapt the existing code to work in
both settings.  I put a lot of work into the xen_clock.c driver to
record useful diagnostics about when the host's time is not behaving
right (vs when NetBSD itself has done something wrong), which we've
seen in practice on various hosts, and it would be a shame to lose
that.

Do you know if there are substantive differences in the interface?

If not, I think long-term we should introduce a new sys/dev/pv or
something, move the bulk of xen_clock.c to that (other than the
Xen-specific parts), and have both the Firecracker code and the Xen
code use it.


Couple notes having only glanced at this code, if we do want to keep
it as is:
- *_activate is obsolete; nothing but cardbus calls it and new drivers
  should never provide it. 
- virtio_membar_sync seems wrong -- this should read-before-read,
  which is membar_consumer, not membar_sync (but I don't know why
  there's a separate virtio_membar_*).


Home | Main Index | Thread Index | Old Index