Re: [PATCH 5/6] x86/apic/msi: Use Real PCI DMA device when configuring IRTE

From: Derrick, Jonathan
Date: Wed Oct 21 2020 - 15:55:11 EST


On Tue, 2020-10-20 at 21:21 -0500, Bjorn Helgaas wrote:
> On Wed, Oct 21, 2020 at 01:20:24AM +0000, Derrick, Jonathan wrote:
> > On Tue, 2020-10-20 at 15:26 -0500, Bjorn Helgaas wrote:
> > > On Tue, Jul 28, 2020 at 01:49:44PM -0600, Jon Derrick wrote:
> > > > VMD retransmits child device MSI/X with the VMD endpoint's requester-id.
> > > > In order to support direct interrupt remapping of VMD child devices,
> > > > ensure that the IRTE is programmed with the VMD endpoint's requester-id
> > > > using pci_real_dma_dev().
> > > >
> > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > > > Signed-off-by: Jon Derrick <jonathan.derrick@xxxxxxxxx>
> > >
> > > As Thomas (and Stephen) pointed out, this conflicts with 7ca435cf857d
> > > ("x86/irq: Cleanup the arch_*_msi_irqs() leftovers"), which removes
> > > native_setup_msi_irqs().
> > >
> > > Stephen resolved the conflict by dropping this hunk. I would rather
> > > just drop this patch completely from the PCI tree. If I keep the
> > > patch, (1) Linus will have to resolve the conflict, and worse, (2)
> > > it's not clear what happened to the use of pci_real_dma_dev() here.
> > > It will just vanish into the ether with no explanation other than
> > > "this function was removed."
> > >
> > > Is dropping this patch the correct thing to do? Or do you need to add
> > > pci_real_dma_dev() elsewhere to compensate?
> >
> > It would still need the pci_real_dma_dev() for IRTE programming.
> >
> > I think at this point I would rather see 5+6 dropped and this included
> > for TGL enablement:
> > https://patchwork.kernel.org/project/linux-pci/patch/20200914190128.5114-1-jonathan.derrick@xxxxxxxxx/
>
> It's too late to add new things for v5.10. I'll drop 5 and I'll be
> happy to drop 6, too, if you want. I have several comments/questions
> on 6 anyway that I haven't finished writing up.
>
> But if you'd rather have 1-4 + 6 in v5.10 instead of just 1-4, let me
> know.
>
> Bjorn

Here's the proposed new location for patch 5 for pci_real_dma_dev(),
but I can't test this at the moment:

diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index 6313f0a05db7..707968b234e9 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -194,6 +194,7 @@ int pci_msi_prepare(struct irq_domain *domain,
struct device *dev, int nvec,
arg->type = X86_IRQ_ALLOC_TYPE_PCI_MSI;
arg->flags |= X86_IRQ_ALLOC_CONTIGUOUS_VECTORS;
}
+ arg->devid = pci_real_dma_dev(pdev);

return 0;
}
--
2.18.1


Otherwise I would want to drop 5 & 6 because 6 will likely break VMD
without patch 5 when IO APIC is in use