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

[Long Li]

> 
> From: Wei Liu <wei.liu@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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

Since you don't want to check the return value from hv_unmap_msi_interrupt(), add a (void) to the front to prevent some tools from giving warnings.