-----Original Message-----
From: Qiang Zhang <qiang4.zhang@...>
Sent: Thursday, March 9, 2023 9:57 AM
To: acrn-dev@...
Cc: Mao, Junjie <junjie.mao@...>; Li, Fei1 <fei1.li@...>
Subject: [PATCH v5 2/2] ptirq: Fix INTx assignment for Post-launched VM

When assigning a physical interrupt to a Post-launched VM, if it 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 | 43 ++++++++++---------
hypervisor/common/hypercall.c | 3 +-
.../include/arch/x86/asm/guest/assign.h | 8 ++--
3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/hypervisor/arch/x86/guest/assign.c b/hypervisor/arch/x86/guest/assign.c
index a49b3314b..f7e688ba1 100644
--- a/hypervisor/arch/x86/guest/assign.c
+++ b/hypervisor/arch/x86/guest/assign.c
@@ -382,15 +382,9 @@ static struct ptirq_remapping_info *add_intx_remapping(struct acrn_vm
*vm, uint3
pr_err("INTX re-add vpin %d", virt_gsi);
}
} else if (entry->vm != vm) {
- if (is_service_vm(entry->vm)) {
- entry->vm = vm;
- entry->virt_sid.value = virt_sid.value;
- entry->polarity = 0U;
- } 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);

