Re: [PATCH 3/4 RFC] fsl/msi: Add MSI bank allocation for kernel owned devices

On Thu, 2015-03-12 at 10:46 -0500, Bhushan Bharat-R65777 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Thursday, March 12, 2015 4:53 AM
To: Bhushan Bharat-R65777
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 3/4 RFC] fsl/msi: Add MSI bank allocation for kernel
owned devices

With the patchset as is, how would one indicate whether kernel devices
should get a bank?

For kernel owned device, in fsl_setup_msi_irqs() we check if there is
a reserved MSI bank, if not then reserve a msi bank. If reserve fails
then setup_msi_irq() fails. I think there is no fallback to Legacy
interrupt.

If enabling MSI fails, it's up to the driver to fall back to legacy
interrupts (grep drivers for pci_enable_msi for examples).

quoted

 Specifically, when the kernel does have an MSI-
capable device but we'd prefer to use legacy interrupts to keep MSIs
available to VFIO.

Do we want this?

You'd previously raised concern about wanting to give all MSI banks to
VFIO.  The kernel might still have PCIe devices that are not
performance-critical.  That said, I'm not going to nack the patchset if
it requires the kernel to allocate a bank for itself.

quoted

quoted

@@ -231,15 +304,12 @@ static int fsl_setup_msi_irqs(struct pci_dev

*pdev, int nvec, int type)

quoted

 		if (specific_msi_bank) {
 			hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
 		} else {
-			/*
-			 * Loop over all the MSI devices until we find one that

has an

quoted

-			 * available interrupt.
-			 */
-			list_for_each_entry(msi_data, &msi_head, list) {
-				hwirq = msi_bitmap_alloc_hwirqs(&msi_data-
bitmap, 1);
-				if (hwirq >= 0)
-					break;
+			msi_data = fsl_msi_get_reserved_msi_bank(pdev);
+			if (!msi_data) {
+				dev_err(&pdev->dev, "No MSI Bank allocated\n");
+				goto out_free;

Is this really an error?  Seems like dev_info() would be more appropriate
to indicate the absence of a resource where you can fall back to another
option.

There is no fallback in fsl_setup_msi_irqs(), We have to error out from fsl_setup_msi_irqs(), no?

I meant as far as the user is concerned, not whether you return an error from the function.

-Scott