Project

General

Profile

Actions

Bug #14974

closed

bhyve vhpet needs one-shot timers

Added by Patrick Mooney 2 months ago. Updated 23 days ago.

Status:
Closed
Priority:
Normal
Category:
bhyve
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

The vhpet exposes a set of timers to the guest which can be configured in one-shot or periodic mode. The actual implementation for those timers appears to be missing a check for when they are not in periodic mode. The callout handler (which fires when the timer expires) is as follows:

static void
vhpet_handler(void *a)
{
        int n;
        uint32_t counter;
        hrtime_t now;
        struct vhpet *vhpet;
        struct callout *callout;
        struct vhpet_callout_arg *arg;

        arg = a;
        vhpet = arg->vhpet;
        n = arg->timer_num;
        callout = &vhpet->timer[n].callout;

        VHPET_LOCK(vhpet);

        if (callout_pending(callout))           /* callout was reset */
                goto done;

        if (!callout_active(callout))           /* callout was stopped */
                goto done;

        callout_deactivate(callout);

        if (!vhpet_counter_enabled(vhpet))
                panic("vhpet(%p) callout with counter disabled", vhpet);

        counter = vhpet_counter(vhpet, &now);
        vhpet_start_timer(vhpet, n, counter, now);
        vhpet_timer_interrupt(vhpet, n);
done:
        VHPET_UNLOCK(vhpet);
}

Note that vhpet_start_timer, which reschedules the callout, makes no check of the periodic status of the vhpet timer in question. If any guests are using this, it probably works by accident, as long as they reschedule their one-shot prior to the periodic firing again.


Related issues

Related to illumos gate - Bug #14568: bhyve should support pause/resumeClosedPatrick Mooney

Actions
Actions #1

Updated by Electric Monk about 1 month ago

  • Gerrit CR set to 2471
Actions #2

Updated by Patrick Mooney 24 days ago

To date, no guest is known to have tripped over this. Few even use the HPET, and those that do likely re-arm the timer soon enough that the previous not-really-one-shot behavior did not trip them up. The in-guest testing of timers is not particularly complete when it comes to actually letting the timers fire - the framework in place does not have any provisions for wiring up interrupts or exceptions in the guest code.

This is being tested hypothetically by running a Windows guest (which should use the HPET for its clock source) and looking for misbehavior. A nice future improvement to the in-guest testing framework would be the means of wiring up interrupts so that the one-shot vs. periodic nature can be exercised in a test.

Actions #3

Updated by Patrick Mooney 24 days ago

I checked a Windows guest and it is indeed using the vhpet in one-shot mode. When idle, it was programming the timer for a roughly 15.5ms duration, and reprogramming the timer within ~100us when it fires. This would explain why the lie of a one-shot was not exposed by a guest (at least by Windows).

Actions #4

Updated by Patrick Mooney 24 days ago

This was tested along side #14568, under which the normal test suite and typical guests ran fine.

Actions #5

Updated by Patrick Mooney 23 days ago

  • Related to Bug #14568: bhyve should support pause/resume added
Actions #6

Updated by Electric Monk 23 days ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 2cac0506db676bc6be4f12b32d0500f6f8ed429d

commit  2cac0506db676bc6be4f12b32d0500f6f8ed429d
Author: Patrick Mooney <pmooney@pfmooney.com>
Date:   2022-11-03T18:07:02.000Z

    14568 bhyve should support pause/resume
    14974 bhyve vhpet needs one-shot timers
    Reviewed by: Jordan Paige Hendricks <jordan@oxidecomputer.com>
    Reviewed by: Greg Colombo <greg@oxidecomputer.com>
    Approved by: Dan McDonald <danmcd@mnx.io>

Actions

Also available in: Atom PDF