- entry = NULL;
- }
+ 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);
+ entry = NULL;
} else {
/* The mapping has already been added to the VM. No action
* required.
@@ -410,15 +404,24 @@ 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)
+/* deactivate & remove mapping entry of vpin for vm */
+static void remove_intx_remapping(const struct acrn_vm *vm, uint32_t gsi, enum intx_ctlr
gsi_ctlr, bool is_phy_gsi)
{
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 (is_phy_gsi) {
+ DEFINE_INTX_SID(sid, gsi, INTX_CTLR_IOAPIC);
+ entry = find_ptirq_entry(PTDEV_INTR_INTX, &sid, 0);
+ if ((entry != NULL) && (entry->vm != vm)) {
+ entry = NULL;
+ }
+ } else {
+ DEFINE_INTX_SID(sid, gsi, gsi_ctlr);
+ entry = find_ptirq_entry(PTDEV_INTR_INTX, &sid, vm);
+ }
+
if (entry != NULL) {
if (is_entry_active(entry)) {
phys_irq = entry->allocated_pirq;
@@ -431,11 +434,11 @@ static void remove_intx_remapping(const struct acrn_vm *vm, uint32_t
virt_gsi, e

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",
+ "deactivate %s intx entry:pgsi=%d, pirq=%d ",
+ (entry->virt_sid.intx_id.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);
+ entry->vm->vm_id, entry->virt_sid.intx_id.gsi);
}

ptirq_release_entry(entry);
@@ -735,7 +738,7 @@ int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint32_t virt_gsi,
enum intx_ct
uint32_t phys_gsi = virt_gsi;

remove_intx_remapping(vm, alt_virt_sid.intx_id.gsi,
- alt_virt_sid.intx_id.ctlr);
+ alt_virt_sid.intx_id.ctlr, false);
entry = add_intx_remapping(vm, virt_gsi, phys_gsi,
vgsi_ctlr);
if (entry == NULL) {
pr_err("%s, add intx remapping failed", __func__);
@@ -806,12 +809,12 @@ int32_t ptirq_add_intx_remapping(struct acrn_vm *vm, uint32_t
virt_gsi, uint32_t
/*
* @pre vm != NULL
*/
-void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t virt_gsi, bool
pic_pin)
+void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t gsi, bool pic_pin,
bool is_phy_gsi)
{
enum intx_ctlr vgsi_ctlr = pic_pin ? INTX_CTLR_PIC : INTX_CTLR_IOAPIC;

spinlock_obtain(&ptdev_lock);
- remove_intx_remapping(vm, virt_gsi, vgsi_ctlr);
+ remove_intx_remapping(vm, gsi, vgsi_ctlr, is_phy_gsi);
spinlock_release(&ptdev_lock);
}

@@ -839,6 +842,6 @@ void ptirq_remove_configured_intx_remappings(const struct acrn_vm *vm)
uint16_t i;

for (i = 0; i < vm_config->pt_intx_num; i++) {
- ptirq_remove_intx_remapping(vm, vm_config->pt_intx[i].virt_gsi, false);
+ ptirq_remove_intx_remapping(vm, vm_config->pt_intx[i].virt_gsi, false, false);
}
}
diff --git a/hypervisor/common/hypercall.c b/hypervisor/common/hypercall.c
index 5269380c1..5e296f710 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -1016,6 +1016,7 @@ int32_t hcall_set_ptdev_intr_info(struct acrn_vcpu *vcpu, struct
acrn_vm *target
if ((((!irq.intx.pic_pin) && (irq.intx.virt_pin <
get_vm_gsicount(target_vm)))
|| ((irq.intx.pic_pin) && (irq.intx.virt_pin <
vpic_pincount())))
&& is_gsi_valid(irq.intx.phys_pin)) {
+ ptirq_remove_intx_remapping(get_service_vm(),
irq.intx.phys_pin, false, true);
ret = ptirq_add_intx_remapping(target_vm,
irq.intx.virt_pin,
irq.intx.phys_pin, irq.intx.pic_pin);
} else {
@@ -1067,7 +1068,7 @@ int32_t hcall_reset_ptdev_intr_info(struct acrn_vcpu *vcpu, struct
acrn_vm *targ
if ((vdev != NULL) && (vdev->pdev->bdf.value == irq.phys_bdf)) {
if (((!irq.intx.pic_pin) && (irq.intx.virt_pin <
get_vm_gsicount(target_vm))) ||
((irq.intx.pic_pin) && (irq.intx.virt_pin <
vpic_pincount()))) {
- ptirq_remove_intx_remapping(target_vm, irq.intx.virt_pin,
irq.intx.pic_pin);
+ ptirq_remove_intx_remapping(target_vm, irq.intx.virt_pin,
irq.intx.pic_pin, false);
ret = 0;
} else {
pr_err("%s: Invalid virt pin\n", __func__);
diff --git a/hypervisor/include/arch/x86/asm/guest/assign.h
b/hypervisor/include/arch/x86/asm/guest/assign.h
index 9b3b666af..e601dcef6 100644
--- a/hypervisor/include/arch/x86/asm/guest/assign.h
+++ b/hypervisor/include/arch/x86/asm/guest/assign.h
@@ -112,18 +112,20 @@ int32_t ptirq_add_intx_remapping(struct acrn_vm *vm, uint32_t
virt_gsi, uint32_t
/**
* @brief Remove an interrupt remapping entry for INTx.
*
- * Deactivate & remove mapping entry of the given virt_pin for given vm.
+ * Deactivate & remove mapping entry of the given virt gsi for given vm or
+ * phys gsi assigned to this vm.
*
* @param[in] vm pointer to acrn_vm
- * @param[in] virt_gsi virtual pin number associated with the passthrough device
+ * @param[in] gsi virtual gsi number or physical gsi number associated with the
passthrough device
* @param[in] pic_pin true for pic, false for ioapic
+ * @param[in] is_phy_gsi true if gsi is physical, false if gsi is virtual
*
* @return None
*
* @pre vm != NULL
*
*/
-void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t virt_gsi, bool
pic_pin);
+void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t gsi, bool pic_pin,
bool is_phy_gsi);

/**
* @brief Remove interrupt remapping entry/entries for MSI/MSI-x.
--
2.30.2

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Qiang Zhang
Sent: Thursday, March 9, 2023 9:57 AM
To: acrn-dev@...
Cc: Mao, Junjie <junjie.mao@...>; Li, Fei1 <fei1.li@...>
Subject: [acrn-dev] [PATCH v5 2/2] ptirq: Fix INTx assignment for Post-
launched VM

When assigning a physical interrupt to a Post-launched VM, if it 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 | 43 ++++++++++---------
hypervisor/common/hypercall.c | 3 +-
.../include/arch/x86/asm/guest/assign.h | 8 ++--
3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/hypervisor/arch/x86/guest/assign.c
b/hypervisor/arch/x86/guest/assign.c
index a49b3314b..f7e688ba1 100644
--- a/hypervisor/arch/x86/guest/assign.c
+++ b/hypervisor/arch/x86/guest/assign.c
@@ -382,15 +382,9 @@ static struct ptirq_remapping_info
*add_intx_remapping(struct acrn_vm *vm, uint3
pr_err("INTX re-add vpin %d", virt_gsi);
}
} else if (entry->vm != vm) {
- if (is_service_vm(entry->vm)) {
- entry->vm = vm;
- entry->virt_sid.value = virt_sid.value;
- entry->polarity = 0U;
- } 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);

- entry = NULL;
- }
+ 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);

+ entry = NULL;
} else {
/* The mapping has already been added to the VM. No
action
* required.
@@ -410,15 +404,24 @@ 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)
+/* deactivate & remove mapping entry of vpin for vm */ static void
+remove_intx_remapping(const struct acrn_vm *vm, uint32_t gsi, enum
+intx_ctlr gsi_ctlr, bool is_phy_gsi)
{
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 (is_phy_gsi) {
+ DEFINE_INTX_SID(sid, gsi, INTX_CTLR_IOAPIC);
+ entry = find_ptirq_entry(PTDEV_INTR_INTX, &sid, 0);

+ if ((entry != NULL) && (entry->vm != vm)) {
+ entry = NULL;
+ }
+ } else {
+ DEFINE_INTX_SID(sid, gsi, gsi_ctlr);
+ entry = find_ptirq_entry(PTDEV_INTR_INTX, &sid, vm);
+ }
+
if (entry != NULL) {
if (is_entry_active(entry)) {
phys_irq = entry->allocated_pirq;
@@ -431,11 +434,11 @@ static void remove_intx_remapping(const struct
acrn_vm *vm, uint32_t virt_gsi, e

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",
+ "deactivate %s intx entry:pgsi=%d, pirq=%d
",
+ (entry->virt_sid.intx_id.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);
+ entry->vm->vm_id, entry-

virt_sid.intx_id.gsi);

}

ptirq_release_entry(entry);
@@ -735,7 +738,7 @@ int32_t ptirq_intx_pin_remap(struct acrn_vm *vm,
uint32_t virt_gsi, enum intx_ct
uint32_t phys_gsi = virt_gsi;


remove_intx_remapping(vm, alt_virt_sid.intx_id.gsi,
-
alt_virt_sid.intx_id.ctlr);
+
alt_virt_sid.intx_id.ctlr, false);
entry =
add_intx_remapping(vm, virt_gsi, phys_gsi, vgsi_ctlr);
if (entry == NULL) {
pr_err("%s, add
intx remapping failed", __func__); @@ -806,12 +809,12 @@ int32_t
ptirq_add_intx_remapping(struct acrn_vm *vm, uint32_t virt_gsi, uint32_t
/*
* @pre vm != NULL
*/
-void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t
virt_gsi, bool pic_pin)
+void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t
+gsi, bool pic_pin, bool is_phy_gsi)
{
enum intx_ctlr vgsi_ctlr = pic_pin ? INTX_CTLR_PIC :
INTX_CTLR_IOAPIC;

spinlock_obtain(&ptdev_lock);
- remove_intx_remapping(vm, virt_gsi, vgsi_ctlr);
+ remove_intx_remapping(vm, gsi, vgsi_ctlr, is_phy_gsi);
spinlock_release(&ptdev_lock);
}

@@ -839,6 +842,6 @@ void
ptirq_remove_configured_intx_remappings(const struct acrn_vm *vm)
uint16_t i;

for (i = 0; i < vm_config->pt_intx_num; i++) {
- ptirq_remove_intx_remapping(vm, vm_config-

pt_intx[i].virt_gsi, false);

+ ptirq_remove_intx_remapping(vm, vm_config-

pt_intx[i].virt_gsi,

+false, false);
}
}
diff --git a/hypervisor/common/hypercall.c
b/hypervisor/common/hypercall.c index 5269380c1..5e296f710 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -1016,6 +1016,7 @@ int32_t hcall_set_ptdev_intr_info(struct
acrn_vcpu *vcpu, struct acrn_vm *target
if ((((!irq.intx.pic_pin) &&
(irq.intx.virt_pin < get_vm_gsicount(target_vm)))
|| ((irq.intx.pic_pin) &&
(irq.intx.virt_pin < vpic_pincount())))
&&
is_gsi_valid(irq.intx.phys_pin)) {
+
ptirq_remove_intx_remapping(get_service_vm(), irq.intx.phys_pin,
+false, true);
ret =
ptirq_add_intx_remapping(target_vm, irq.intx.virt_pin,

irq.intx.phys_pin, irq.intx.pic_pin);
} else {
@@ -1067,7 +1068,7 @@ int32_t hcall_reset_ptdev_intr_info(struct
acrn_vcpu *vcpu, struct acrn_vm *targ
if ((vdev != NULL) && (vdev->pdev-

bdf.value == irq.phys_bdf)) {

if (((!irq.intx.pic_pin) &&
(irq.intx.virt_pin < get_vm_gsicount(target_vm))) ||
((irq.intx.pic_pin) &&
(irq.intx.virt_pin < vpic_pincount()))) {
-
ptirq_remove_intx_remapping(target_vm, irq.intx.virt_pin,
irq.intx.pic_pin);
+
ptirq_remove_intx_remapping(target_vm, irq.intx.virt_pin,
+irq.intx.pic_pin, false);
ret = 0;
} else {
pr_err("%s: Invalid virt
pin\n", __func__); diff --git
a/hypervisor/include/arch/x86/asm/guest/assign.h
b/hypervisor/include/arch/x86/asm/guest/assign.h
index 9b3b666af..e601dcef6 100644
--- a/hypervisor/include/arch/x86/asm/guest/assign.h
+++ b/hypervisor/include/arch/x86/asm/guest/assign.h
@@ -112,18 +112,20 @@ int32_t ptirq_add_intx_remapping(struct acrn_vm
*vm, uint32_t virt_gsi, uint32_t
/**
* @brief Remove an interrupt remapping entry for INTx.
*
- * Deactivate & remove mapping entry of the given virt_pin for given vm.
+ * Deactivate & remove mapping entry of the given virt gsi for given vm
+ or
+ * phys gsi assigned to this vm.
*
* @param[in] vm pointer to acrn_vm
- * @param[in] virt_gsi virtual pin number associated with the passthrough
device
+ * @param[in] gsi virtual gsi number or physical gsi number associated
+ with the passthrough device
* @param[in] pic_pin true for pic, false for ioapic
+ * @param[in] is_phy_gsi true if gsi is physical, false if gsi is
+ virtual
*
* @return None
*
* @pre vm != NULL
*
*/
-void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t
virt_gsi, bool pic_pin);
+void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t
+gsi, bool pic_pin, bool is_phy_gsi);

/**
* @brief Remove interrupt remapping entry/entries for MSI/MSI-x.
--
2.30.2