From f67ac091416de02cdb55ee77e4942e1b2c7ef572 Mon Sep 17 00:00:00 2001 From: Sainath Grandhi Date: Mon, 24 Feb 2020 17:02:41 -0800 Subject: [PATCH] hv: Handle holes in GSI i.e. Global System Interrupt for multiple IO-APICs MADT is used to specify the GSI base for each IO-APIC and the number of interrupt pins per IO-APIC is programmed into Max. Redir. Entry register of that IO-APIC. On platforms with multiple IO-APICs, there can be holes in the GSI space. For example, on a platform with 2 IO-APICs, the following configuration has a hole (from 24 to 31) in the GSI space. IO-APIC 1: GSI base - 0, number of pins - 24 IO-APIC 2: GSI base - 32, number of pins - 8 This patch also adjusts the size for variables used to represent the total number of IO-APICs on the system from uint16_t to uint8_t as the ACPI MADT uses only 8-bits to indicate the unique IO-APIC IDs. Tracked-On: #4151 Signed-off-by: Sainath Grandhi Acked-by: Eddie Dong --- hypervisor/arch/x86/ioapic.c | 189 ++++++++++++--------------- hypervisor/arch/x86/irq.c | 6 +- hypervisor/boot/acpi_base.c | 8 +- hypervisor/boot/include/acpi.h | 2 +- hypervisor/common/hypercall.c | 2 +- hypervisor/debug/shell.c | 25 ++-- hypervisor/include/arch/x86/ioapic.h | 27 ++-- 7 files changed, 124 insertions(+), 135 deletions(-) diff --git a/hypervisor/arch/x86/ioapic.c b/hypervisor/arch/x86/ioapic.c index 3219bf06e..f8cbd59ab 100644 --- a/hypervisor/arch/x86/ioapic.c +++ b/hypervisor/arch/x86/ioapic.c @@ -15,43 +15,18 @@ #include #include -#define IOAPIC_INVALID_ID 0xFFU - #define NR_MAX_GSI (CONFIG_MAX_IOAPIC_NUM * CONFIG_MAX_IOAPIC_LINES) static struct gsi_table gsi_table_data[NR_MAX_GSI]; -static uint32_t ioapic_nr_gsi; +static uint32_t ioapic_max_nr_gsi; static spinlock_t ioapic_lock; static union ioapic_rte saved_rte[CONFIG_MAX_IOAPIC_NUM][CONFIG_MAX_IOAPIC_LINES]; -/* - * the irq to ioapic pin mapping should extract from ACPI MADT table - * hardcoded here - */ -static const uint32_t legacy_irq_to_pin[NR_LEGACY_IRQ] = { - 2U, /* IRQ0*/ - 1U, /* IRQ1*/ - 0U, /* IRQ2 connected to Pin0 (ExtInt source of PIC) if existing */ - 3U, /* IRQ3*/ - 4U, /* IRQ4*/ - 5U, /* IRQ5*/ - 6U, /* IRQ6*/ - 7U, /* IRQ7*/ - 8U, /* IRQ8*/ - 9U, /* IRQ9*/ - 10U, /* IRQ10*/ - 11U, /* IRQ11*/ - 12U, /* IRQ12*/ - 13U, /* IRQ13*/ - 14U, /* IRQ14*/ - 15U, /* IRQ15*/ -}; - -static const uint32_t legacy_irq_trigger_mode[NR_LEGACY_IRQ] = { - IOAPIC_RTE_TRGRMODE_EDGE, /* IRQ0*/ - IOAPIC_RTE_TRGRMODE_EDGE, /* IRQ1*/ +static const uint32_t legacy_irq_trigger_mode[NR_LEGACY_PIN] = { IOAPIC_RTE_TRGRMODE_EDGE, /* IRQ2*/ + IOAPIC_RTE_TRGRMODE_EDGE, /* IRQ1*/ + IOAPIC_RTE_TRGRMODE_EDGE, /* IRQ0*/ IOAPIC_RTE_TRGRMODE_EDGE, /* IRQ3*/ IOAPIC_RTE_TRGRMODE_EDGE, /* IRQ4*/ IOAPIC_RTE_TRGRMODE_EDGE, /* IRQ5*/ @@ -87,7 +62,7 @@ static const uint32_t pic_ioapic_pin_map[NR_LEGACY_PIN] = { }; static struct ioapic_info ioapic_array[CONFIG_MAX_IOAPIC_NUM]; -static uint16_t ioapic_num; +static uint8_t ioapic_num; uint32_t get_pic_pin_from_ioapic_pin(uint32_t pin_index) { @@ -104,12 +79,12 @@ uint32_t get_pic_pin_from_ioapic_pin(uint32_t pin_index) void *gsi_to_ioapic_base(uint32_t gsi) { - return gsi_table_data[gsi].addr; + return gsi_table_data[gsi].ioapic_info.base_addr; } -uint32_t ioapic_get_nr_gsi(void) +uint32_t get_max_nr_gsi(void) { - return ioapic_nr_gsi; + return ioapic_max_nr_gsi; } static void *map_ioapic(uint64_t ioapic_paddr) @@ -211,7 +186,7 @@ create_rte_for_gsi_irq(uint32_t irq, uint32_t vr) rte.full = 0UL; - if (irq < NR_LEGACY_IRQ) { + if (irq < NR_LEGACY_PIN) { rte = create_rte_for_legacy_irq(irq, vr); } else { /* irq default masked, level trig */ @@ -238,7 +213,7 @@ static void ioapic_set_routing(uint32_t gsi, uint32_t vr) ioapic_base = gsi_to_ioapic_base(gsi); rte = create_rte_for_gsi_irq(gsi, vr); - ioapic_set_rte_entry(ioapic_base, gsi_table_data[gsi].pin, rte); + ioapic_set_rte_entry(ioapic_base, gsi_table_data[gsi].ioapic_info.pin, rte); if (rte.bits.trigger_mode == IOAPIC_RTE_TRGRMODE_LEVEL) { set_irq_trigger_mode(gsi, true); @@ -247,62 +222,71 @@ static void ioapic_set_routing(uint32_t gsi, uint32_t vr) } dev_dbg(DBG_LEVEL_IRQ, "GSI: irq:%d pin:%hhu rte:%lx", - gsi, gsi_table_data[gsi].pin, + gsi, gsi_table_data[gsi].ioapic_info.pin, rte.full); } -/** +/* * @pre rte != NULL + * @pre is_ioapic_irq(irq) == true */ void ioapic_get_rte(uint32_t irq, union ioapic_rte *rte) { void *addr; - if (ioapic_irq_is_gsi(irq)) { - addr = gsi_to_ioapic_base(irq); - ioapic_get_rte_entry(addr, gsi_table_data[irq].pin, rte); - } + addr = gsi_to_ioapic_base(irq); + ioapic_get_rte_entry(addr, gsi_table_data[irq].ioapic_info.pin, rte); } +/* + * @pre is_ioapic_irq(irq) == true + */ void ioapic_set_rte(uint32_t irq, union ioapic_rte rte) { void *addr; - if (ioapic_irq_is_gsi(irq)) { - addr = gsi_to_ioapic_base(irq); - ioapic_set_rte_entry(addr, gsi_table_data[irq].pin, rte); + addr = gsi_to_ioapic_base(irq); + ioapic_set_rte_entry(addr, gsi_table_data[irq].ioapic_info.pin, rte); - dev_dbg(DBG_LEVEL_IRQ, "GSI: irq:%d pin:%hhu rte:%lx", - irq, gsi_table_data[irq].pin, - rte.full); - } -} - -bool ioapic_irq_is_gsi(uint32_t irq) -{ - return irq < ioapic_nr_gsi; -} - -uint32_t ioapic_irq_to_pin(uint32_t irq) -{ - uint32_t ret; - - if (ioapic_irq_is_gsi(irq)) { - ret = gsi_table_data[irq].pin; - } else { - ret = INVALID_INTERRUPT_PIN; - } - - return ret; -} - -bool ioapic_is_pin_valid(uint32_t pin) -{ - return (pin != INVALID_INTERRUPT_PIN); + dev_dbg(DBG_LEVEL_IRQ, "GSI: irq:%d pin:%hhu rte:%lx", + irq, gsi_table_data[irq].ioapic_info.pin, + rte.full); } /* - *@pre ioapic_irq_is_gsi(gsi) == true + * Checks if the gsi is valid + * 1) gsi < NR_MAX_GSI + * 2) gsi is valid on the platform according to ACPI MADT info + */ +bool is_gsi_valid(uint32_t gsi) +{ + + return (gsi < NR_MAX_GSI) && (gsi_table_data[gsi].is_valid); +} + +/* + * IO-APIC gsi and irq are identity mapped in ioapic_setup_irqs + * So #gsi = #irq for ACRN + */ + +bool is_ioapic_irq(uint32_t irq) +{ + + return is_gsi_valid(irq); +} + +/* + *@pre gsi < NR_MAX_GSI + *@pre is_gsi_valid(gsi) == true + */ + +uint32_t gsi_to_ioapic_pin(uint32_t gsi) +{ + return gsi_table_data[gsi].ioapic_info.pin; +} + +/* + *@pre is_gsi_valid(gsi) == true */ uint32_t ioapic_gsi_to_irq(uint32_t gsi) { @@ -316,23 +300,21 @@ ioapic_irq_gsi_mask_unmask(uint32_t irq, bool mask) uint32_t pin; union ioapic_rte rte; - if (ioapic_irq_is_gsi(irq)) { - addr = gsi_to_ioapic_base(irq); - pin = gsi_table_data[irq].pin; + addr = gsi_to_ioapic_base(irq); + pin = gsi_table_data[irq].ioapic_info.pin; - if (addr != NULL) { - ioapic_get_rte_entry(addr, pin, &rte); - if (mask) { - rte.bits.intr_mask = IOAPIC_RTE_MASK_SET; - } else { - rte.bits.intr_mask = IOAPIC_RTE_MASK_CLR; - } - ioapic_set_rte_entry(addr, pin, rte); - dev_dbg(DBG_LEVEL_PTIRQ, "update: irq:%d pin:%hhu rte:%lx", - irq, pin, rte.full); + if (addr != NULL) { + ioapic_get_rte_entry(addr, pin, &rte); + if (mask) { + rte.bits.intr_mask = IOAPIC_RTE_MASK_SET; } else { - dev_dbg(DBG_LEVEL_PTIRQ, "NULL Address returned from gsi_table_data"); + rte.bits.intr_mask = IOAPIC_RTE_MASK_CLR; } + ioapic_set_rte_entry(addr, pin, rte); + dev_dbg(DBG_LEVEL_PTIRQ, "update: irq:%d pin:%hhu rte:%lx", + irq, pin, rte.full); + } else { + dev_dbg(DBG_LEVEL_PTIRQ, "NULL Address returned from gsi_table_data"); } } @@ -364,17 +346,13 @@ ioapic_nr_pins(void *ioapic_base) return nr_pins; } +/* + * @pre is_ioapic_irq(irq) == true + */ uint8_t ioapic_irq_to_ioapic_id(uint32_t irq) { - uint8_t ret; - if (ioapic_irq_is_gsi(irq)) { - ret = gsi_table_data[irq].ioapic_id; - } else { - ret = IOAPIC_INVALID_ID; - } - - return ret; + return gsi_table_data[irq].ioapic_info.acpi_id; } int32_t init_ioapic_id_info(void) @@ -385,7 +363,7 @@ int32_t init_ioapic_id_info(void) uint32_t nr_pins, gsi; ioapic_num = parse_madt_ioapic(&ioapic_array[0]); - if (ioapic_num <= (uint16_t)CONFIG_MAX_IOAPIC_NUM) { + if (ioapic_num <= (uint8_t)CONFIG_MAX_IOAPIC_NUM) { /* * Iterate thru all the IO-APICs on the platform * Check the number of pins available on each IOAPIC is less @@ -415,7 +393,7 @@ int32_t init_ioapic_id_info(void) */ if (ret == 0) { - if (gsi < (uint32_t) NR_LEGACY_IRQ) { + if (gsi < (uint32_t) NR_LEGACY_PIN) { pr_err ("Total pin count (%x) is less than NR_LEGACY_IRQ!", gsi); ret = -EINVAL; } @@ -446,16 +424,13 @@ void ioapic_setup_irqs(void) addr = map_ioapic(ioapic_array[ioapic_id].addr); nr_pins = ioapic_array[ioapic_id].nr_pins; + gsi = ioapic_array[ioapic_id].gsi_base; for (pin = 0U; pin < nr_pins; pin++) { - gsi_table_data[gsi].ioapic_id = ioapic_array[ioapic_id].id; - gsi_table_data[gsi].addr = addr; - - if (gsi < NR_LEGACY_IRQ) { - gsi_table_data[gsi].pin = - legacy_irq_to_pin[gsi] & 0xffU; - } else { - gsi_table_data[gsi].pin = pin; - } + gsi_table_data[gsi].is_valid = true; + gsi_table_data[gsi].ioapic_info.acpi_id = ioapic_array[ioapic_id].id; + gsi_table_data[gsi].ioapic_info.base_addr = addr; + gsi_table_data[gsi].ioapic_info.pin = pin; + gsi_table_data[gsi].ioapic_info.index = ioapic_id; /* pinned irq before use it */ if (alloc_irq_num(gsi) == IRQ_INVALID) { @@ -467,7 +442,7 @@ void ioapic_setup_irqs(void) /* assign vector for this GSI * for legacy irq, reserved vector and never free */ - if (gsi < NR_LEGACY_IRQ) { + if (gsi < NR_LEGACY_PIN) { vr = alloc_irq_vector(gsi); if (vr == VECTOR_INVALID) { pr_err("failed to alloc VR"); @@ -484,7 +459,7 @@ void ioapic_setup_irqs(void) } /* system max gsi numbers */ - ioapic_nr_gsi = gsi; + ioapic_max_nr_gsi = gsi; } void suspend_ioapic(void) diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c index ffc91500b..dfe5b16c8 100644 --- a/hypervisor/arch/x86/irq.c +++ b/hypervisor/arch/x86/irq.c @@ -82,7 +82,7 @@ static void free_irq_num(uint32_t irq) uint64_t rflags; if (irq < NR_IRQS) { - if (!ioapic_irq_is_gsi(irq)) { + if (!is_ioapic_irq(irq)) { spinlock_irqsave_obtain(&irq_alloc_spinlock, &rflags); (void)bitmap_test_and_clear_nolock((uint16_t)(irq & 0x3FU), irq_alloc_bitmap + (irq >> 6U)); @@ -301,7 +301,7 @@ static inline bool irq_need_mask(const struct irq_desc *desc) { /* level triggered gsi should be masked */ return (((desc->flags & IRQF_LEVEL) != 0U) - && ioapic_irq_is_gsi(desc->irq)); + && is_ioapic_irq(desc->irq)); } static inline bool irq_need_unmask(const struct irq_desc *desc) @@ -309,7 +309,7 @@ static inline bool irq_need_unmask(const struct irq_desc *desc) /* level triggered gsi for non-ptdev should be unmasked */ return (((desc->flags & IRQF_LEVEL) != 0U) && ((desc->flags & IRQF_PT) == 0U) - && ioapic_irq_is_gsi(desc->irq)); + && is_ioapic_irq(desc->irq)); } static inline void handle_irq(const struct irq_desc *desc) diff --git a/hypervisor/boot/acpi_base.c b/hypervisor/boot/acpi_base.c index e1b8377e9..f586a2734 100644 --- a/hypervisor/boot/acpi_base.c +++ b/hypervisor/boot/acpi_base.c @@ -209,14 +209,14 @@ local_parse_madt(struct acpi_table_madt *madt, uint32_t lapic_id_array[MAX_PCPU_ return pcpu_num; } -static uint16_t +static uint8_t ioapic_parse_madt(void *madt, struct ioapic_info *ioapic_id_array) { struct acpi_madt_ioapic *ioapic; struct acpi_table_madt *madt_ptr; void *first, *end, *iterator; struct acpi_subtable_header *entry; - uint16_t ioapic_idx = 0U; + uint8_t ioapic_idx = 0U; madt_ptr = (struct acpi_table_madt *)madt; @@ -261,9 +261,9 @@ uint16_t parse_madt(uint32_t lapic_id_array[MAX_PCPU_NUM]) return ret; } -uint16_t parse_madt_ioapic(struct ioapic_info *ioapic_id_array) +uint8_t parse_madt_ioapic(struct ioapic_info *ioapic_id_array) { - uint16_t ret = 0U; + uint8_t ret = 0U; struct acpi_table_rsdp *rsdp = NULL; rsdp = get_rsdp(); diff --git a/hypervisor/boot/include/acpi.h b/hypervisor/boot/include/acpi.h index 8641dcbd1..027827c46 100644 --- a/hypervisor/boot/include/acpi.h +++ b/hypervisor/boot/include/acpi.h @@ -203,7 +203,7 @@ void *get_acpi_tbl(const char *signature); struct ioapic_info; uint16_t parse_madt(uint32_t lapic_id_array[MAX_PCPU_NUM]); -uint16_t parse_madt_ioapic(struct ioapic_info *ioapic_id_array); +uint8_t parse_madt_ioapic(struct ioapic_info *ioapic_id_array); #ifdef CONFIG_ACPI_PARSE_ENABLED int32_t acpi_fixup(void); diff --git a/hypervisor/common/hypercall.c b/hypervisor/common/hypercall.c index d53ec370a..8561087d6 100644 --- a/hypervisor/common/hypercall.c +++ b/hypervisor/common/hypercall.c @@ -908,7 +908,7 @@ int32_t hcall_set_ptdev_intr_info(struct acrn_vm *vm, uint16_t vmid, uint64_t pa if ((vdev != NULL) && (vdev->pdev->bdf.value == irq.phys_bdf)) { if ((((!irq.intx.pic_pin) && (irq.intx.virt_pin < vioapic_pincount(target_vm))) || ((irq.intx.pic_pin) && (irq.intx.virt_pin < vpic_pincount()))) && - ioapic_irq_is_gsi(irq.intx.phys_pin)) { + is_gsi_valid(irq.intx.phys_pin)) { ret = ptirq_add_intx_remapping(target_vm, irq.intx.virt_pin, irq.intx.phys_pin, irq.intx.pic_pin); } else { diff --git a/hypervisor/debug/shell.c b/hypervisor/debug/shell.c index 3cf9132b4..b6055ec63 100644 --- a/hypervisor/debug/shell.c +++ b/hypervisor/debug/shell.c @@ -1253,7 +1253,7 @@ static int32_t shell_show_vioapic_info(int32_t argc, char **argv) static int32_t get_ioapic_info(char *str_arg, size_t str_max_len) { char *str = str_arg; - uint32_t irq; + uint32_t gsi; size_t len, size = str_max_len; uint32_t ioapic_nr_gsi = 0U; @@ -1264,20 +1264,21 @@ static int32_t get_ioapic_info(char *str_arg, size_t str_max_len) size -= len; str += len; - ioapic_nr_gsi = ioapic_get_nr_gsi (); - for (irq = 0U; irq < ioapic_nr_gsi; irq++) { - void *addr = gsi_to_ioapic_base(irq); - uint32_t pin = ioapic_irq_to_pin(irq); + ioapic_nr_gsi = get_max_nr_gsi (); + for (gsi = 0U; gsi < ioapic_nr_gsi; gsi++) { + void *addr; + uint32_t pin; union ioapic_rte rte; - /* Add NULL check for addr, INVALID_PIN check for pin */ - if ((addr == NULL) || (!ioapic_is_pin_valid(pin))) { - goto overflow; + if (!is_gsi_valid(gsi)) { + continue; } + addr = gsi_to_ioapic_base(gsi); + pin = gsi_to_ioapic_pin(gsi); ioapic_get_rte_entry(addr, pin, &rte); - len = snprintf(str, size, "\r\n%03d\t%03hhu\t0x%08X\t0x%08X\t", irq, pin, rte.u.hi_32, rte.u.lo_32); + len = snprintf(str, size, "\r\n%03d\t%03hhu\t0x%08X\t0x%08X\t", gsi, pin, rte.u.hi_32, rte.u.lo_32); if (len >= size) { goto overflow; } @@ -1285,8 +1286,10 @@ static int32_t get_ioapic_info(char *str_arg, size_t str_max_len) str += len; len = snprintf(str, size, "0x%02X\t0x%02X\t%s\t%s\t%u\t%d\t%d", - rte.bits.vector, rte.bits.dest_field, rte.bits.dest_mode ? "logic" : "phys", - rte.bits.trigger_mode ? "level" : "edge", rte.bits.delivery_mode, rte.bits.remote_irr, + rte.bits.vector, rte.bits.dest_field, + (rte.bits.dest_mode == IOAPIC_RTE_DESTMODE_LOGICAL)? "logic" : "phys", + (rte.bits.trigger_mode == IOAPIC_RTE_TRGRMODE_LEVEL)? "level" : "edge", + rte.bits.delivery_mode, rte.bits.remote_irr, rte.bits.intr_mask); if (len >= size) { goto overflow; diff --git a/hypervisor/include/arch/x86/ioapic.h b/hypervisor/include/arch/x86/ioapic.h index 2578f63fe..a1a9c4f9e 100644 --- a/hypervisor/include/arch/x86/ioapic.h +++ b/hypervisor/include/arch/x86/ioapic.h @@ -21,8 +21,8 @@ struct ioapic_info { void ioapic_setup_irqs(void); -bool ioapic_irq_is_gsi(uint32_t irq); -uint32_t ioapic_irq_to_pin(uint32_t irq); +bool is_ioapic_irq(uint32_t irq); +uint32_t gsi_to_ioapic_pin(uint32_t gsi); int32_t init_ioapic_id_info(void); uint8_t ioapic_irq_to_ioapic_id(uint32_t irq); @@ -88,15 +88,26 @@ void ioapic_gsi_unmask_irq(uint32_t irq); void ioapic_get_rte_entry(void *ioapic_base, uint32_t pin, union ioapic_rte *rte); +/* + * is_valid is by default false when all the + * static variables, part of .bss, are initialized to 0s + * It is set to true, if the corresponding + * gsi falls in ranges identified by IOAPIC data + * in ACPI MADT in ioapic_setup_irqs. + */ + struct gsi_table { - uint8_t ioapic_id; - uint32_t pin; - void *addr; + bool is_valid; + struct { + uint8_t acpi_id; + uint8_t index; + uint32_t pin; + void *base_addr; + } ioapic_info; }; void *gsi_to_ioapic_base(uint32_t gsi); -uint32_t ioapic_get_nr_gsi(void); +uint32_t get_max_nr_gsi(void); uint32_t get_pic_pin_from_ioapic_pin(uint32_t pin_index); -bool ioapic_is_pin_valid(uint32_t pin); - +bool is_gsi_valid(uint32_t gsi); #endif /* IOAPIC_H */