[PATCH] ptirq: Fix ptirq hash tables and uos intx assignment


Qiang Zhang
 

ptirq_remapping_info records which pGSI is mapped to the vGSI in a VM.
As we need to knonw whether a pGSI has been assigned to a VM and
whether a vGSI in a VM has been used, there should be two hash
tables to link and iterate ptirq_remapping_info:
- One is used to lookup from pGSI, linking phys_link.
- The other is used to lookup from vGSI in a VM, linking virt_link

When assigning a pGSI to a Post-launched VM, if the pGSI has been
assigned to ServiceVM, we should remove that mapping first to reset
ioapic pin state and rte, and build new mapping for the Post-launched
VM.

Signed-off-by: Qiang Zhang <qiang4.zhang@...>
---
hypervisor/arch/x86/guest/assign.c | 86 ++++++++++++++++++------------
hypervisor/common/ptdev.c | 54 +++++++++++--------
hypervisor/dm/io_req.c | 1 +
3 files changed, 85 insertions(+), 56 deletions(-)

diff --git a/hypervisor/arch/x86/guest/assign.c b/hypervisor/arch/x86/guest/assign.c
index a49b3314b..db33c7f91 100644
--- a/hypervisor/arch/x86/guest/assign.c
+++ b/hypervisor/arch/x86/guest/assign.c
@@ -350,6 +350,38 @@ remove_msix_remapping(const struct acrn_vm *vm, uint16_t phys_bdf, uint32_t entr

}

