[PATCH 2/3] PCI: hv: unmap MSI interrupt on the nested root partition teardown path

[PATCH 2/3] PCI: hv: unmap MSI interrupt on the nested root partition teardown path

[wei.liu]

From: Wei Liu <wei.liu@kernel.org>

On a nested root partition the vPCI MSI/MSI-X interrupts of vmbus
devices (e.g. the MANA NIC) are mapped in the hypervisor with a
MAP_DEVICE_INTERRUPT hypercall.  This is done from hv_arch_irq_unmask()
-> hv_map_msi_interrupt() because the nested hypervisor performs the
interrupt remapping and a RETARGET_INTERRUPT is not usable there.

The mapping was never removed: hv_arch_irq_unmask() called
hv_map_msi_interrupt(data, NULL), so the returned hv_interrupt_entry was
discarded, and hv_msi_free() tears the interrupt down with a vmbus
PCI_DELETE_INTERRUPT message (hv_int_desc_free()) without issuing
UNMAP_DEVICE_INTERRUPT.

This has led to MSHV rejecting already-mapped (vp, vector) pair from
being used. When this happens during early boot, the system hangs.

Keep the hypervisor mapping in sync with the kernel's interrupt
lifecycle.

The mapping is only created on x86 (hv_arch_irq_unmask() is a stub on
arm64), so the unmap hypercall is guarded accordingly.

Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 drivers/pci/controller/pci-hyperv.c | 85 ++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index cfc8fa403dad..5a36382742bf 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -283,6 +283,35 @@ struct tran_int_desc {
 	u64	address;
 } __packed;
 
+/*
+ * On a nested root partition a vPCI MSI is mapped in the hypervisor with a
+ * MAP_DEVICE_INTERRUPT hypercall in hv_arch_irq_unmask().  Keep the entry the
+ * hypervisor returns next to the per-interrupt transaction descriptor so the
+ * mapping can be removed again with UNMAP_DEVICE_INTERRUPT when the interrupt
+ * is torn down.  tran_int_desc stays first: chip_data is used as a struct
+ * tran_int_desc throughout this driver.
+ */
+struct hv_msi_int_entry {
+	struct tran_int_desc		int_desc;
+	struct hv_interrupt_entry	hv_entry;
+};
+
+/* chip_data is passed around as a struct tran_int_desc *, so it must be first. */
+static_assert(offsetof(struct hv_msi_int_entry, int_desc) == 0);
+
+static void hv_vmbus_unmap_msi_interrupt(struct pci_dev *pdev __maybe_unused,
+					 void *chip_data)
+{
+	struct hv_msi_int_entry *ie = chip_data;
+
+	if (!ie || !ie->hv_entry.source)
+		return;
+#ifdef CONFIG_X86
+	hv_unmap_msi_interrupt(pdev, &ie->hv_entry);
+#endif
+	memset(&ie->hv_entry, 0, sizeof(ie->hv_entry));
+}
+
 /*
  * A generic message format for virtual PCI.
  * Specific message formats are defined later in the file.
@@ -715,16 +744,30 @@ static void hv_irq_retarget_interrupt(struct irq_data *data)
 
 static void hv_arch_irq_unmask(struct irq_data *data)
 {
-	if (hv_root_partition())
+	if (hv_root_partition()) {
 		/*
 		 * In case of the nested root partition, the nested hypervisor
 		 * is taking care of interrupt remapping and thus the
 		 * MAP_DEVICE_INTERRUPT hypercall is required instead of
 		 * RETARGET_INTERRUPT.
+		 *
+		 * Keep the returned entry so the mapping can be removed again
+		 * when the interrupt is torn down.
 		 */
