From bf9341844a526b74d859b3acd4607502c0200838 Mon Sep 17 00:00:00 2001 From: Qiang Zhang Date: Mon, 27 Feb 2023 15:33:14 +0800 Subject: [PATCH] ptirq: Fix ptirq hash tables ptirq_remapping_info records which physical interrupt is mapped to the virtual interrupt in a VM. As we need to knonw whether a physical sid has been mapped to a VM and whether a virtual sid 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 physical sid, linking phys_link. - The other is used to lookup from virtual sid in a VM, linking virt_link Without this patch, phys_link or virt_link from different ptirq_remapping_info was linked by one hash list head if they got the same hash value, as shown in following diagram. When looking for a ptirq_remapping_info from physical sid, the original code took all hash list node as phys_link and failed to get ptirq_remapping_info linked with virt_link and later references to its members are wrong. The same problem also occurred when looking for a ptirq_remapping_info from virtual sid and vm. ---------- <- hash table |hlist_head| --actual ptirq_remapping_info address ---------- --------- / --used as ptirq_remapping_info | ... | |phys_link| ___/ ---------- --------- --------- / --------- |hlist_head| -> |phys_link| <-> |virt_link| <-> |phys_link| ---------- --------- --------- --------- | ... | |virt_link| | other | |virt_link| ---------- --------- | members | --------- | other | --------- | other | | members | | members | --------- --------- Tracked-On: #8370 Signed-off-by: Qiang Zhang Reviewed-by: Junjie Mao Acked-by: Eddie Dong --- hypervisor/common/ptdev.c | 67 +++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/hypervisor/common/ptdev.c b/hypervisor/common/ptdev.c index e7e49934a..323067ded 100644 --- a/hypervisor/common/ptdev.c +++ b/hypervisor/common/ptdev.c @@ -22,9 +22,10 @@ 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]; +/* lookup mapping info from phyical sid, hashing from sid + acrn_vm structure address (NULL) */ +static struct hlist_head phys_sid_htable[PTIRQ_ENTRY_HASHSIZE]; +/* lookup mapping info from virtual sid within a vm, hashing from sid + acrn_vm structure address */ +static struct hlist_head virt_sid_htable[PTIRQ_ENTRY_HASHSIZE]; static inline uint16_t ptirq_alloc_entry_id(void) { @@ -40,28 +41,52 @@ static inline uint16_t ptirq_alloc_entry_id(void) return (id < CONFIG_MAX_PT_IRQ_ENTRIES) ? id: INVALID_PTDEV_ENTRY_ID; } +/* + * get the hash key when looking up ptirq_remapping_info from virtual + * source id in a VM, or just physical source id (vm == NULL). + * Hashing from source id value and acrn_vm structure address can decrease + * the probability of hash collisions as different VMs may have equal + * virtual source ids. + */ +static inline uint64_t ptirq_hash_key(const struct acrn_vm *vm, + const union source_id *sid) +{ + return hash64(sid->value + (uint64_t)vm, PTIRQ_ENTRY_HASHBITS); +} + +/* + * to find ptirq_remapping_info from phyical source id (vm == NULL) or + * virtual source id 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 = ptirq_hash_key(vm, sid); - hlist_for_each(p, &b->list) { - if (vm == NULL) { + if (vm == NULL) { + b = &(phys_sid_htable[key]); + + 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 = &(virt_sid_htable[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; + } } } } @@ -211,10 +236,10 @@ int32_t ptirq_activate_entry(struct ptirq_remapping_info *entry, uint32_t phys_i entry->allocated_pirq = irq; 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)); + key = ptirq_hash_key(NULL, &(entry->phys_sid)); + hlist_add_head(&entry->phys_link, &(phys_sid_htable[key])); + key = ptirq_hash_key(entry->vm, &(entry->virt_sid)); + hlist_add_head(&entry->virt_link, &(virt_sid_htable[key])); } return ret;