[PATCH 09/11][jammy/linux-azure] PCI: hv: Fix hv_arch_irq_unmask() for multi-MSI

[Bartlomiej Zolnierkiewicz]

On Wed, Jul 13, 2022 at 2:58 PM Tim Gardner <tim.gardner at canonical.com> wrote:
>
> From: Jeffrey Hugo <quic_jhugo at quicinc.com>
>
> BugLink: https://bugs.launchpad.net/bugs/1981577
>
> In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first
> MSI of the N allocated.  This is because only the first msi_desc is cached
> and it is shared by all the MSIs of the multi-MSI block.  This means that
> hv_arch_irq_unmask() gets the correct address, but the wrong data (always
> 0).
>
> This can break MSIs.
>
> Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0.
>
> hv_arch_irq_unmask() is called on MSI0.  It uses a hypercall to configure
> the MSI address and data (0) to vector 34 of CPU0.  This is correct.  Then
> hv_arch_irq_unmask is called on MSI1.  It uses another hypercall to
> configure the MSI address and data (0) to vector 33 of CPU0.  This is
> wrong, and results in both MSI0 and MSI1 being routed to vector 33.  Linux
> will observe extra instances of MSI1 and no instances of MSI0 despite the
> endpoint device behaving correctly.
>
> For the multi-MSI case, we need unique address and data info for each MSI,
> but the cached msi_desc does not provide that.  However, that information
> can be gotten from the int_desc cached in the chip_data by
> compose_msi_msg().  Fix the multi-MSI case to use that cached information
> instead.  Since hv_set_msi_entry_from_desc() is no longer applicable,
> remove it.
>
> Signed-off-by: Jeffrey Hugo <quic_jhugo at quicinc.com>
> Reviewed-by: Michael Kelley <mikelley at microsoft.com>
> Link: https://lore.kernel.org/r/1651068453-29588-1-git-send-email-quic_jhugo@quicinc.com
> Signed-off-by: Wei Liu <wei.liu at kernel.org>
> (cherry picked from commit 455880dfe292a2bdd3b4ad6a107299fce610e64b)
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 2244338a56ba..317d8d8e52e9 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -605,13 +605,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data)
>         return cfg->vector;
>  }
>
> -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
> -                                      struct msi_desc *msi_desc)
> -{
> -       msi_entry->address.as_uint32 = msi_desc->msg.address_lo;
> -       msi_entry->data.as_uint32 = msi_desc->msg.data;
> -}
> -

Don't we also need to backport commit
22ef7ee3eeb2a41e07f611754ab9a2663232fedf ("PCI: hv: Remove unused
hv_set_msi_entry_from_desc()") after applying patch #8 [ which is
commit d06957d7a6929e6a4aa959cb59d66f0c095fc974 ("PCI: hv: Avoid the
retarget interrupt hypercall in irq_unmask() on ARM64") ] and before
applying this patch?

Best regards,
Bartlomiej

>  static int hv_msi_prepare(struct irq_domain *domain, struct device *dev,
>                           int nvec, msi_alloc_info_t *info)
>  {
> @@ -641,6 +634,7 @@ static void hv_arch_irq_unmask(struct irq_data *data)
>  {
>         struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
>         struct hv_retarget_device_interrupt *params;
> +       struct tran_int_desc *int_desc;
>         struct hv_pcibus_device *hbus;
>         struct cpumask *dest;
>         cpumask_var_t tmp;
> @@ -655,6 +649,7 @@ static void hv_arch_irq_unmask(struct irq_data *data)
>         pdev = msi_desc_to_pci_dev(msi_desc);
>         pbus = pdev->bus;
>         hbus = container_of(pbus->sysdata, struct hv_pcibus_device, sysdata);
> +       int_desc = data->chip_data;
>
>         spin_lock_irqsave(&hbus->retarget_msi_interrupt_lock, flags);
>
> @@ -662,7 +657,8 @@ static void hv_arch_irq_unmask(struct irq_data *data)
>         memset(params, 0, sizeof(*params));
>         params->partition_id = HV_PARTITION_ID_SELF;
>         params->int_entry.source = HV_INTERRUPT_SOURCE_MSI;
> -       hv_set_msi_entry_from_desc(&params->int_entry.msi_entry, msi_desc);
> +       params->int_entry.msi_entry.address.as_uint32 = int_desc->address & 0xffffffff;
> +       params->int_entry.msi_entry.data.as_uint32 = int_desc->data;
>         params->device_id = (hbus->hdev->dev_instance.b[5] << 24) |
>                            (hbus->hdev->dev_instance.b[4] << 16) |
>                            (hbus->hdev->dev_instance.b[7] << 8) |
> --
> 2.37.0
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team