hv: pirq: clean up irq handlers

There are several similar irq handlers with confusing function names and it's
not friendly to call update_irq_handler() to update a proper handler after
request_irq().

With this commit, a single generic irq handler is being used, in which, no lock
need to be acquired because our design could guarantee there is no concurrent
irq handling and irq handler request/free.
A flags field is added to irq_desc struct to select the proper processing flow
for an irq. Irqflags is defined as follows:
 IRQF_NONE       (0U)
 IRQF_LEVEL      (1U << 1U)       /* 1: level trigger; 0: edge trigger */
 IRQF_PT         (1U << 2U)       /* 1: for passthrough dev */

Because we have only one irq handler, update_irq_handler() should be replace by
set_irq_trigger_mode(), whichs set trigger mode flag of a certian irq.
Accordingly, the code where called update_irq_handler() need to be updated.

Signed-off-by: Yan, Like <like.yan@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>

hypervisor/arch/x86/assign.c

@@ -139,51 +139,6 @@ lookup_entry_by_vintx(struct vm *vm, uint8_t vpin,
 	return entry;
 }
 
-static void
-ptdev_update_irq_handler(struct vm *vm, struct ptdev_remapping_info *entry)
-{
-	uint32_t phys_irq = entry->allocated_pirq;
-	struct ptdev_intx_info *intx = &entry->ptdev_intr_info.intx;
-
-	if (entry->type == PTDEV_INTR_MSI) {
-		/* all other MSI and normal maskable */
-		update_irq_handler(phys_irq, common_handler_edge);
-	}
-	/* update irq handler for IOAPIC */
-	if ((entry->type == PTDEV_INTR_INTX)
-		&& (intx->vpin_src
-			== PTDEV_VPIN_IOAPIC)) {
-		union ioapic_rte rte;
-		bool trigger_lvl = false;
-
-		/* VPIN_IOAPIC src means we have vioapic enabled */
-		vioapic_get_rte(vm, intx->virt_pin, &rte);
-		if ((rte.full & IOAPIC_RTE_TRGRMOD) == IOAPIC_RTE_TRGRLVL) {
-			trigger_lvl = true;
-		}
-
-		if (trigger_lvl) {
-			update_irq_handler(phys_irq, common_dev_handler_level);
-		} else {
-			update_irq_handler(phys_irq, common_handler_edge);
-		}
-	}
-	/* update irq handler for PIC */
-	if ((entry->type == PTDEV_INTR_INTX) && (phys_irq < NR_LEGACY_IRQ)
-		&& (intx->vpin_src == PTDEV_VPIN_PIC)) {
-		enum vpic_trigger trigger;
-
-		/* VPIN_PIC src means we have vpic enabled */
-		vpic_get_irq_trigger(vm,
-		        intx->virt_pin, &trigger);
-		if (trigger == LEVEL_TRIGGER) {
-			update_irq_handler(phys_irq, common_dev_handler_level);
-		} else {
-			update_irq_handler(phys_irq, common_handler_edge);
-		}
-	}
-}
-
 static bool ptdev_hv_owned_intx(struct vm *vm, struct ptdev_intx_info *info)
 {
 	/* vm0 pin 4 (uart) is owned by hypervisor under debug version */
@@ -677,9 +632,6 @@ int ptdev_msix_remap(struct vm *vm, uint16_t virt_bdf,
 	entry->ptdev_intr_info.msi.phys_vector =
 					irq_to_vector(entry->allocated_pirq);
 
-	/* update irq handler according to info in guest */
-	ptdev_update_irq_handler(vm, entry);
-
 	dev_dbg(ACRN_DBG_IRQ,
 		"PCI %x:%x.%x MSI VR[%d] 0x%x->0x%x assigned to vm%d",
 		(entry->virt_bdf >> 8) & 0xFFU,
@@ -711,6 +663,7 @@ static void activate_physical_ioapic(struct vm *vm,
 {
 	union ioapic_rte rte;
 	uint32_t phys_irq = entry->allocated_pirq;
+	bool is_lvl_trigger = false;
 
 	/* disable interrupt */
 	GSI_MASK_IRQ(phys_irq);
@@ -722,8 +675,11 @@ static void activate_physical_ioapic(struct vm *vm,
 	rte.full |= IOAPIC_RTE_INTMSET;
 	ioapic_set_rte(phys_irq, rte);
 
-	/* update irq handler according to info in guest */
+	/* update irq trigger mode according to info in guest */
-	ptdev_update_irq_handler(vm, entry);
+	if ((rte.full & IOAPIC_RTE_TRGRMOD) == IOAPIC_RTE_TRGRLVL) {
+		is_lvl_trigger = true;
+	}
+	set_irq_trigger_mode(phys_irq, is_lvl_trigger);
 
 	/* enable interrupt */
 	GSI_UNMASK_IRQ(phys_irq);

hypervisor/arch/x86/ioapic.c

@@ -219,9 +219,9 @@ static void ioapic_set_routing(uint32_t gsi, uint32_t vr)
 	ioapic_set_rte_entry(addr, gsi_table[gsi].pin, rte);
 
 	if ((rte.full & IOAPIC_RTE_TRGRMOD) != 0UL) {
-		update_irq_handler(gsi, handle_level_interrupt_common);
+		set_irq_trigger_mode(gsi, true);
 	} else {
-		update_irq_handler(gsi, common_handler_edge);
+		set_irq_trigger_mode(gsi, false);
 	}
 
 	dev_dbg(ACRN_DBG_IRQ, "GSI: irq:%d pin:%hhu rte:%lx",

hypervisor/arch/x86/irq.c

@@ -15,6 +15,8 @@ static uint32_t vector_to_irq[NR_MAX_VECTOR + 1];
 
 spurious_handler_t spurious_handler;
 
+static inline void handle_irq(struct irq_desc *desc);
+
 #define NR_STATIC_MAPPINGS     (2U)
 static uint32_t irq_static_mappings[NR_STATIC_MAPPINGS][2] = {
 	{TIMER_IRQ, VECTOR_TIMER},
@@ -212,7 +214,8 @@ static void disable_pic_irq(void)
  */
 int32_t request_irq(uint32_t req_irq,
 		    irq_action_t action_fn,
-		    void *priv_data)
+		    void *priv_data,
+		    uint32_t flags)
 {
 	struct irq_desc *desc;
 	uint32_t irq, vector;
@@ -234,11 +237,8 @@ int32_t request_irq(uint32_t req_irq,
 
 	desc = &irq_desc_array[irq];
 	spinlock_irqsave_obtain(&desc->lock, &rflags);
-	if (desc->irq_handler == NULL) {
-		desc->irq_handler = common_handler_edge;
-	}
-
 	if (desc->action == NULL) {
+		desc->flags = flags;
 		desc->priv_data = priv_data;
 		desc->action = action_fn;
 		spinlock_irqrestore_release(&desc->lock, rflags);
@@ -327,12 +327,12 @@ void dispatch_interrupt(struct intr_excp_ctx *ctx)
 		goto ERR;
 	}
 
-	if ((desc->used == IRQ_NOT_ASSIGNED) || (desc->irq_handler == NULL)) {
+	if (desc->used == IRQ_NOT_ASSIGNED) {
 		/* mask irq if possible */
 		goto ERR;
 	}
 
-	desc->irq_handler(desc, NULL);
+	handle_irq(desc);
 	return;
 ERR:
 	handle_spurious_interrupt(vr);
@@ -361,16 +361,26 @@ void partition_mode_dispatch_interrupt(struct intr_excp_ctx *ctx)
 }
 #endif
 
-int handle_level_interrupt_common(struct irq_desc *desc,
+static inline bool irq_need_mask(struct irq_desc *desc)
-			__unused void *handler_data)
+{
+	/* level triggered gsi should be masked */
+	return (((desc->flags & IRQF_LEVEL) != 0U)
+		&& irq_is_gsi(desc->irq));
+}
+
+static inline bool irq_need_unmask(struct irq_desc *desc)
+{
+	/* level triggered gsi for non-ptdev should be unmasked */
+	return (((desc->flags & IRQF_LEVEL) != 0U)
+		&& ((desc->flags & IRQF_PT) == 0U)
+		&& irq_is_gsi(desc->irq));
+}
+
+static inline void handle_irq(struct irq_desc *desc)
 {
-	uint64_t rflags;
 	irq_action_t action = desc->action;
 
-	spinlock_irqsave_obtain(&desc->lock, &rflags);
+	if (irq_need_mask(desc)) {
-
-	/* mask iopaic pin */
-	if (irq_is_gsi(desc->irq)) {
 		GSI_MASK_IRQ(desc->irq);
 	}
 
@@ -381,75 +391,12 @@ int handle_level_interrupt_common(struct irq_desc *desc,
 		action(desc->irq, desc->priv_data);
  	}
 
-	if (irq_is_gsi(desc->irq)) {
+	if (irq_need_unmask(desc)) {
 		GSI_UNMASK_IRQ(desc->irq);
 	}
-
-	spinlock_irqrestore_release(&desc->lock, rflags);
-
-	return 0;
 }
 
-int common_handler_edge(struct irq_desc *desc, __unused void *handler_data)
+void set_irq_trigger_mode(uint32_t irq, bool is_level_trigger)
-{
-	uint64_t rflags;
-	irq_action_t action = desc->action;
-
-	spinlock_irqsave_obtain(&desc->lock, &rflags);
-
-	/* Send EOI to LAPIC/IOAPIC IRR */
-	send_lapic_eoi();
-
-	if (action != NULL) {
-		action(desc->irq, desc->priv_data);
- 	}
-
-	spinlock_irqrestore_release(&desc->lock, rflags);
-
-	return 0;
-}
-
-int common_dev_handler_level(struct irq_desc *desc, __unused void *handler_data)
-{
-	uint64_t rflags;
-	irq_action_t action = desc->action;
-
-	spinlock_irqsave_obtain(&desc->lock, &rflags);
-
-	/* mask iopaic pin */
-	if (irq_is_gsi(desc->irq))  {
-		GSI_MASK_IRQ(desc->irq);
-	}
-
-	/* Send EOI to LAPIC/IOAPIC IRR */
-	send_lapic_eoi();
-
-	if (action != NULL) {
-		action(desc->irq, desc->priv_data);
- 	}
-
-	spinlock_irqrestore_release(&desc->lock, rflags);
-
-	/* we did not unmask irq until guest EOI the vector */
-	return 0;
-}
-
-/* no desc->lock for quick handling local interrupt like lapic timer */
-int quick_handler_nolock(struct irq_desc *desc, __unused void *handler_data)
-{
-	irq_action_t action = desc->action;
-
-	/* Send EOI to LAPIC/IOAPIC IRR */
-	send_lapic_eoi();
-
-	if (action != NULL) {
-		action(desc->irq, desc->priv_data);
- 	}
-
-	return 0;
-}
-
-void update_irq_handler(uint32_t irq, irq_handler_t func)
 {
 	uint64_t rflags;
 	struct irq_desc *desc;
@@ -460,7 +407,11 @@ void update_irq_handler(uint32_t irq, irq_handler_t func)
 
 	desc = &irq_desc_array[irq];
 	spinlock_irqsave_obtain(&desc->lock, &rflags);
-	desc->irq_handler = func;
+	if (is_level_trigger == true) {
+		desc->flags |= IRQF_LEVEL;
+	} else {
+		desc->flags &= ~IRQF_LEVEL;
+	}
 	spinlock_irqrestore_release(&desc->lock, rflags);
 }
 
@@ -483,6 +434,7 @@ void free_irq(uint32_t irq)
 	spinlock_irqsave_obtain(&desc->lock, &rflags);
 	desc->action = NULL;
 	desc->priv_data = NULL;
+	desc->flags = IRQF_NONE;
 	spinlock_irqrestore_release(&desc->lock, rflags);
 }
 

hypervisor/arch/x86/notify.c

@@ -66,7 +66,7 @@ static int request_notification_irq(irq_action_t func, void *data)
 	}
 
 	/* all cpu register the same notification vector */
-	retval = request_irq(NOTIFY_IRQ, func, data);
+	retval = request_irq(NOTIFY_IRQ, func, data, IRQF_NONE);
 	if (retval < 0) {
 		pr_err("Failed to add notify isr");
 		return -ENODEV;

hypervisor/arch/x86/timer.c

@@ -24,7 +24,7 @@ static void run_timer(struct hv_timer *timer)
 }
 
 /* run in interrupt context */
-static int tsc_deadline_handler(__unused int irq, __unused void *data)
+static int tsc_deadline_handler(__unused uint32_t irq, __unused void *data)
 {
 	fire_softirq(SOFTIRQ_TIMER);
 	return 0;
@@ -107,21 +107,6 @@ void del_timer(struct hv_timer *timer)
 	}
 }
 
-static int request_timer_irq(irq_action_t func)
-{
-	int32_t retval;
-
-	retval = request_irq(TIMER_IRQ, func, NULL);
-	if (retval >= 0) {
-		update_irq_handler(TIMER_IRQ, quick_handler_nolock);
-	} else {
-		pr_err("Failed to add timer isr");
-		return -ENODEV;
-	}
-
-	return 0;
-}
-
 static void init_percpu_timer(uint16_t pcpu_id)
 {
 	struct per_cpu_timers *cpu_timer;
@@ -186,14 +171,16 @@ static void timer_softirq(uint16_t pcpu_id)
 void timer_init(void)
 {
 	uint16_t pcpu_id = get_cpu_id();
+	int32_t retval;
 
 	init_percpu_timer(pcpu_id);
 
 	if (pcpu_id == BOOT_CPU_ID) {
 		register_softirq(SOFTIRQ_TIMER, timer_softirq);
 
-		if (request_timer_irq((irq_action_t)tsc_deadline_handler)
+		retval = request_irq(TIMER_IRQ, (irq_action_t)tsc_deadline_handler,
-				< 0) {
+				     NULL, IRQF_NONE);
+		if (retval < 0) {
 			pr_err("Timer setup failed");
 			return;
 		}

hypervisor/arch/x86/vtd.c

@@ -828,7 +828,8 @@ static int dmar_setup_interrupt(struct dmar_drhd_rt *dmar_uint)
 
 	retval = request_irq(IRQ_INVALID,
 			     dmar_fault_handler,
-			     dmar_uint);
+			     dmar_uint,
+			     IRQF_NONE);
 
 	if (retval < 0 ) {
 		pr_err("%s: fail to setup interrupt", __func__);

hypervisor/common/ptdev.c

@@ -136,7 +136,7 @@ ptdev_activate_entry(struct ptdev_remapping_info *entry, uint32_t phys_irq)
 
 	/* register and allocate host vector/irq */
 	retval = request_irq(phys_irq, ptdev_interrupt_handler,
-				         (void *)entry);
+		             (void *)entry, IRQF_PT);
 
 	ASSERT(retval >= 0, "dev register failed");
 	entry->allocated_pirq = (uint32_t)retval;

hypervisor/include/common/irq.h

@@ -7,6 +7,10 @@
 #ifndef COMMON_IRQ_H
 #define COMMON_IRQ_H
 
+#define IRQF_NONE	(0U)
+#define IRQF_LEVEL	(1U << 1U)	/* 1: level trigger; 0: edge trigger */
+#define IRQF_PT		(1U << 2U)	/* 1: for passthrough dev */
+
 enum irq_mode {
 	IRQ_PULSE,
 	IRQ_ASSERT,
@@ -26,20 +30,19 @@ struct irq_desc {
 	enum irq_use_state used;	/* this irq have assigned to device */
 	uint32_t vector;	/* assigned vector */
 
-	int (*irq_handler)(struct irq_desc *irq_desc, void *handler_data);
-				/* callback for irq flow handling */
 	irq_action_t action;	/* callback registered from component */
 	void *priv_data;	/* irq_action private data */
+	uint32_t flags;		/* flags for trigger mode/ptdev */
 
 	spinlock_t lock;
 };
 
 int32_t request_irq(uint32_t irq,
 		    irq_action_t action_fn,
-		    void *priv_data);
+		    void *priv_data,
+		    uint32_t flags);
 
 void free_irq(uint32_t irq);
 
-typedef int (*irq_handler_t)(struct irq_desc *desc, void *handler_data);
+void set_irq_trigger_mode(uint32_t irq, bool is_level_trigger);
-void update_irq_handler(uint32_t irq, irq_handler_t func);
 #endif /* COMMON_IRQ_H */