acrn-dev@lists.projectacrn.org | [PATCH v3 2/2] ptirq: Fix INTx assignment for Post-launched VM

Home Messages Hashtags Subgroups

Date Date 1 - 3 of 3

[PATCH v3 2/2] ptirq: Fix INTx assignment for Post-launched VM

Qiang Zhang #37363 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 \| 41 ++++++++++++++----- hypervisor/common/hypercall.c \| 2 +- .../include/arch/x86/asm/guest/assign.h \| 21 ++++++++++ 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/hypervisor/arch/x86/guest/assign.c b/hypervisor/arch/x86/guest/assign.c index a49b3314b..0baa9404b 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,7 +404,7 @@ static struct ptirq_remapping_info add_intx_remapping(struct acrn_vm vm, uint3 return entry; } -/* deactive & remove mapping entry of vpin for vm / +/ 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; @@ -431,7 +425,7 @@ 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 ", + "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", @@ -442,6 +436,19 @@ static void remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, e } } +static struct ptirq_remapping_info reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, + uint32_t phys_gsi, enum intx_ctlr vgsi_ctlr) +{ + struct ptirq_remapping_info entry = NULL; + DEFINE_INTX_SID(phys_sid, phys_gsi, INTX_CTLR_IOAPIC); + + entry = find_ptirq_entry(PTDEV_INTR_INTX, &phys_sid, NULL); + if (entry) + remove_intx_remapping(entry->vm, entry->virt_sid.intx_id.gsi, entry->virt_sid.intx_id.ctlr); + + return add_intx_remapping(vm, virt_gsi, phys_gsi, vgsi_ctlr); +} + static void ptirq_handle_intx(struct acrn_vm vm, const struct ptirq_remapping_info entry) { @@ -815,6 +822,18 @@ void ptirq_remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, bo spinlock_release(&ptdev_lock); } +int32_t ptirq_reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t phys_gsi, bool pic_pin) +{ + struct ptirq_remapping_info entry; + enum intx_ctlr vgsi_ctlr = pic_pin ? INTX_CTLR_PIC : INTX_CTLR_IOAPIC; + + spinlock_obtain(&ptdev_lock); + entry = reassign_intx_remapping(vm, virt_gsi, phys_gsi, vgsi_ctlr); + spinlock_release(&ptdev_lock); + + return (entry != NULL) ? 0 : -ENODEV; +} + /* * @pre vm != NULL / diff --git a/hypervisor/common/hypercall.c b/hypervisor/common/hypercall.c index 5269380c1..faec16545 100644 --- a/hypervisor/common/hypercall.c +++ b/hypervisor/common/hypercall.c @@ -1016,7 +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)) { - ret = ptirq_add_intx_remapping(target_vm, irq.intx.virt_pin, + ret = ptirq_reassign_intx_remapping(target_vm, irq.intx.virt_pin, irq.intx.phys_pin, irq.intx.pic_pin); } else { pr_err("%s: Invalid phys pin or 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..1387b9825 100644 --- a/hypervisor/include/arch/x86/asm/guest/assign.h +++ b/hypervisor/include/arch/x86/asm/guest/assign.h @@ -125,6 +125,27 @@ int32_t ptirq_add_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t / void ptirq_remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, bool pic_pin); +/** + * @brief Assign a INTx to Postlaunched VM. + * + * If the physical pin was assigned to another VM, remove it first. + * Then assign the physical pin to target virtual VM. + * + * @param[in] vm pointer to acrn_vm + * @param[in] virt_gsi virtual pin number associated with the passthrough device + * @param[in] phys_gsi physical pin number associated with the passthrough device + * @param[in] pic_pin true for pic, false for ioapic + * + * @return + * - 0: on success + * - \p -EINVAL: invalid virt_pin value + * - \p -ENODEV: failed to add the remapping entry + * + * @pre vm != NULL + * + / +int32_t ptirq_reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t phys_gsi, bool pic_pin); + /** * @brief Remove interrupt remapping entry/entries for MSI/MSI-x. * -- 2.30.2
More All Messages By This Member
Li, Fei1 #37365 On 2023-02-27 at 20:34:52 +0800, Qiang Zhang wrote: 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 \| 41 ++++++++++++++----- hypervisor/common/hypercall.c \| 2 +- .../include/arch/x86/asm/guest/assign.h \| 21 ++++++++++ 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/hypervisor/arch/x86/guest/assign.c b/hypervisor/arch/x86/guest/assign.c index a49b3314b..0baa9404b 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,7 +404,7 @@ static struct ptirq_remapping_info add_intx_remapping(struct acrn_vm vm, uint3 return entry; } -/* deactive & remove mapping entry of vpin for vm / +/ 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; @@ -431,7 +425,7 @@ 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 ", + "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", @@ -442,6 +436,19 @@ static void remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, e } } +static struct ptirq_remapping_info reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, + uint32_t phys_gsi, enum intx_ctlr vgsi_ctlr) +{ + struct ptirq_remapping_info entry = NULL; + DEFINE_INTX_SID(phys_sid, phys_gsi, INTX_CTLR_IOAPIC); + + entry = find_ptirq_entry(PTDEV_INTR_INTX, &phys_sid, NULL); + if (entry) This means you could re-assign a ptirq from pre-launched/Service/post-launched VM to any other VM ? + remove_intx_remapping(entry->vm, entry->virt_sid.intx_id.gsi, entry->virt_sid.intx_id.ctlr); + + return add_intx_remapping(vm, virt_gsi, phys_gsi, vgsi_ctlr); +} + static void ptirq_handle_intx(struct acrn_vm vm, const struct ptirq_remapping_info entry) { @@ -815,6 +822,18 @@ void ptirq_remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, bo spinlock_release(&ptdev_lock); } +int32_t ptirq_reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t phys_gsi, bool pic_pin) +{ + struct ptirq_remapping_info entry; + enum intx_ctlr vgsi_ctlr = pic_pin ? INTX_CTLR_PIC : INTX_CTLR_IOAPIC; + + spinlock_obtain(&ptdev_lock); + entry = reassign_intx_remapping(vm, virt_gsi, phys_gsi, vgsi_ctlr); + spinlock_release(&ptdev_lock); + + return (entry != NULL) ? 0 : -ENODEV; +} + /* * @pre vm != NULL / diff --git a/hypervisor/common/hypercall.c b/hypervisor/common/hypercall.c index 5269380c1..faec16545 100644 --- a/hypervisor/common/hypercall.c +++ b/hypervisor/common/hypercall.c @@ -1016,7 +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)) { - ret = ptirq_add_intx_remapping(target_vm, irq.intx.virt_pin, + ret = ptirq_reassign_intx_remapping(target_vm, irq.intx.virt_pin, irq.intx.phys_pin, irq.intx.pic_pin); } else { pr_err("%s: Invalid phys pin or 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..1387b9825 100644 --- a/hypervisor/include/arch/x86/asm/guest/assign.h +++ b/hypervisor/include/arch/x86/asm/guest/assign.h @@ -125,6 +125,27 @@ int32_t ptirq_add_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t / void ptirq_remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, bool pic_pin); +/** + * @brief Assign a INTx to Postlaunched VM. + * + * If the physical pin was assigned to another VM, remove it first. + * Then assign the physical pin to target virtual VM. + * + * @param[in] vm pointer to acrn_vm + * @param[in] virt_gsi virtual pin number associated with the passthrough device + * @param[in] phys_gsi physical pin number associated with the passthrough device + * @param[in] pic_pin true for pic, false for ioapic + * + * @return + * - 0: on success + * - \p -EINVAL: invalid virt_pin value + * - \p -ENODEV: failed to add the remapping entry + * + * @pre vm != NULL + * + / +int32_t ptirq_reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t phys_gsi, bool pic_pin); + /** * @brief Remove interrupt remapping entry/entries for MSI/MSI-x. * -- 2.30.2
More All Messages By This Member
Qiang Zhang #37367 On Tue, Feb 28, 2023 at 04:47:55PM +0800, Fei Li wrote: On 2023-02-27 at 20:34:52 +0800, Qiang Zhang wrote: 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 \| 41 ++++++++++++++----- hypervisor/common/hypercall.c \| 2 +- .../include/arch/x86/asm/guest/assign.h \| 21 ++++++++++ 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/hypervisor/arch/x86/guest/assign.c b/hypervisor/arch/x86/guest/assign.c index a49b3314b..0baa9404b 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,7 +404,7 @@ static struct ptirq_remapping_info add_intx_remapping(struct acrn_vm vm, uint3 return entry; } -/* deactive & remove mapping entry of vpin for vm / +/ 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; @@ -431,7 +425,7 @@ 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 ", + "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", @@ -442,6 +436,19 @@ static void remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, e } } +static struct ptirq_remapping_info reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, + uint32_t phys_gsi, enum intx_ctlr vgsi_ctlr) +{ + struct ptirq_remapping_info entry = NULL; + DEFINE_INTX_SID(phys_sid, phys_gsi, INTX_CTLR_IOAPIC); + + entry = find_ptirq_entry(PTDEV_INTR_INTX, &phys_sid, NULL); + if (entry) This means you could re-assign a ptirq from pre-launched/Service/post-launched VM to any other VM ? How about move the check/remove/add logic to hypercall.c ? + remove_intx_remapping(entry->vm, entry->virt_sid.intx_id.gsi, entry->virt_sid.intx_id.ctlr); + + return add_intx_remapping(vm, virt_gsi, phys_gsi, vgsi_ctlr); +} + static void ptirq_handle_intx(struct acrn_vm vm, const struct ptirq_remapping_info entry) { @@ -815,6 +822,18 @@ void ptirq_remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, bo spinlock_release(&ptdev_lock); } +int32_t ptirq_reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t phys_gsi, bool pic_pin) +{ + struct ptirq_remapping_info entry; + enum intx_ctlr vgsi_ctlr = pic_pin ? INTX_CTLR_PIC : INTX_CTLR_IOAPIC; + + spinlock_obtain(&ptdev_lock); + entry = reassign_intx_remapping(vm, virt_gsi, phys_gsi, vgsi_ctlr); + spinlock_release(&ptdev_lock); + + return (entry != NULL) ? 0 : -ENODEV; +} + /* * @pre vm != NULL / diff --git a/hypervisor/common/hypercall.c b/hypervisor/common/hypercall.c index 5269380c1..faec16545 100644 --- a/hypervisor/common/hypercall.c +++ b/hypervisor/common/hypercall.c @@ -1016,7 +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)) { - ret = ptirq_add_intx_remapping(target_vm, irq.intx.virt_pin, + ret = ptirq_reassign_intx_remapping(target_vm, irq.intx.virt_pin, irq.intx.phys_pin, irq.intx.pic_pin); } else { pr_err("%s: Invalid phys pin or 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..1387b9825 100644 --- a/hypervisor/include/arch/x86/asm/guest/assign.h +++ b/hypervisor/include/arch/x86/asm/guest/assign.h @@ -125,6 +125,27 @@ int32_t ptirq_add_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t / void ptirq_remove_intx_remapping(const struct acrn_vm vm, uint32_t virt_gsi, bool pic_pin); +/** + * @brief Assign a INTx to Postlaunched VM. + * + * If the physical pin was assigned to another VM, remove it first. + * Then assign the physical pin to target virtual VM. + * + * @param[in] vm pointer to acrn_vm + * @param[in] virt_gsi virtual pin number associated with the passthrough device + * @param[in] phys_gsi physical pin number associated with the passthrough device + * @param[in] pic_pin true for pic, false for ioapic + * + * @return + * - 0: on success + * - \p -EINVAL: invalid virt_pin value + * - \p -ENODEV: failed to add the remapping entry + * + * @pre vm != NULL + * + / +int32_t ptirq_reassign_intx_remapping(struct acrn_vm vm, uint32_t virt_gsi, uint32_t phys_gsi, bool pic_pin); + /** * @brief Remove interrupt remapping entry/entries for MSI/MSI-x. * -- 2.30.2
More All Messages By This Member

1 - 3 of 3

1

Previous Topic Next Topic

Home Hashtags Subgroups Terms