diff mbox series

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

Message ID 20200728194945.14126-6-jonathan.derrick@intel.com
State New
Headers show
Series VMD MSI Remapping Bypass | expand

Commit Message

Jon Derrick July 28, 2020, 7:49 p.m. UTC 
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>
---
 arch/x86/kernel/apic/msi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lorenzo Pieralisi Sept. 7, 2020, 2:32 p.m. UTC | #1 
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>
> ---
>  arch/x86/kernel/apic/msi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I'd need an x86 maintainer ACK on this patch.

Thanks,
Lorenzo

> 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
>
Thomas Gleixner Sept. 28, 2020, 11:32 a.m. UTC | #2 
On Mon, Sep 07 2020 at 15:32, Lorenzo Pieralisi 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>
>> ---
>>  arch/x86/kernel/apic/msi.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> I'd need an x86 maintainer ACK on this patch.

That conflicts with the big PCI/MSI overhaul which is pending in

  git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/irq

native_setup_msi_irqs() does not exist anymore.

patch 3 has conflicts as well.

Thanks,

        tglx
Bjorn Helgaas Oct. 20, 2020, 8:26 p.m. UTC | #3 
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?

> ---
>  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
>
Jon Derrick Oct. 21, 2020, 1:20 a.m. UTC | #4 
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
> >
Bjorn Helgaas Oct. 21, 2020, 2:21 a.m. UTC | #5 
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@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/

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
Jon Derrick Oct. 21, 2020, 7:55 p.m. UTC | #6 
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@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/
> 
> 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;
 }
diff mbox series

Patch

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)