+/* deactivate & remove mapping entry of vpin for vm */
+static void remove_intx_remapping(const struct acrn_vm *vm, uint32_t virt_gsi, enum intx_ctlr vgsi_ctlr)
+{
+ uint32_t phys_irq;
+ struct ptirq_remapping_info *entry;
+ struct intr_source intr_src;
+ DEFINE_INTX_SID(virt_sid, virt_gsi, vgsi_ctlr);
+
+ entry = find_ptirq_entry(PTDEV_INTR_INTX, &virt_sid, vm);
+ if (entry != NULL) {
+ if (is_entry_active(entry)) {
+ phys_irq = entry->allocated_pirq;
+ /* disable interrupt */
+ ioapic_gsi_mask_irq(phys_irq);
+
+ ptirq_deactivate_entry(entry);
+ intr_src.is_msi = false;
+ intr_src.src.ioapic_id = ioapic_irq_to_ioapic_id(phys_irq);
+
+ dmar_free_irte(&intr_src, entry->irte_idx);
+ dev_dbg(DBG_LEVEL_IRQ,
+ "deactivate %s intx entry:pgsi=%d, pirq=%d ",
+ (vgsi_ctlr == INTX_CTLR_PIC) ? "vPIC" : "vIOAPIC",
+ entry->phys_sid.intx_id.gsi, phys_irq);
+ dev_dbg(DBG_LEVEL_IRQ, "from vm%d vgsi=%d\n",
+ entry->vm->vm_id, virt_gsi);
+ }
+
+ ptirq_release_entry(entry);
+ }
+}
+
/* add intx entry for a vm, based on intx id (phys_pin)
* - if the entry not be added by any vm, allocate it
* - if the entry already be added by Service VM, then change the owner to current vm
@@ -383,9 +415,25 @@ static struct ptirq_remapping_info *add_intx_remapping(struct acrn_vm *vm, uint3
}
} else if (entry->vm != vm) {
if (is_service_vm(entry->vm)) {
- entry->vm = vm;
- entry->virt_sid.value = virt_sid.value;
- entry->polarity = 0U;
+ remove_intx_remapping(entry->vm, entry->virt_sid.intx_id.gsi, entry->virt_sid.intx_id.ctlr);
+
+ if (find_ptirq_entry(PTDEV_INTR_INTX, &virt_sid, vm) == NULL) {
+ entry = ptirq_alloc_entry(vm, PTDEV_INTR_INTX);
+ if (entry != NULL) {
+ entry->phys_sid.value = phys_sid.value;
+ entry->virt_sid.value = virt_sid.value;
+ entry->release_cb = ptirq_free_irte;
+
+ /* activate entry */
+ if (ptirq_activate_entry(entry, phys_irq) < 0) {
+ ptirq_release_entry(entry);
+ entry = NULL;
+ }
+ }
+ } else {
+ pr_err("INTX re-add vpin %d", virt_gsi);
+ }
+
} else {
pr_err("INTX gsi%d already in vm%d with vgsi%d, not able to add into vm%d with vgsi%d",
phys_gsi, entry->vm->vm_id, entry->virt_sid.intx_id.gsi, vm->vm_id, virt_gsi);
@@ -410,38 +458,6 @@ static struct ptirq_remapping_info *add_intx_remapping(struct acrn_vm *vm, uint3
return entry;
}

-/* deactive & remove mapping entry of vpin for vm */
-static void remove_intx_remapping(const struct acrn_vm *vm, uint32_t virt_gsi, enum intx_ctlr vgsi_ctlr)
-{
- uint32_t phys_irq;
- struct ptirq_remapping_info *entry;
- struct intr_source intr_src;
- DEFINE_INTX_SID(virt_sid, virt_gsi, vgsi_ctlr);
-
- entry = find_ptirq_entry(PTDEV_INTR_INTX, &virt_sid, vm);
- if (entry != NULL) {
- if (is_entry_active(entry)) {
- phys_irq = entry->allocated_pirq;
- /* disable interrupt */
- ioapic_gsi_mask_irq(phys_irq);
-
- ptirq_deactivate_entry(entry);
- intr_src.is_msi = false;
- intr_src.src.ioapic_id = ioapic_irq_to_ioapic_id(phys_irq);
-
- dmar_free_irte(&intr_src, entry->irte_idx);
- dev_dbg(DBG_LEVEL_IRQ,
- "deactive %s intx entry:pgsi=%d, pirq=%d ",
- (vgsi_ctlr == INTX_CTLR_PIC) ? "vPIC" : "vIOAPIC",
- entry->phys_sid.intx_id.gsi, phys_irq);
- dev_dbg(DBG_LEVEL_IRQ, "from vm%d vgsi=%d\n",
- entry->vm->vm_id, virt_gsi);
- }
-
- ptirq_release_entry(entry);
- }
-}
-
static void ptirq_handle_intx(struct acrn_vm *vm,
const struct ptirq_remapping_info *entry)
{
diff --git a/hypervisor/common/ptdev.c b/hypervisor/common/ptdev.c
index e7e49934a..c980d8865 100644
--- a/hypervisor/common/ptdev.c
+++ b/hypervisor/common/ptdev.c
@@ -22,9 +22,14 @@ struct ptirq_remapping_info ptirq_entries[CONFIG_MAX_PT_IRQ_ENTRIES];
static uint64_t ptirq_entry_bitmaps[PTIRQ_BITMAP_ARRAY_SIZE];
spinlock_t ptdev_lock = { .head = 0U, .tail = 0U, };

-static struct ptirq_entry_head {
- struct hlist_head list;
-} ptirq_entry_heads[PTIRQ_ENTRY_HASHSIZE];
+struct ptirq_hash_table {
+ struct hlist_head buckets[PTIRQ_ENTRY_HASHSIZE];
+};
+
+/* lookup mapping info from phyical gsi, hashing from gsi + vm address(0) */
+static struct ptirq_hash_table phy_gsi_htable;
+/* lookup mapping info from vgsi within a vm, hashing from vm address + vgsi */
+static struct ptirq_hash_table vm_vgsi_htable;

static inline uint16_t ptirq_alloc_entry_id(void)
{
@@ -40,32 +45,39 @@ static inline uint16_t ptirq_alloc_entry_id(void)
return (id < CONFIG_MAX_PT_IRQ_ENTRIES) ? id: INVALID_PTDEV_ENTRY_ID;
}

+/* to find ptirq_remapping_info from phy gsi or vgsi in a vm */
struct ptirq_remapping_info *find_ptirq_entry(uint32_t intr_type,
const union source_id *sid, const struct acrn_vm *vm)
{
struct hlist_node *p;
+ struct hlist_head *b;
struct ptirq_remapping_info *n, *entry = NULL;
- uint64_t key = hash64(sid->value, PTIRQ_ENTRY_HASHBITS);
- struct ptirq_entry_head *b = &ptirq_entry_heads[key];
+ uint64_t key = hash64(sid->value + (uint64_t)vm, PTIRQ_ENTRY_HASHBITS);
+
+ if (vm == NULL) {
+ b = &phy_gsi_htable.buckets[key];

- hlist_for_each(p, &b->list) {
- if (vm == NULL) {
+ hlist_for_each(p, b) {
n = hlist_entry(p, struct ptirq_remapping_info, phys_link);
- } else {
- n = hlist_entry(p, struct ptirq_remapping_info, virt_link);
+ if (is_entry_active(n)) {
+ if ((intr_type == n->intr_type) && (sid->value == n->phys_sid.value)) {
+ entry = n;
+ break;
+ }
+ }
}
-
- if (is_entry_active(n)) {
- if ((intr_type == n->intr_type) &&
- ((vm == NULL) ?
- (sid->value == n->phys_sid.value) :
- ((vm == n->vm) && (sid->value == n->virt_sid.value)))) {
- entry = n;
- break;
+ } else {
+ b = &vm_vgsi_htable.buckets[key];
+ hlist_for_each(p, b) {
+ n = hlist_entry(p, struct ptirq_remapping_info, virt_link);
+ if (is_entry_active(n)) {
+ if ((intr_type == n->intr_type) && (sid->value == n->virt_sid.value) && (vm == n->vm)) {
+ entry = n;
+ break;
+ }
}
}
}
-
return entry;
}

@@ -212,9 +224,9 @@ int32_t ptirq_activate_entry(struct ptirq_remapping_info *entry, uint32_t phys_i
entry->active = true;

key = hash64(entry->phys_sid.value, PTIRQ_ENTRY_HASHBITS);
- hlist_add_head(&entry->phys_link, &(ptirq_entry_heads[key].list));
- key = hash64(entry->virt_sid.value, PTIRQ_ENTRY_HASHBITS);
- hlist_add_head(&entry->virt_link, &(ptirq_entry_heads[key].list));
+ hlist_add_head(&entry->phys_link, &(phy_gsi_htable.buckets[key]));
+ key = hash64(entry->virt_sid.value + (uint64_t)entry->vm, PTIRQ_ENTRY_HASHBITS);
+ hlist_add_head(&entry->virt_link, &(vm_vgsi_htable.buckets[key]));
}

return ret;
diff --git a/hypervisor/dm/io_req.c b/hypervisor/dm/io_req.c
index a1f719153..4fff882ca 100644
--- a/hypervisor/dm/io_req.c
+++ b/hypervisor/dm/io_req.c
@@ -503,6 +503,7 @@ static int32_t mmio_default_access_handler(struct io_request *io_req,
{
struct acrn_mmio_request *mmio = &io_req->reqs.mmio_request;

+ /* unintended accesses from PreVM/SOS shouldn't be silently ignored */
if (mmio->direction == ACRN_IOREQ_DIR_READ) {
switch (mmio->size) {
case 1U:
--
2.30.2