Re: [PATCH v2] PCI/portdrv: Do not require an interrupt for all AER capable ports

[Bjorn Helgaas <helgaas@kernel.org>]

Hi Mika,

On Wed, Dec 07, 2022 at 10:41:05AM +0200, Mika Westerberg wrote:
> Only Root Ports and Event Collectors use MSI for AER. PCIe Switch ports
> or endpoints on the other hand only send messages (that get collected by
> the former). For this reason do not require PCIe switch ports and
> endpoints to use interrupt if they support AER.
> 
> This allows portdrv to attach PCIe switch ports of Intel DG1 and DG2
> discrete graphics cards. These do not declare MSI or legacy interrupts.

Help me understand more about this situation.  I guess we want portdrv
to attach not to a GPU itself, but to a switch port on the card that
*leads* to the GPU?

From the patch, it looks like the only PCIe port service this switch
port advertises is AER (not PME, DPC, hotplug, etc), and it doesn't
have MSI or MSI-X.

So aerdriver should be able to register for PCIE_PORT_SERVICE_AER, but
aer_probe() ignores everything except Root Ports and RCECs.  What's
the benefit then?  I must be missing something.

Bjorn

> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> Changes from v1:
> 
>  * Updated commit message to be more specific on which hardware this is
>    needed.
> 
>  drivers/pci/pcie/portdrv_core.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index 1ac7fec47d6f..1b1c386e50c4 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -164,7 +164,7 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
>   */
>  static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
>  {
> -	int ret, i;
> +	int ret, i, type;
>  
>  	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
>  		irqs[i] = -1;
> @@ -177,6 +177,19 @@ static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
>  	if ((mask & PCIE_PORT_SERVICE_PME) && pcie_pme_no_msi())
>  		goto legacy_irq;
>  
> +	/*
> +	 * Only root ports and event collectors use MSI for errors. Endpoints,
> +	 * switch ports send messages to them but don't use MSI for that (PCIe
> +	 * 5.0 sec 6.2.3.2).
> +	 */
> +	type = pci_pcie_type(dev);
> +	if ((mask & PCIE_PORT_SERVICE_AER) &&
> +	    type != PCI_EXP_TYPE_ROOT_PORT && type != PCI_EXP_TYPE_RC_EC)
> +		mask &= ~PCIE_PORT_SERVICE_AER;
> +
> +	if (!mask)
> +		return 0;
> +
>  	/* Try to use MSI-X or MSI if supported */
>  	if (pcie_port_enable_irq_vec(dev, irqs, mask) == 0)
>  		return 0;
> -- 
> 2.35.1
>