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

From: "Derrick, Jonathan" <jonathan.derrick@intel.com>
To: "helgaas@kernel.org" <helgaas@kernel.org>
Cc: "hch@lst.de" <hch@lst.de>, "x86@kernel.org" <x86@kernel.org>,
	"Shevchenko, Andriy" <andriy.shevchenko@intel.com>,
	"lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
	"andrzej.jakowski@linux.intel.com"
	<andrzej.jakowski@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"Kalakota, SushmaX" <sushmax.kalakota@intel.com>
Subject: Re: [PATCH 5/6] x86/apic/msi: Use Real PCI DMA device when configuring IRTE
Date: Wed, 21 Oct 2020 01:20:24 +0000	[thread overview]
Message-ID: <a7862c6f4ee5ccad035037473b211366e29436ba.camel@intel.com> (raw)
In-Reply-To: <20201020202649.GA394225@bjorn-Precision-5520>

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@intel.com>
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> 
> 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@intel.com/

> 
> > ---
> >  arch/x86/kernel/apic/msi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
> > index c2b2911feeef..7ca271b8d891 100644
> > --- a/arch/x86/kernel/apic/msi.c
> > +++ b/arch/x86/kernel/apic/msi.c
> > @@ -189,7 +189,7 @@ int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> >  
> >  	init_irq_alloc_info(&info, NULL);
> >  	info.type = X86_IRQ_ALLOC_TYPE_MSI;
> > -	info.msi_dev = dev;
> > +	info.msi_dev = pci_real_dma_dev(dev);
> >  
> >  	domain = irq_remapping_get_irq_domain(&info);
> >  	if (domain == NULL)
> > -- 
> > 2.27.0
> >