On Oct 24, 2014, at 12:20 PM, Konstantin Belousov <kostik...@gmail.com> wrote:

On Fri, Oct 24, 2014 at 06:39:16PM +0000, Rui Paulo wrote:
Author: rpaulo
Date: Fri Oct 24 18:39:15 2014
New Revision: 273598
URL: https://svnweb.freebsd.org/changeset/base/273598

Log:
HPET: create /dev/hpetN as a way to access HPET from userland.

In some cases, TSC is broken and special applications might benefit
from memory mapping HPET and reading the registers to count time.
Most often the main HPET counter is 32-bit only[1], so this only gives
the application a 300 second window based on the default HPET
interval.
Other applications, such as Intel's DPDK, expect /dev/hpet to be
present and use it to count time as well.

Although we have an almost userland version of gettimeofday() which
uses rdtsc in userland, it's not always possible to use it, depending
on how broken the multi-socket hardware is.
Yes, and hpet userland mapping would be better handled through the same
fake-vdso framework. As designed, it has discriminator to inform
userspace about algorithm, and can happilly utilize HPET timecounter
automatically mapped by kernel into the process address space.
 
I'm aware of that, but I found the vdso a bit confusing and decided to work on 
that later.

+static int
+hpet_open(struct cdev *cdev, int oflags, int devtype, struct thread *td)
+{
+      struct hpet_softc *sc;
+
+        sc = cdev->si_drv1;
+        if (!sc->mmap_allow)
+          return (EPERM);
+      if (atomic_cmpset_32(&sc->devinuse, 0, 1) == 0)
+            return (EBUSY);
This is extra-weird.
The devinuse business disallows simultaneous opens, which prevents
other process from opening and mapping. But if the original caller
does mmap and close, second process now is allowed to open and mmap.

That said, why do you need this devinuse at all ?
 
Hmm, I wanted to avoid multiple mmap's, but that doesn't work like you said.  I 
may just remove this restriction. 

+static int
+hpet_mmap(struct cdev *cdev, vm_ooffset_t offset, vm_paddr_t *paddr,
+ int nprot, vm_memattr_t *memattr)
+{
+    struct hpet_softc *sc;
+
+    sc = cdev->si_drv1;
+        if (offset > rman_get_size(sc->mem_res))
+                return (EINVAL);
+    if (!sc->mmap_allow_write && (nprot & PROT_WRITE))
+      return (EPERM);
+      *paddr = rman_get_start(sc->mem_res) + offset;
What is the memattr for the backing page ? Is it set to non-cached
mode somehow ? I was not able to find place where would this happen.
 
I expect it to be set to non-cached since it's a device anyway, but I don't 
know where it is.  During my testing, I did not see any problems with cached 
values, though.

+  sc->pdev = make_dev(&hpet_cdevsw, 0, UID_ROOT, GID_WHEEL,
+         0600, "hpet%d", device_get_unit(dev));
+        if (sc->pdev) {
+                sc->pdev->si_drv1 = sc;
+      sc->mmap_allow = 1;
+                TUNABLE_INT_FETCH("hw.acpi.hpet.mmap_allow",
+             &sc->mmap_allow);
+            sc->mmap_allow_write = 1;
+            TUNABLE_INT_FETCH("hw.acpi.hpet.mmap_allow_write",
+                 &sc->mmap_allow_write);
+                SYSCTL_ADD_INT(device_get_sysctl_ctx(dev),
+                 SYSCTL_CHILDREN(device_get_sysctl_tree(dev)),
+       OID_AUTO, "mmap_allow",
+           CTLFLAG_RW, &sc->mmap_allow, 0,
+                 "Allow userland to memory map HPET");
Why is mmap_allow is per-instance, while mmap_allow_write taken from
the global tunable ?
 
Are you asking why there's no sysctl for it?
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to