From 6805510d77f4eff685bb710a4ebd52563ecb54df Mon Sep 17 00:00:00 2001 From: Liang Yi Date: Mon, 19 Apr 2021 16:55:07 +0800 Subject: [PATCH] hv/mod_timer: refine timer interface 1. do not allow external modules to touch internal field of a timer. 2. make timer mode internal, period_in_ticks will decide the mode. API wise: 1. the "mode" parameter was taken out of initialize_timer(). 2. a new function update_timer() was added to update the timeout and period fields. 3. the timer_expired() function was extended with an output parameter to return the remaining cycles before expiration. Also, the "fire_tsc" field name of hv_timer was renamed to "timeout". With the new API, however, this change should not concern user code. Tracked-On: #5920 Signed-off-by: Jason Chen CJ --- hypervisor/arch/x86/guest/vlapic.c | 47 ++++++------------- hypervisor/common/ptdev.c | 10 ++-- hypervisor/common/sched_bvt.c | 2 +- hypervisor/common/sched_iorr.c | 2 +- hypervisor/common/timer.c | 75 ++++++++++++++++++++++++++---- hypervisor/debug/console.c | 2 +- hypervisor/include/common/timer.h | 46 +++++++++--------- 7 files changed, 108 insertions(+), 76 deletions(-) diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index c4f534727..88469f3c1 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -260,9 +260,7 @@ static void vlapic_init_timer(struct acrn_vlapic *vlapic) vtimer = &vlapic->vtimer; (void)memset(vtimer, 0U, sizeof(struct vlapic_timer)); - initialize_timer(&vtimer->timer, - vlapic_timer_expired, vlapic2vcpu(vlapic), - 0UL, 0, 0UL); + initialize_timer(&vtimer->timer, vlapic_timer_expired, vlapic2vcpu(vlapic), 0UL, 0UL); } /** @@ -274,16 +272,12 @@ static void vlapic_reset_timer(struct acrn_vlapic *vlapic) timer = &vlapic->vtimer.timer; del_timer(timer); - timer->mode = TICK_MODE_ONESHOT; - timer->fire_tsc = 0UL; - timer->period_in_cycle = 0UL; + update_timer(timer, 0UL, 0UL); } static bool set_expiration(struct acrn_vlapic *vlapic) { - uint64_t now = cpu_ticks(); - uint64_t delta; struct vlapic_timer *vtimer; struct hv_timer *timer; uint32_t tmicr, divisor_shift; @@ -296,13 +290,11 @@ set_expiration(struct acrn_vlapic *vlapic) if ((tmicr == 0U) || (divisor_shift > 8U)) { ret = false; } else { - delta = (uint64_t)tmicr << divisor_shift; + uint64_t delta = (uint64_t)tmicr << divisor_shift; timer = &vtimer->timer; - if (vlapic_lvtt_period(vlapic)) { - timer->period_in_cycle = delta; - } - timer->fire_tsc = now + delta; + update_timer(timer, cpu_ticks() + delta, + vlapic_lvtt_period(vlapic) ? delta : 0UL); ret = true; } return ret; @@ -322,10 +314,7 @@ static void vlapic_update_lvtt(struct acrn_vlapic *vlapic, * the timer mode disarms the local APIC timer. */ del_timer(timer); - timer->mode = (timer_mode == APIC_LVTT_TM_PERIODIC) ? - TICK_MODE_PERIODIC: TICK_MODE_ONESHOT; - timer->fire_tsc = 0UL; - timer->period_in_cycle = 0UL; + update_timer(timer, 0UL, 0UL); vtimer->mode = timer_mode; } @@ -333,19 +322,13 @@ static void vlapic_update_lvtt(struct acrn_vlapic *vlapic, static uint32_t vlapic_get_ccr(const struct acrn_vlapic *vlapic) { - uint64_t now = cpu_ticks(); uint32_t remain_count = 0U; - const struct vlapic_timer *vtimer; - - vtimer = &vlapic->vtimer; + const struct vlapic_timer *vtimer = &vlapic->vtimer; if ((vtimer->tmicr != 0U) && (!vlapic_lvtt_tsc_deadline(vlapic))) { - uint64_t fire_tsc = vtimer->timer.fire_tsc; - - if (now < fire_tsc) { - uint32_t divisor_shift = vtimer->divisor_shift; - uint64_t shifted_delta = - (fire_tsc - now) >> divisor_shift; + uint64_t remains; + if (!timer_expired(&vtimer->timer, cpu_ticks(), &remains)) { + uint64_t shifted_delta = remains >> vtimer->divisor_shift; remain_count = (uint32_t)shifted_delta; } } @@ -405,7 +388,7 @@ uint64_t vlapic_get_tsc_deadline_msr(const struct acrn_vlapic *vlapic) } else if (!vlapic_lvtt_tsc_deadline(vlapic)) { ret = 0UL; } else { - ret = (vlapic->vtimer.timer.fire_tsc == 0UL) ? 0UL : + ret = timer_expired(&vlapic->vtimer.timer, cpu_ticks(), NULL) ? 0UL : vcpu_get_guest_msr(vcpu, MSR_IA32_TSC_DEADLINE); } @@ -444,14 +427,14 @@ void vlapic_set_tsc_deadline_msr(struct acrn_vlapic *vlapic, uint64_t val_arg) if (val != 0UL) { /* transfer guest tsc to host tsc */ val -= exec_vmread64(VMX_TSC_OFFSET_FULL); - timer->fire_tsc = val; + update_timer(timer, val, 0UL); /* vlapic_init_timer has been called, * and timer->fire_tsc is not 0,here * add_timer should not return error */ (void)add_timer(timer); } else { - timer->fire_tsc = 0UL; + update_timer(timer, 0UL, 0UL); } } else { /* No action required */ @@ -2039,10 +2022,6 @@ static void vlapic_timer_expired(void *data) if (!vlapic_lvtt_masked(vlapic)) { vlapic_set_intr(vcpu, lapic->lvt[APIC_LVT_TIMER].v & APIC_LVTT_VECTOR, LAPIC_TRIG_EDGE); } - - if (!vlapic_lvtt_period(vlapic)) { - vlapic->vtimer.timer.fire_tsc = 0UL; - } } /* diff --git a/hypervisor/common/ptdev.c b/hypervisor/common/ptdev.c index 9c5c43b0b..1a985e1e5 100644 --- a/hypervisor/common/ptdev.c +++ b/hypervisor/common/ptdev.c @@ -104,7 +104,7 @@ struct ptirq_remapping_info *ptirq_dequeue_softirq(uint16_t pcpu_id) list_del_init(&entry->softirq_node); /* if sos vm, just dequeue, if uos, check delay timer */ - if (is_sos_vm(entry->vm) || timer_expired(&entry->intr_delay_timer)) { + if (is_sos_vm(entry->vm) || timer_expired(&entry->intr_delay_timer, cpu_ticks(), NULL)) { break; } else { /* add it into timer list; dequeue next one */ @@ -133,7 +133,7 @@ struct ptirq_remapping_info *ptirq_alloc_entry(struct acrn_vm *vm, uint32_t intr INIT_LIST_HEAD(&entry->softirq_node); - initialize_timer(&entry->intr_delay_timer, ptirq_intr_delay_callback, entry, 0UL, 0, 0UL); + initialize_timer(&entry->intr_delay_timer, ptirq_intr_delay_callback, entry, 0UL, 0UL); entry->active = false; } else { @@ -177,11 +177,11 @@ static void ptirq_interrupt_handler(__unused uint32_t irq, void *data) if (timer_is_started(&entry->intr_delay_timer)) { to_enqueue = false; } else { - entry->intr_delay_timer.fire_tsc = - cpu_ticks() + entry->vm->intr_inject_delay_delta; + update_timer(&entry->intr_delay_timer, + cpu_ticks() + entry->vm->intr_inject_delay_delta, 0UL); } } else { - entry->intr_delay_timer.fire_tsc = 0UL; + update_timer(&entry->intr_delay_timer, 0UL, 0UL); } } diff --git a/hypervisor/common/sched_bvt.c b/hypervisor/common/sched_bvt.c index 3f699822b..946c3ab25 100644 --- a/hypervisor/common/sched_bvt.c +++ b/hypervisor/common/sched_bvt.c @@ -159,7 +159,7 @@ static int sched_bvt_init(struct sched_control *ctl) /* The tick_timer is periodically */ initialize_timer(&bvt_ctl->tick_timer, sched_tick_handler, ctl, - cpu_ticks() + tick_period, TICK_MODE_PERIODIC, tick_period); + cpu_ticks() + tick_period, tick_period); if (add_timer(&bvt_ctl->tick_timer) < 0) { pr_err("Failed to add schedule tick timer!"); diff --git a/hypervisor/common/sched_iorr.c b/hypervisor/common/sched_iorr.c index c7339acb2..ee4eb9a8c 100644 --- a/hypervisor/common/sched_iorr.c +++ b/hypervisor/common/sched_iorr.c @@ -117,7 +117,7 @@ int sched_iorr_init(struct sched_control *ctl) /* The tick_timer is periodically */ initialize_timer(&iorr_ctl->tick_timer, sched_tick_handler, ctl, - cpu_ticks() + tick_period, TICK_MODE_PERIODIC, tick_period); + cpu_ticks() + tick_period, tick_period); if (add_timer(&iorr_ctl->tick_timer) < 0) { pr_err("Failed to add schedule tick timer!"); diff --git a/hypervisor/common/timer.c b/hypervisor/common/timer.c index 7ca613318..7b940143d 100644 --- a/hypervisor/common/timer.c +++ b/hypervisor/common/timer.c @@ -20,14 +20,36 @@ #define MAX_TIMER_ACTIONS 32U #define MIN_TIMER_PERIOD_US 500U +bool timer_expired(const struct hv_timer *timer, uint64_t now, uint64_t *delta) +{ + bool ret = true; + uint64_t delt = 0UL; + + if ((timer->timeout != 0UL) && (now < timer->timeout)) { + ret = false; + delt = timer->timeout - now; + } + + if (delta != NULL) { + *delta = delt; + } + + return ret; +} + +bool timer_is_started(const struct hv_timer *timer) +{ + return (!list_empty(&timer->node)); +} + static void run_timer(const struct hv_timer *timer) { /* deadline = 0 means stop timer, we should skip */ - if ((timer->func != NULL) && (timer->fire_tsc != 0UL)) { + if ((timer->func != NULL) && (timer->timeout != 0UL)) { timer->func(timer->priv_data); } - TRACE_2L(TRACE_TIMER_ACTION_PCKUP, timer->fire_tsc, 0UL); + TRACE_2L(TRACE_TIMER_ACTION_PCKUP, timer->timeout, 0UL); } static inline void update_physical_timer(struct per_cpu_timers *cpu_timer) @@ -40,7 +62,7 @@ static inline void update_physical_timer(struct per_cpu_timers *cpu_timer) struct hv_timer, node); /* it is okay to program a expired time */ - msr_write(MSR_IA32_TSC_DEADLINE, timer->fire_tsc); + msr_write(MSR_IA32_TSC_DEADLINE, timer->timeout); } } @@ -52,12 +74,12 @@ static bool local_add_timer(struct per_cpu_timers *cpu_timer, { struct list_head *pos, *prev; struct hv_timer *tmp; - uint64_t tsc = timer->fire_tsc; + uint64_t tsc = timer->timeout; prev = &cpu_timer->timer_list; list_for_each(pos, &cpu_timer->timer_list) { tmp = container_of(pos, struct hv_timer, node); - if (tmp->fire_tsc < tsc) { + if (tmp->timeout < tsc) { prev = &tmp->node; } else { break; @@ -76,7 +98,7 @@ int32_t add_timer(struct hv_timer *timer) int32_t ret = 0; uint64_t rflags; - if ((timer == NULL) || (timer->func == NULL) || (timer->fire_tsc == 0UL)) { + if ((timer == NULL) || (timer->func == NULL) || (timer->timeout == 0UL)) { ret = -EINVAL; } else { ASSERT(list_empty(&timer->node), "add timer again!\n"); @@ -96,13 +118,46 @@ int32_t add_timer(struct hv_timer *timer) } CPU_INT_ALL_RESTORE(rflags); - TRACE_2L(TRACE_TIMER_ACTION_ADDED, timer->fire_tsc, 0UL); + TRACE_2L(TRACE_TIMER_ACTION_ADDED, timer->timeout, 0UL); } return ret; } +void initialize_timer(struct hv_timer *timer, + timer_handle_t func, void *priv_data, + uint64_t timeout, uint64_t period_in_cycle) +{ + if (timer != NULL) { + timer->func = func; + timer->priv_data = priv_data; + timer->timeout = timeout; + if (period_in_cycle > 0UL) { + timer->mode = TICK_MODE_PERIODIC; + timer->period_in_cycle = period_in_cycle; + } else { + timer->mode = TICK_MODE_ONESHOT; + timer->period_in_cycle = 0UL; + } + INIT_LIST_HEAD(&timer->node); + } +} + +void update_timer(struct hv_timer *timer, uint64_t timeout, uint64_t period) +{ + if (timer != NULL) { + timer->timeout = timeout; + if (period > 0UL) { + timer->mode = TICK_MODE_PERIODIC; + timer->period_in_cycle = period; + } else { + timer->mode = TICK_MODE_ONESHOT; + timer->period_in_cycle = 0UL; + } + } +} + void del_timer(struct hv_timer *timer) { uint64_t rflags; @@ -143,15 +198,17 @@ static void timer_softirq(uint16_t pcpu_id) timer = container_of(pos, struct hv_timer, node); /* timer expried */ tries--; - if ((timer->fire_tsc <= current_tsc) && (tries != 0U)) { + if ((timer->timeout <= current_tsc) && (tries != 0U)) { del_timer(timer); run_timer(timer); if (timer->mode == TICK_MODE_PERIODIC) { /* update periodic timer fire tsc */ - timer->fire_tsc += timer->period_in_cycle; + timer->timeout += timer->period_in_cycle; (void)local_add_timer(cpu_timer, timer); + } else { + timer->timeout = 0UL; } } else { break; diff --git a/hypervisor/debug/console.c b/hypervisor/debug/console.c index 811cc7ad3..ced1b85eb 100644 --- a/hypervisor/debug/console.c +++ b/hypervisor/debug/console.c @@ -166,7 +166,7 @@ void console_setup_timer(void) fire_tsc = cpu_ticks() + period_in_cycle; initialize_timer(&console_timer, console_timer_callback, NULL, - fire_tsc, TICK_MODE_PERIODIC, period_in_cycle); + fire_tsc, period_in_cycle); /* Start an periodic timer */ if (add_timer(&console_timer) != 0) { diff --git a/hypervisor/include/common/timer.h b/hypervisor/include/common/timer.h index 340498aa3..a4807f04b 100644 --- a/hypervisor/include/common/timer.h +++ b/hypervisor/include/common/timer.h @@ -40,8 +40,8 @@ struct per_cpu_timers { struct hv_timer { struct list_head node; /**< link all timers */ enum tick_mode mode; /**< timer mode: one-shot or periodic */ - uint64_t fire_tsc; /**< tsc deadline to interrupt */ - uint64_t period_in_cycle; /**< period of the periodic timer in unit of TSC cycles */ + uint64_t timeout; /**< tsc deadline to interrupt */ + uint64_t period_in_cycle; /**< period of the periodic timer in CPU ticks */ timer_handle_t func; /**< callback if time reached */ void *priv_data; /**< func private data */ }; @@ -55,7 +55,6 @@ struct hv_timer { * @param[in] func irq callback if time reached. * @param[in] priv_data func private data. * @param[in] fire_tsc tsc deadline to interrupt. - * @param[in] mode timer mode. * @param[in] period_in_cycle period of the periodic timer in unit of TSC cycles. * * @remark Don't initialize a timer twice if it has been added to the timer list @@ -63,43 +62,40 @@ struct hv_timer { * * @return None */ -static inline void initialize_timer(struct hv_timer *timer, - timer_handle_t func, void *priv_data, - uint64_t fire_tsc, int32_t mode, uint64_t period_in_cycle) -{ - if (timer != NULL) { - timer->func = func; - timer->priv_data = priv_data; - timer->fire_tsc = fire_tsc; - timer->mode = mode; - timer->period_in_cycle = period_in_cycle; - INIT_LIST_HEAD(&timer->node); - } -} +void initialize_timer(struct hv_timer *timer, + timer_handle_t func, void *priv_data, + uint64_t fire_tsc, uint64_t period_in_cycle); /** * @brief Check a timer whether expired. * * @param[in] timer Pointer to timer. + * @param[in] now to compare. + * @param[in] delta Pointer to return the delta (timeout - now) if timer is not expired. * * @retval true if the timer is expired, false otherwise. */ -static inline bool timer_expired(const struct hv_timer *timer) -{ - return ((timer->fire_tsc == 0UL) || (cpu_ticks() >= timer->fire_tsc)); -} +bool timer_expired(const struct hv_timer *timer, uint64_t now, uint64_t *delta); /** - * @brief Check a timer whether in timer list. + * @brief Check if a timer is active (in the timer list) or not. * * @param[in] timer Pointer to timer. * * @retval true if the timer is in timer list, false otherwise. */ -static inline bool timer_is_started(const struct hv_timer *timer) -{ - return (!list_empty(&timer->node)); -} +bool timer_is_started(const struct hv_timer *timer); + +/** + * @brief Update a timer. + * + * @param[in] timer Pointer to timer. + * @param[in] timeout deadline to interrupt. + * @param[in] period period of the periodic timer in unit of CPU ticks. + * + * @return None + */ +void update_timer(struct hv_timer *timer, uint64_t timeout, uint64_t period); /** * @brief Add a timer.