svn commit: r273598 - in head: include sys/dev/acpica

[Rui Paulo]

On Oct 24, 2014, at 12:20 PM, Konstantin Belousov <kostikbel at 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?