Re: [PATCH v2] PCI: vmd: Clear PCI_INTERRUPT_LINE for VMD sub-devices

[Nirmal Patel <nirmal.patel@linux.intel.com>]

On Thu, 22 Aug 2024 15:18:06 +0530
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:

> On Tue, Aug 20, 2024 at 03:32:13PM -0700, Nirmal Patel wrote:
> > VMD does not support INTx for devices downstream from a VMD
> > endpoint. So initialize the PCI_INTERRUPT_LINE to 0 for all NVMe
> > devices under VMD to ensure other applications don't try to set up
> > an INTx for them.
> > 
> > Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>  
> 
> I shared a diff to put it in pci_assign_irq() and you said that you
> were going to test it [1]. I don't see a reply to that and now you
> came up with another approach.
> 
> What happened inbetween?

Apologies, I did perform the tests and the patch worked fine. However, I
was able to see lot of bridge devices had the register set to 0xFF and I
didn't want to alter them. Also pci_assign_irg would still set the
interrupt line register to 0 with or without VMD. Since I didn't want to
introduce issues for non-VMD setup, I decide to keep the change limited
only to the VMD.

-Nirmal
> 
> - Mani
> 
> [1]
> https://lore.kernel.org/linux-pci/20240801115756.0000272e@linux.intel.com
> 
> > ---
> > v2->v1: Change the execution from fixup.c to vmd.c
> > ---
> >  drivers/pci/controller/vmd.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/vmd.c
> > b/drivers/pci/controller/vmd.c index a726de0af011..2e9b99969b81
> > 100644 --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c
> > @@ -778,6 +778,18 @@ static int vmd_pm_enable_quirk(struct pci_dev
> > *pdev, void *userdata) return 0;
> >  }
> >  
> > +/*
> > + * Some applications like SPDK reads PCI_INTERRUPT_LINE to decide
> > + * whether INTx is enabled or not. Since VMD doesn't support INTx,
> > + * write 0 to all NVMe devices under VMD.
> > + */
> > +static int vmd_clr_int_line_reg(struct pci_dev *dev, void
> > *userdata) +{
> > +	if(dev->class == PCI_CLASS_STORAGE_EXPRESS)
> > +		pci_write_config_byte(dev, PCI_INTERRUPT_LINE, 0);
> > +	return 0;
> > +}
> > +
> >  static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long
> > features) {
> >  	struct pci_sysdata *sd = &vmd->sysdata;
> > @@ -932,6 +944,7 @@ static int vmd_enable_domain(struct vmd_dev
> > *vmd, unsigned long features) 
> >  	pci_scan_child_bus(vmd->bus);
> >  	vmd_domain_reset(vmd);
> > +	pci_walk_bus(vmd->bus, vmd_clr_int_line_reg, &features);
> >  
> >  	/* When Intel VMD is enabled, the OS does not discover the
> > Root Ports
> >  	 * owned by Intel VMD within the MMCFG space.
> > pci_reset_bus() applies -- 
> > 2.39.1
> > 
> >   
>