From 457ecd6ef72dcb65f2e6e61133cd7ca08c8f32e6 Mon Sep 17 00:00:00 2001 From: "Li, Fei1" Date: Mon, 30 Jul 2018 15:58:36 +0800 Subject: [PATCH] hv: softirq: refine softirq 1. add register_softirq to register a softirq handler 2. rename exec_softirq to do_softirq; raise_softirq to fire_softirq. 3. in do_softirq call registered softirq handler not call the device softirq handle function directly 4. enable irq after vm exit and disable irq after the first call do_softirq before vm enter. 5. call do_softirq again when irq disabled to handle the risk unhandled softirq. 6. rename SOFTIRQ_DEV_ASSIGN to SOFTIRQ_PTDEV 7. remove SOFTIRQ_ATOMIC Signed-off-by: Li, Fei1 Acked-by: Eddie Dong --- hypervisor/arch/x86/irq.c | 1 + hypervisor/arch/x86/timer.c | 58 ++++++++-------- hypervisor/arch/x86/vmexit.c | 8 +-- hypervisor/common/hv_main.c | 9 ++- hypervisor/common/ptdev.c | 8 ++- hypervisor/common/softirq.c | 103 ++++++++-------------------- hypervisor/include/arch/x86/timer.h | 1 - hypervisor/include/common/softirq.h | 16 ++--- 8 files changed, 82 insertions(+), 122 deletions(-) diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c index fa5facbc6..177733e5a 100644 --- a/hypervisor/arch/x86/irq.c +++ b/hypervisor/arch/x86/irq.c @@ -5,6 +5,7 @@ */ #include +#include static spinlock_t exception_spinlock = { .head = 0U, .tail = 0U, }; diff --git a/hypervisor/arch/x86/timer.c b/hypervisor/arch/x86/timer.c index 9d78d74ea..3f3553db7 100644 --- a/hypervisor/arch/x86/timer.c +++ b/hypervisor/arch/x86/timer.c @@ -27,7 +27,7 @@ static void run_timer(struct hv_timer *timer) /* run in interrupt context */ static int tsc_deadline_handler(__unused int irq, __unused void *data) { - raise_softirq(SOFTIRQ_TIMER); + fire_softirq(SOFTIRQ_TIMER); return 0; } @@ -156,33 +156,7 @@ static void init_tsc_deadline_timer(void) msr_write(MSR_IA32_TSC_DEADLINE, 0UL); } -void timer_init(void) -{ - char name[32] = {0}; - uint16_t pcpu_id = get_cpu_id(); - - snprintf(name, 32, "timer_tick[%hu]", pcpu_id); - if (request_timer_irq(pcpu_id, tsc_deadline_handler, NULL, name) < 0) { - pr_err("Timer setup failed"); - return; - } - - init_tsc_deadline_timer(); - init_percpu_timer(pcpu_id); -} - -void timer_cleanup(void) -{ - uint16_t pcpu_id = get_cpu_id(); - - if (per_cpu(timer_node, pcpu_id) != NULL) { - unregister_handler_common(per_cpu(timer_node, pcpu_id)); - } - - per_cpu(timer_node, pcpu_id) = NULL; -} - -void timer_softirq(uint16_t pcpu_id) +static void timer_softirq(uint16_t pcpu_id) { struct per_cpu_timers *cpu_timer; struct hv_timer *timer; @@ -222,6 +196,34 @@ void timer_softirq(uint16_t pcpu_id) update_physical_timer(cpu_timer); } +void timer_init(void) +{ + char name[32] = {0}; + uint16_t pcpu_id = get_cpu_id(); + + init_percpu_timer(pcpu_id); + register_softirq(SOFTIRQ_TIMER, timer_softirq); + + snprintf(name, 32, "timer_tick[%hu]", pcpu_id); + if (request_timer_irq(pcpu_id, tsc_deadline_handler, NULL, name) < 0) { + pr_err("Timer setup failed"); + return; + } + + init_tsc_deadline_timer(); +} + +void timer_cleanup(void) +{ + uint16_t pcpu_id = get_cpu_id(); + + if (per_cpu(timer_node, pcpu_id) != NULL) { + unregister_handler_common(per_cpu(timer_node, pcpu_id)); + } + + per_cpu(timer_node, pcpu_id) = NULL; +} + void check_tsc(void) { uint64_t temp64; diff --git a/hypervisor/arch/x86/vmexit.c b/hypervisor/arch/x86/vmexit.c index c70965bf4..54e4cd857 100644 --- a/hypervisor/arch/x86/vmexit.c +++ b/hypervisor/arch/x86/vmexit.c @@ -216,11 +216,11 @@ int vmexit_handler(struct vcpu *vcpu) /* Handling external_interrupt * should disable intr */ - ret = dispatch->handler(vcpu); - } else { - CPU_IRQ_ENABLE(); - ret = dispatch->handler(vcpu); CPU_IRQ_DISABLE(); + ret = dispatch->handler(vcpu); + CPU_IRQ_ENABLE(); + } else { + ret = dispatch->handler(vcpu); } return ret; diff --git a/hypervisor/common/hv_main.c b/hypervisor/common/hv_main.c index f0779b6f1..d35751222 100644 --- a/hypervisor/common/hv_main.c +++ b/hypervisor/common/hv_main.c @@ -6,6 +6,7 @@ #include #include +#include bool x2apic_enabled; @@ -33,8 +34,11 @@ void vcpu_thread(struct vcpu *vcpu) run_vcpu_pre_work(vcpu); do { - /* handling pending softirq */ - exec_softirq(); + /* handle pending softirq when irq enable*/ + do_softirq(); + CPU_IRQ_DISABLE(); + /* handle risk softirq when disabling irq*/ + do_softirq(); /* Check and process pending requests(including interrupt) */ ret = acrn_handle_pending_request(vcpu); @@ -85,6 +89,7 @@ void vcpu_thread(struct vcpu *vcpu) /* Restore native TSC_AUX */ CPU_MSR_WRITE(MSR_IA32_TSC_AUX, tsc_aux_hyp_cpu); + CPU_IRQ_ENABLE(); /* Dispatch handler */ ret = vmexit_handler(vcpu); if (ret < 0) { diff --git a/hypervisor/common/ptdev.c b/hypervisor/common/ptdev.c index f49531429..c5f8cd61b 100644 --- a/hypervisor/common/ptdev.c +++ b/hypervisor/common/ptdev.c @@ -8,7 +8,7 @@ #include #include -/* SOFTIRQ_DEV_ASSIGN list for all CPUs */ +/* SOFTIRQ_PTDEV list for all CPUs */ struct list_head softirq_dev_entry_list; /* passthrough device link */ struct list_head ptdev_list; @@ -31,7 +31,7 @@ spinlock_t softirq_dev_lock; static void ptdev_enqueue_softirq(struct ptdev_remapping_info *entry) { spinlock_rflags; - /* enqueue request in order, SOFTIRQ_DEV_ASSIGN will pickup */ + /* enqueue request in order, SOFTIRQ_PTDEV will pickup */ spinlock_irqsave_obtain(&softirq_dev_lock); /* avoid adding recursively */ @@ -40,7 +40,7 @@ static void ptdev_enqueue_softirq(struct ptdev_remapping_info *entry) list_add_tail(&entry->softirq_node, &softirq_dev_entry_list); spinlock_irqrestore_release(&softirq_dev_lock); - raise_softirq(SOFTIRQ_DEV_ASSIGN); + fire_softirq(SOFTIRQ_PTDEV); } struct ptdev_remapping_info* @@ -169,6 +169,8 @@ void ptdev_init(void) spinlock_init(&ptdev_lock); INIT_LIST_HEAD(&softirq_dev_entry_list); spinlock_init(&softirq_dev_lock); + + register_softirq(SOFTIRQ_PTDEV, ptdev_softirq); } void ptdev_release_all_entries(struct vm *vm) diff --git a/hypervisor/common/softirq.c b/hypervisor/common/softirq.c index eda8fdedd..180621887 100644 --- a/hypervisor/common/softirq.c +++ b/hypervisor/common/softirq.c @@ -7,89 +7,42 @@ #include #include -void disable_softirq(uint16_t cpu_id) -{ - bitmap_clear_lock(SOFTIRQ_ATOMIC, &per_cpu(softirq_pending, cpu_id)); -} -void enable_softirq(uint16_t cpu_id) -{ - bitmap_set_lock(SOFTIRQ_ATOMIC, &per_cpu(softirq_pending, cpu_id)); -} +static softirq_handler softirq_handlers[NR_SOFTIRQS]; void init_softirq(void) { - uint16_t pcpu_id; - - for (pcpu_id = 0U; pcpu_id < phys_cpu_num; pcpu_id++) { - per_cpu(softirq_pending, pcpu_id) = 0UL; - bitmap_set_lock(SOFTIRQ_ATOMIC, &per_cpu(softirq_pending, pcpu_id)); - } } -void raise_softirq(uint16_t softirq_id) +/* + * @pre: nr will not equal or large than NR_SOFTIRQS + */ +void register_softirq(uint16_t nr, softirq_handler handler) { - uint16_t cpu_id = get_cpu_id(); - uint64_t *bitmap = &per_cpu(softirq_pending, cpu_id); - - if (cpu_id >= phys_cpu_num) { - return; - } - - bitmap_set_lock(softirq_id, bitmap); + softirq_handlers[nr] = handler; } -void exec_softirq(void) +/* + * @pre: nr will not equal or large than NR_SOFTIRQS + */ +void fire_softirq(uint16_t nr) { - uint16_t cpu_id = get_cpu_id(); - volatile uint64_t *bitmap = &per_cpu(softirq_pending, cpu_id); - - uint16_t softirq_id; - - if (cpu_id >= phys_cpu_num) { - return; - } - - if (((*bitmap) & SOFTIRQ_MASK) == 0UL) { - return; - } - - /* Disable softirq - * SOFTIRQ_ATOMIC bit = 0 means softirq already in execution - */ - if (!bitmap_test_and_clear_lock(SOFTIRQ_ATOMIC, bitmap)) { - return; - } - -again: - CPU_IRQ_ENABLE(); - - while (1) { - softirq_id = ffs64(*bitmap); - if ((softirq_id == INVALID_BIT_INDEX) || (softirq_id >= SOFTIRQ_MAX)) { - break; - } - - bitmap_clear_lock(softirq_id, bitmap); - - switch (softirq_id) { - case SOFTIRQ_TIMER: - timer_softirq(cpu_id); - break; - case SOFTIRQ_DEV_ASSIGN: - ptdev_softirq(cpu_id); - break; - default: - break; - - } - } - - CPU_IRQ_DISABLE(); - - if (((*bitmap) & SOFTIRQ_MASK) != 0U) { - goto again; - } - - enable_softirq(cpu_id); + bitmap_set_lock(nr, &per_cpu(softirq_pending, get_cpu_id())); +} + +void do_softirq(void) +{ + uint16_t nr; + uint16_t cpu_id = get_cpu_id(); + volatile uint64_t *softirq_pending_bitmap = + &per_cpu(softirq_pending, cpu_id); + + while (true) { + nr = ffs64(*softirq_pending_bitmap); + if (nr >= NR_SOFTIRQS) + break; + + bitmap_clear_lock(nr, softirq_pending_bitmap); + (*softirq_handlers[nr])(cpu_id); + } } diff --git a/hypervisor/include/arch/x86/timer.h b/hypervisor/include/arch/x86/timer.h index d0868bab2..453e85f57 100644 --- a/hypervisor/include/arch/x86/timer.h +++ b/hypervisor/include/arch/x86/timer.h @@ -54,7 +54,6 @@ static inline void initialize_timer(struct hv_timer *timer, int add_timer(struct hv_timer *timer); void del_timer(struct hv_timer *timer); -void timer_softirq(uint16_t pcpu_id); void timer_init(void); void timer_cleanup(void); void check_tsc(void); diff --git a/hypervisor/include/common/softirq.h b/hypervisor/include/common/softirq.h index 4f0280285..d21e71cf8 100644 --- a/hypervisor/include/common/softirq.h +++ b/hypervisor/include/common/softirq.h @@ -8,16 +8,14 @@ #define SOFTIRQ_H #define SOFTIRQ_TIMER 0U -#define SOFTIRQ_DEV_ASSIGN 1U -#define SOFTIRQ_MAX 2U -#define SOFTIRQ_MASK ((1UL<