-		(void)hv_map_msi_interrupt(data, NULL);
-	else
+		struct hv_msi_int_entry *ie = data->chip_data;
+
+		/*
+		 * A NULL chip_data means hv_compose_msi_msg() failed and the
+		 * interrupt was never set up, so there is nothing to map.
+		 */
+		if (!ie)
+			return;
+
+		if (hv_map_msi_interrupt(data, &ie->hv_entry))
+			memset(&ie->hv_entry, 0, sizeof(ie->hv_entry));
+	} else {
 		hv_irq_retarget_interrupt(data);
+	}
 }
 #elif defined(CONFIG_ARM64)
 /*
@@ -1708,6 +1751,7 @@ static void hv_msi_free(struct irq_domain *domain, unsigned int irq)
 		return;
 	}
 
+	hv_vmbus_unmap_msi_interrupt(pdev, int_desc);
 	hv_int_desc_free(hpdev, int_desc);
 	put_pcichild(hpdev);
 }
@@ -1882,6 +1926,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	const struct cpumask *dest;
 	struct compose_comp_ctxt comp;
 	struct tran_int_desc *int_desc;
+	struct hv_msi_int_entry *int_entry;
 	struct msi_desc *msi_desc;
 	/*
 	 * vector_count should be u16: see hv_msi_desc, hv_msi_desc2
@@ -1932,9 +1977,10 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 		hv_int_desc_free(hpdev, int_desc);
 	}
 
-	int_desc = kzalloc_obj(*int_desc, GFP_ATOMIC);
-	if (!int_desc)
+	int_entry = kzalloc_obj(*int_entry, GFP_ATOMIC);
+	if (!int_entry)
 		goto drop_reference;
+	int_desc = &int_entry->int_desc;
 
 	if (multi_msi) {
 		/*
@@ -2184,9 +2230,34 @@ static void hv_pcie_domain_free(struct irq_domain *d, unsigned int virq, unsigne
 	irq_domain_free_irqs_top(d, virq, nr_irqs);
 }
 
+/*
+ * Runs from irq_domain_deactivate_irq() during irq_shutdown(), before the
+ * parent (x86 vector) domain is deactivated and the (cpu, vector) is returned
+ * to the matrix allocator, so a freed vector can never collide with a stale
+ * hypervisor entry when it is reused.
+ */
+static void hv_pcie_domain_deactivate(struct irq_domain *d,
+				      struct irq_data *data)
+{
+	struct msi_desc *msi_desc;
+	struct pci_dev *pdev;
+
+	if (!hv_root_partition())
+		return;
+
+	msi_desc = irq_data_get_msi_desc(data);
+	if (!msi_desc)
+		return;
+
+	pdev = msi_desc_to_pci_dev(msi_desc);
+	if (pdev)
+		hv_vmbus_unmap_msi_interrupt(pdev, data->chip_data);
+}
+
 static const struct irq_domain_ops hv_pcie_domain_ops = {
-	.alloc	= hv_pcie_domain_alloc,
-	.free	= hv_pcie_domain_free,
+	.alloc		= hv_pcie_domain_alloc,
+	.free		= hv_pcie_domain_free,
+	.deactivate	= hv_pcie_domain_deactivate,
 };
 
 /**
-- 
2.53.0

[PATCH 3/3] PCI: hv: fix interrupt affinity change on the nested root partition

[wei.liu]

From: Wei Liu <wei.liu@kernel.org>

hv_compose_msi_msg() sends PCI_CREATE_INTERRUPT carrying
int_desc.vector, and the vPCI backend programs the device with the
address/data it returns for it.

An affinity change only ran irq_chip_set_affinity_parent(), which
re-allocates the x86 vector and nothing else, and hv_arch_irq_unmask()
then issued MAP_DEVICE_INTERRUPT for the new (VP, vector).  Nothing
re-composed the interrupt, so the device kept signalling the vector it
was created with and the new mapping was never used. This led to loss of
interrupts.

Do the re-target where the vector is known and the interrupt is
quiescent.  If the vector changed, re-compose the VMBus interrupt for
it, write the resulting message to the device, and only then map the new
(vp, vector); the old mapping is destroyed as the new message is
composed.

Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 drivers/pci/controller/pci-hyperv.c | 41 +++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 5a36382742bf..5acb8e41567e 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -294,6 +294,7 @@ struct tran_int_desc {
 struct hv_msi_int_entry {
 	struct tran_int_desc		int_desc;
 	struct hv_interrupt_entry	hv_entry;
+	unsigned int			mapped_vector;
 };
 
 /* chip_data is passed around as a struct tran_int_desc *, so it must be first. */
@@ -742,6 +743,8 @@ static void hv_irq_retarget_interrupt(struct irq_data *data)
 			"%s() failed: %#llx", __func__, res);
 }
 
