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.

Tracked-On: #8370
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

On 2023-02-23 at 15:40:48 +0800, Qiang Zhang wrote:

Hi Qiang

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.

I think you try to fix two issues:
One is there needs two hash list to record the ptirq remapping info,
one for physical ptirq remapping info, the other for virtual ptirq
remapping info;
the secode is 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. Maybe we should do this in the hypercall phase since
pre-launched VM doesn't need this logic.


Thanks.

Tracked-On: #8370
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

On Fri, Feb 24, 2023 at 02:03:50PM +0800, Fei Li wrote:

On 2023-02-23 at 15:40:48 +0800, Qiang Zhang wrote:

Hi Qiang

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.

I think you try to fix two issues:
One is there needs two hash list to record the ptirq remapping info,
one for physical ptirq remapping info, the other for virtual ptirq
remapping info;
the secode is 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. Maybe we should do this in the hypercall phase since
pre-launched VM doesn't need this logic.


Thanks.

I think we shouldn't put too much logic in hypercall.c. Maybe the hypercall
handler needs to call something like `ptirq_reassign_intx_remapping` to
reassign a pGSI to a Post-launched VM if it's already assigned to ServiceVM.
Thereafter we can move the remove logic out of the `add_intx_remapping`.

Tracked-On: #8370
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