From 460e7ee5b1c4c3f533efc764dfd1b34a36985d9e Mon Sep 17 00:00:00 2001 From: Sainath Grandhi Date: Mon, 2 Mar 2020 06:14:19 -0800 Subject: [PATCH] hv: Variable/macro renaming for intr handling of PT devices using IO-APIC/PIC 1. Renames DEFINE_IOAPIC_SID with DEFINE_INTX_SID as the virtual source can be IOAPIC or PIC 2. Rename the src member of source_id.intx_id to ctlr to indicate interrupt controller 2. Changes the type of src member of source_id.intx_id from uint32_t to enum with INTX_CTLR_IOAPIC and INTX_CTLR_PIC Tracked-On: #4447 Signed-off-by: Sainath Grandhi --- hypervisor/arch/x86/guest/assign.c | 52 +++++++++++----------- hypervisor/debug/shell.c | 2 +- hypervisor/dm/vioapic.c | 4 +- hypervisor/dm/vpic.c | 4 +- hypervisor/include/arch/x86/guest/assign.h | 8 ++-- hypervisor/include/common/ptdev.h | 20 ++++++--- 6 files changed, 49 insertions(+), 41 deletions(-) diff --git a/hypervisor/arch/x86/guest/assign.c b/hypervisor/arch/x86/guest/assign.c index f672c0b2c..42e6bde59 100644 --- a/hypervisor/arch/x86/guest/assign.c +++ b/hypervisor/arch/x86/guest/assign.c @@ -177,7 +177,7 @@ ptirq_build_physical_rte(struct acrn_vm *vm, struct ptirq_remapping_info *entry) struct intr_source intr_src; int32_t ret; - if (virt_sid->intx_id.src == PTDEV_VPIN_IOAPIC) { + if (virt_sid->intx_id.ctlr == INTX_CTLR_IOAPIC) { uint64_t vdmask, pdmask; uint32_t dest, delmode, dest_mask, vector; union ioapic_rte virt_rte; @@ -360,9 +360,9 @@ static struct ptirq_remapping_info *add_intx_remapping(struct acrn_vm *vm, uint3 { struct ptirq_remapping_info *entry = NULL; bool entry_is_updated = true; - uint32_t vpin_src = pic_pin ? PTDEV_VPIN_PIC : PTDEV_VPIN_IOAPIC; - DEFINE_IOAPIC_SID(phys_sid, phys_pin, 0U); - DEFINE_IOAPIC_SID(virt_sid, virt_pin, vpin_src); + enum intx_ctlr vpin_ctlr = pic_pin ? INTX_CTLR_PIC : INTX_CTLR_IOAPIC; + DEFINE_INTX_SID(phys_sid, phys_pin, INTX_CTLR_IOAPIC); + DEFINE_INTX_SID(virt_sid, virt_pin, vpin_ctlr); uint32_t phys_irq = ioapic_pin_to_irq(phys_pin); if (((!pic_pin) && (virt_pin >= vioapic_pincount(vm))) || (pic_pin && (virt_pin >= vpic_pincount()))) { @@ -463,13 +463,13 @@ static void ptirq_handle_intx(struct acrn_vm *vm, const struct ptirq_remapping_info *entry) { const union source_id *virt_sid = &entry->virt_sid; - switch (virt_sid->intx_id.src) { - case PTDEV_VPIN_IOAPIC: + switch (virt_sid->intx_id.ctlr) { + case INTX_CTLR_IOAPIC: { union ioapic_rte rte; bool trigger_lvl = false; - /* VPIN_IOAPIC src means we have vioapic enabled */ + /* INTX_CTLR_IOAPIC means we have vioapic enabled */ vioapic_get_rte(vm, (uint32_t)virt_sid->intx_id.pin, &rte); if (rte.bits.trigger_mode == IOAPIC_RTE_TRGRMODE_LEVEL) { trigger_lvl = true; @@ -496,11 +496,11 @@ static void ptirq_handle_intx(struct acrn_vm *vm, rte.full); break; } - case PTDEV_VPIN_PIC: + case INTX_CTLR_PIC: { enum vpic_trigger trigger; - /* VPIN_PIC src means we have vpic enabled */ + /* INTX_CTLR_PIC means we have vpic enabled */ vpic_get_irqline_trigger_mode(vm_pic(vm), virt_sid->intx_id.pin, &trigger); if (trigger == LEVEL_TRIGGER) { vpic_set_irqline(vm_pic(vm), virt_sid->intx_id.pin, GSI_SET_HIGH); @@ -511,8 +511,8 @@ static void ptirq_handle_intx(struct acrn_vm *vm, } default: /* - * In this switch statement, virt_sid->intx_id.src shall - * either be PTDEV_VPIN_IOAPIC or PTDEV_VPIN_PIC. + * In this switch statement, virt_sid->intx_id.ctlr shall + * either be INTX_CTLR_IOAPIC or INTX_CTLR_PIC. * Gracefully return if prior case clauses have not been met. */ break; @@ -556,11 +556,11 @@ void ptirq_softirq(uint16_t pcpu_id) } } -void ptirq_intx_ack(struct acrn_vm *vm, uint32_t virt_pin, uint32_t vpin_src) +void ptirq_intx_ack(struct acrn_vm *vm, uint32_t virt_pin, enum intx_ctlr vpin_ctlr) { uint32_t phys_irq; struct ptirq_remapping_info *entry; - bool pic_pin = (vpin_src == PTDEV_VPIN_PIC); + bool pic_pin = (vpin_ctlr == INTX_CTLR_PIC); entry = ptirq_lookup_entry_by_vpin(vm, virt_pin, pic_pin); if (entry != NULL) { @@ -569,21 +569,21 @@ void ptirq_intx_ack(struct acrn_vm *vm, uint32_t virt_pin, uint32_t vpin_src) /* NOTE: only Level trigger will process EOI/ACK and if we got here * means we have this vioapic or vpic or both enabled */ - switch (vpin_src) { - case PTDEV_VPIN_IOAPIC: + switch (vpin_ctlr) { + case INTX_CTLR_IOAPIC: if (entry->polarity != 0U) { vioapic_set_irqline_lock(vm, virt_pin, GSI_SET_HIGH); } else { vioapic_set_irqline_lock(vm, virt_pin, GSI_SET_LOW); } break; - case PTDEV_VPIN_PIC: + case INTX_CTLR_PIC: vpic_set_irqline(vm_pic(vm), virt_pin, GSI_SET_LOW); break; default: /* - * In this switch statement, vpin_src shall either be - * PTDEV_VPIN_IOAPIC or PTDEV_VPIN_PIC. + * In this switch statement, vpin_ctlr shall either be + * INTX_CTLR_IOAPIC or INTX_CTLR_PIC. * Gracefully return if prior case clauses have not been met. */ break; @@ -701,13 +701,13 @@ static void activate_physical_ioapic(struct acrn_vm *vm, /* Main entry for PCI/Legacy device assignment with INTx, calling from vIOAPIC * or vPIC */ -int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint32_t virt_pin, uint32_t vpin_src) +int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint32_t virt_pin, enum intx_ctlr vpin_ctlr) { int32_t status = 0; struct ptirq_remapping_info *entry = NULL; - bool need_switch_vpin_src = false; - DEFINE_IOAPIC_SID(virt_sid, virt_pin, vpin_src); - bool pic_pin = (vpin_src == PTDEV_VPIN_PIC); + bool need_switch_vpin_ctlr = false; + DEFINE_INTX_SID(virt_sid, virt_pin, vpin_ctlr); + bool pic_pin = (vpin_ctlr == INTX_CTLR_PIC); /* * virt pin could come from vpic master, vpic slave or vioapic @@ -746,7 +746,7 @@ int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint32_t virt_pin, uint32_t vpi entry = ptirq_lookup_entry_by_vpin(vm, vpin, !pic_pin); if (entry != NULL) { - need_switch_vpin_src = true; + need_switch_vpin_ctlr = true; } } @@ -779,13 +779,13 @@ int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint32_t virt_pin, uint32_t vpi if (status == 0) { spinlock_obtain(&ptdev_lock); /* if vpin source need switch */ - if ((need_switch_vpin_src) && (entry != NULL)) { + if ((need_switch_vpin_ctlr) && (entry != NULL)) { dev_dbg(DBG_LEVEL_IRQ, "IOAPIC pin=%hhu pirq=%u vpin=%d switch from %s to %s vpin=%d for vm%d", entry->phys_sid.intx_id.pin, entry->allocated_pirq, entry->virt_sid.intx_id.pin, - (vpin_src == 0U) ? "vPIC" : "vIOAPIC", - (vpin_src == 0U) ? "vIOPIC" : "vPIC", + (vpin_ctlr == INTX_CTLR_IOAPIC) ? "vPIC" : "vIOAPIC", + (vpin_ctlr == INTX_CTLR_IOAPIC) ? "vIOPIC" : "vPIC", virt_pin, entry->vm->vm_id); entry->virt_sid.value = virt_sid.value; } diff --git a/hypervisor/debug/shell.c b/hypervisor/debug/shell.c index f769e265d..d5d340a44 100644 --- a/hypervisor/debug/shell.c +++ b/hypervisor/debug/shell.c @@ -1073,7 +1073,7 @@ static void get_entry_info(const struct ptirq_remapping_info *entry, char *type, uint32_t phys_irq = entry->allocated_pirq; union ioapic_rte rte; - if (entry->virt_sid.intx_id.src == PTDEV_VPIN_IOAPIC) { + if (entry->virt_sid.intx_id.ctlr == INTX_CTLR_IOAPIC) { (void)strncpy_s(type, 16U, "IOAPIC", 16U); } else { (void)strncpy_s(type, 16U, "PIC", 16U); diff --git a/hypervisor/dm/vioapic.c b/hypervisor/dm/vioapic.c index 2df15efee..fcb2bde7a 100644 --- a/hypervisor/dm/vioapic.c +++ b/hypervisor/dm/vioapic.c @@ -333,7 +333,7 @@ static void vioapic_indirect_write(struct acrn_vioapic *vioapic, uint32_t addr, if ((new.bits.intr_mask == IOAPIC_RTE_MASK_CLR) || (last.bits.intr_mask == IOAPIC_RTE_MASK_CLR)) { /* VM enable intr */ /* NOTE: only support max 256 pin */ - (void)ptirq_intx_pin_remap(vioapic->vm, pin, PTDEV_VPIN_IOAPIC); + (void)ptirq_intx_pin_remap(vioapic->vm, pin, INTX_CTLR_IOAPIC); } /* @@ -420,7 +420,7 @@ vioapic_process_eoi(struct acrn_vm *vm, uint32_t vector) continue; } - ptirq_intx_ack(vm, pin, PTDEV_VPIN_IOAPIC); + ptirq_intx_ack(vm, pin, INTX_CTLR_IOAPIC); } /* diff --git a/hypervisor/dm/vpic.c b/hypervisor/dm/vpic.c index 46a35a04b..b5f3af218 100644 --- a/hypervisor/dm/vpic.c +++ b/hypervisor/dm/vpic.c @@ -342,7 +342,7 @@ static int32_t vpic_ocw1(const struct acrn_vpic *vpic, struct i8259_reg_state *i virt_pin = (master_pic(vpic, i8259)) ? pin : (pin + 8U); - (void)ptirq_intx_pin_remap(vpic->vm, virt_pin, PTDEV_VPIN_PIC); + (void)ptirq_intx_pin_remap(vpic->vm, virt_pin, INTX_CTLR_PIC); } pin = (pin + 1U) & 0x7U; } @@ -379,7 +379,7 @@ static int32_t vpic_ocw2(const struct acrn_vpic *vpic, struct i8259_reg_state *i /* if level ack PTDEV */ if ((i8259->elc & (1U << (isr_bit & 0x7U))) != 0U) { ptirq_intx_ack(vpic->vm, (master_pic(vpic, i8259) ? isr_bit : isr_bit + 8U), - PTDEV_VPIN_PIC); + INTX_CTLR_PIC); } } else if (((val & OCW2_SL) != 0U) && i8259->rotate) { /* specific priority */ diff --git a/hypervisor/include/arch/x86/guest/assign.h b/hypervisor/include/arch/x86/guest/assign.h index 2b6f0e6eb..e965dd2d7 100644 --- a/hypervisor/include/arch/x86/guest/assign.h +++ b/hypervisor/include/arch/x86/guest/assign.h @@ -30,14 +30,14 @@ * * @param[in] vm pointer to acrn_vm * @param[in] virt_pin virtual pin number associated with the passthrough device - * @param[in] vpin_src ioapic or pic + * @param[in] vpin_ctlr INTX_CTLR_IOAPIC or INTX_CTLR_PIC * * @return None * * @pre vm != NULL * */ -void ptirq_intx_ack(struct acrn_vm *vm, uint32_t virt_pin, uint32_t vpin_src); +void ptirq_intx_ack(struct acrn_vm *vm, uint32_t virt_pin, enum intx_ctlr vpin_ctlr); /** * @brief MSI/MSI-x remapping for passthrough device. @@ -73,7 +73,7 @@ int32_t ptirq_prepare_msix_remap(struct acrn_vm *vm, uint16_t virt_bdf, uint16_ * * @param[in] vm pointer to acrn_vm * @param[in] virt_pin virtual pin number associated with the passthrough device - * @param[in] vpin_src ioapic or pic + * @param[in] vpin_ctlr INTX_CTLR_IOAPIC or INTX_CTLR_PIC * * @return * - 0: on success @@ -84,7 +84,7 @@ int32_t ptirq_prepare_msix_remap(struct acrn_vm *vm, uint16_t virt_bdf, uint16_ * @pre vm != NULL * */ -int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint32_t virt_pin, uint32_t vpin_src); +int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint32_t virt_pin, enum intx_ctlr vpin_ctlr); /** * @brief Add an interrupt remapping entry for INTx as pre-hold mapping. diff --git a/hypervisor/include/common/ptdev.h b/hypervisor/include/common/ptdev.h index 18ade97b4..158316c1d 100644 --- a/hypervisor/include/common/ptdev.h +++ b/hypervisor/include/common/ptdev.h @@ -10,19 +10,22 @@ #include #include + +enum intx_ctlr { + INTX_CTLR_IOAPIC = 0U, + INTX_CTLR_PIC +}; + #define PTDEV_INTR_MSI (1U << 0U) #define PTDEV_INTR_INTX (1U << 1U) #define INVALID_PTDEV_ENTRY_ID 0xffffU -#define PTDEV_VPIN_IOAPIC 0x0U -#define PTDEV_VPIN_PIC 0x1U - #define DEFINE_MSI_SID(name, a, b) \ union source_id (name) = {.msi_id = {.bdf = (a), .entry_nr = (b)} } -#define DEFINE_IOAPIC_SID(name, a, b) \ -union source_id (name) = {.intx_id = {.pin = (a), .src = (b)} } +#define DEFINE_INTX_SID(name, a, b) \ +union source_id (name) = {.intx_id = {.pin = (a), .ctlr = (b)} } union irte_index { uint16_t index; @@ -32,6 +35,7 @@ union irte_index { } bits __packed; }; + union source_id { uint64_t value; struct { @@ -39,9 +43,13 @@ union source_id { uint16_t entry_nr; uint32_t reserved; } msi_id; + /* + * ctlr indicates if the source of interrupt is IO-APIC or PIC + * pin indicates the pin number of interrupt controller determined by ctlr + */ struct { + enum intx_ctlr ctlr; uint32_t pin; - uint32_t src; } intx_id; };