+static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
+
 static void hv_arch_irq_unmask(struct irq_data *data)
 {
 	if (hv_root_partition()) {
@@ -752,9 +755,17 @@ static void hv_arch_irq_unmask(struct irq_data *data)
 		 * RETARGET_INTERRUPT.
 		 *
 		 * Keep the returned entry so the mapping can be removed again
-		 * when the interrupt is torn down.
+		 * when the interrupt is re-targeted or torn down.
+		 *
+		 * This is also the re-target point.  The core calls us from
+		 * __irq_move_irq() with the interrupt masked once the new
+		 * vector has been assigned, so if the vector changed the vmbus
+		 * interrupt is re-composed for it first -- PCI_CREATE_INTERRUPT
+		 * carries the vector, so the device would otherwise keep
+		 * signalling the one it was created with.
 		 */
 		struct hv_msi_int_entry *ie = data->chip_data;
+		unsigned int vec = hv_msi_get_int_vector(data);
 
 		/*
 		 * A NULL chip_data means hv_compose_msi_msg() failed and the
@@ -763,8 +774,29 @@ static void hv_arch_irq_unmask(struct irq_data *data)
 		if (!ie)
 			return;
 
-		if (hv_map_msi_interrupt(data, &ie->hv_entry))
+		/* Already mapped for this vector, nothing changed. */
+		if (ie->mapped_vector == vec && ie->hv_entry.source)
+			return;
+
+		if (ie->mapped_vector && ie->mapped_vector != vec) {
+			struct msi_msg msg;
+
+			hv_compose_msi_msg(data, &msg);
+
+			ie = data->chip_data;
+			if (!ie)
+				return;
+
+			if (data->chip->irq_write_msi_msg)
+				data->chip->irq_write_msi_msg(data, &msg);
+		}
+
+		if (hv_map_msi_interrupt(data, &ie->hv_entry)) {
 			memset(&ie->hv_entry, 0, sizeof(ie->hv_entry));
+			ie->mapped_vector = 0;
+			return;
+		}
+		ie->mapped_vector = vec;
 	} else {
 		hv_irq_retarget_interrupt(data);
 	}
@@ -1974,6 +2006,11 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	if (data->chip_data && !multi_msi) {
 		int_desc = data->chip_data;
 		data->chip_data = NULL;
+		/*
+		 * The descriptor is about to be destroyed, so release the
+		 * hypervisor mapping that belongs to it first.
+		 */
+		hv_vmbus_unmap_msi_interrupt(pdev, int_desc);
 		hv_int_desc_free(hpdev, int_desc);
 	}
 
-- 
2.53.0

Re: [PATCH 2/3] PCI: hv: unmap MSI interrupt on the nested root partition teardown path

[sashiko-bot]

> From: Wei Liu <wei.liu@kernel.org>
> 
> On a nested root partition the vPCI MSI/MSI-X interrupts of vmbus
> devices (e.g. the MANA NIC) are mapped in the hypervisor with a
> MAP_DEVICE_INTERRUPT hypercall.  This is done from hv_arch_irq_unmask()
> -> hv_map_msi_interrupt() because the nested hypervisor performs the
> interrupt remapping and a RETARGET_INTERRUPT is not usable there.
> 
> The mapping was never removed: hv_arch_irq_unmask() called
> hv_map_msi_interrupt(data, NULL), so the returned hv_interrupt_entry was
> discarded, and hv_msi_free() tears the interrupt down with a vmbus
> PCI_DELETE_INTERRUPT message (hv_int_desc_free()) without issuing
> UNMAP_DEVICE_INTERRUPT.
> 
> This has led to MSHV rejecting already-mapped (vp, vector) pair from
> being used. When this happens during early boot, the system hangs.
> 
> Keep the hypervisor mapping in sync with the kernel's interrupt
> lifecycle.
> 
> The mapping is only created on x86 (hv_arch_irq_unmask() is a stub on
> arm64), so the unmap hypercall is guarded accordingly.
> 
> Signed-off-by: Wei Liu <wei.liu@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822003623.2925128-1-wei.liu@kernel.org?part=2

Re: [PATCH 3/3] PCI: hv: fix interrupt affinity change on the nested root partition

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The newly composed MSI message on affinity change is never written to the PCI device because `data->chip->irq_write_msi_msg` is NULL.
- [High] VMBus recomposition is permanently skipped when `mapped_vector` is 0, causing the device to use a stale vector.
- [High] Multi-MSI interrupts leak hypervisor mappings on every affinity change.
--

--- Patch [3]: [PATCH 3/3] PCI: hv: fix interrupt affinity change on the nested root partition ---
commit b9428909b0f32d968c5c2cd7f2d85948c6d7893a
Author: Wei Liu <wei.liu@kernel.org>

PCI: hv: fix interrupt affinity change on the nested root partition

This commit updates interrupt handling on affinity changes in the nested root
partition. It recomposes the VMBus interrupt with the new vector and programs
the device with the resulting message before mapping the new interrupt.

> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 5a36382742bf7..5acb8e41567e6 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c

[ ... ]

> @@ -752,9 +755,17 @@ static void hv_arch_irq_unmask(struct irq_data *data)
>  		 * RETARGET_INTERRUPT.
>  		 *
>  		 * Keep the returned entry so the mapping can be removed again
> -		 * when the interrupt is torn down.
> +		 * when the interrupt is re-targeted or torn down.
> +		 *
> +		 * This is also the re-target point.  The core calls us from
> +		 * __irq_move_irq() with the interrupt masked once the new
> +		 * vector has been assigned, so if the vector changed the vmbus
> +		 * interrupt is re-composed for it first -- PCI_CREATE_INTERRUPT
> +		 * carries the vector, so the device would otherwise keep
> +		 * signalling the one it was created with.
>  		 */
>  		struct hv_msi_int_entry *ie = data->chip_data;
> +		unsigned int vec = hv_msi_get_int_vector(data);
>  
>  		/*
>  		 * A NULL chip_data means hv_compose_msi_msg() failed and the
> @@ -763,8 +774,29 @@ static void hv_arch_irq_unmask(struct irq_data *data)
>  		if (!ie)
>  			return;
>  
> -		if (hv_map_msi_interrupt(data, &ie->hv_entry))
> +		/* Already mapped for this vector, nothing changed. */
> +		if (ie->mapped_vector == vec && ie->hv_entry.source)
> +			return;
> +
> +		if (ie->mapped_vector && ie->mapped_vector != vec) {

[Severity: High]
If hv_map_msi_interrupt() fails previously, ie->mapped_vector is set to 0.

On the next unmask, if the vector has changed due to an affinity change,
will this ie->mapped_vector != 0 check skip recomposing the message?

It appears this could leave the device programmed with the old vector while
the hypervisor is programmed with the new one.

> +			struct msi_msg msg;
> +
> +			hv_compose_msi_msg(data, &msg);
> +
> +			ie = data->chip_data;
> +			if (!ie)
> +				return;
> +
> +			if (data->chip->irq_write_msi_msg)
> +				data->chip->irq_write_msi_msg(data, &msg);

[Severity: High]
Does hv_msi_irq_chip actually implement the irq_write_msi_msg callback?

Looking at the hv_msi_irq_chip definition, it seems to lack this callback.

If so, would this check fail and prevent the new MSI message from ever being
written to the device, causing it to continue signalling the old unmapped
vector?

> +		}
> +
> +		if (hv_map_msi_interrupt(data, &ie->hv_entry)) {

[Severity: High]
For multi-MSI devices, does this overwrite ie->hv_entry and leak hypervisor
mappings on every affinity change?

When hv_compose_msi_msg() is called for a multi-MSI device, it hits an
early return and bypasses the hv_vmbus_unmap_msi_interrupt() cleanup:

hv_compose_msi_msg() {
    ...
    if (data->chip_data && multi_msi) {
        ...
        return;
    }
    ...
}

Then hv_map_msi_interrupt() is called here, which issues a new
HVCALL_MAP_DEVICE_INTERRUPT and overwrites ie->hv_entry. Since the
previous mapping wasn't unmapped, is it permanently leaked?

>  			memset(&ie->hv_entry, 0, sizeof(ie->hv_entry));
> +			ie->mapped_vector = 0;
> +			return;
> +		}
> +		ie->mapped_vector = vec;
>  	} else {
>  		hv_irq_retarget_interrupt(data);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822003623.2925128-1-wei.liu@kernel.org?part=3