Hi Zenghui,
On Fri, 18 Dec 2020 06:00:39 +0000,
Zenghui Yu <yuzenghui@xxxxxxxxxx> wrote:
>
> Since commit 5fe71d271df8 ("irqchip/gic-v3-its: Tag ITS device as shared if
> allocating for a proxy device"), some of the devices are wrongly marked as
> "shared" by the ITS driver on systems equipped with the ITS(es). The
> problem is that the @info->flags may not be initialized anywhere and we end
> up looking at random bits on the stack. That's obviously not good.
>
> The straightforward fix is to properly initialize msi_alloc_info inside the
> .prepare callback of affected MSI domains (its-pci-msi, its-platform-msi,
> etc). We can also perform the initialization in IRQ core layer for
> msi_domain_prepare_irqs() API and it looks much neater to me.
>
> Signed-off-by: Zenghui Yu <yuzenghui@xxxxxxxxxx>
> ---
>
> This was noticed when I was playing with the assigned devices on arm64 and
> VFIO failed to enable MSI-X vectors for almost all VFs (CCed kvm list in
> case others will hit the same issue). It turned out that these VFs are
> marked as "shared" by mistake and have trouble with the following sequence:
>
> pci_alloc_irq_vectors(pdev, 1, 1, flag);
> pci_free_irq_vectors(pdev);
> pci_alloc_irq_vectors(pdev, 1, 2, flag); --> we can only get
> *one* vector
>
> But besides VFIO, I guess there are already some devices get into trouble
> at probe time and can't work properly.
>
> kernel/irq/msi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index 2c0c4d6d0f83..dc0e2d7fbdfd 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -402,7 +402,7 @@ int __msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
> struct msi_domain_ops *ops = info->ops;
> struct irq_data *irq_data;
> struct msi_desc *desc;
> - msi_alloc_info_t arg;
> + msi_alloc_info_t arg = { };
> int i, ret, virq;
> bool can_reserve;
Thanks for having investigated this. I guess my only worry with this
is that msi_alloc_info_t is a pretty large structure on x86, and
zeroing it isn't totally free. But this definitely looks nicer than
some of the alternatives (.prepare isn't a good option, as we do rely
on the flag being set in __platform_msi_create_device_domain(), which
calls itself .prepare).
I'll queue it, and we can always revisit this later if Thomas (or
anyone else) has a better idea.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
