Skip to content

Commit bb8e97e

Browse files
Carl Vanderlipquic-jhugo
authored andcommitted
accel/qaic: Enable 1 MSI fallback mode
Several virtualization use-cases either don't support 32 MultiMSIs (Xen/VMware) or have significant drawbacks to their use (KVM's vIOMMU, which is required to support 32 MSI, needs to allocate an alternate system memory space for each device using vIOMMU (e.g. 8GB VM mem and 2 cards => 8 + 2 * 8 = 24GB host memory required)). Support these cases by enabling a 1 MSI fallback mode. Whenever all 32 MSIs requested are not available, a second request for a single MSI is made. Its success is the initiator of single MSI mode. This mode causes all interrupts generated by the device to be directed to the 0th MSI (firmware >=v1.10 will do this as a response to the PCIe MSI capability configuration). Likewise, all interrupt handlers for the device are registered to the 0th MSI. Since the DBC interrupt handler checks if the DBC is in use or if there is any pending changes, the 'spurious' interrupts are disregarded. If there is work to be done, the standard threaded IRQ handler is dispatched. On every interrupt, the MHI handler wakes up its threaded interrupt handler, and attempts to wake any waiters for MHI state events. Performance is within +-0.6% for test cases that typify real world use. Larger differences ([-4,+132]%, avg +47%) exist for very simple tasks (e.g. addition) compiled for single NSPs. It is assumed that the small work and many interrupts typically cause contention (e.g. 16 NSPs vs 4 CPUs), as evidenced by the standard deviation between runs also decreasing (r=-0.48 between delta(Performace_test) and delta(StdDev_test/Avg_test)) Signed-off-by: Carl Vanderlip <[email protected]> Reviewed-by: Pranjal Ramajor Asha Kanojiya <[email protected]> Reviewed-by: Jeffrey Hugo <[email protected]> Signed-off-by: Jeffrey Hugo <[email protected]> Reviewed-by: Stanislaw Gruszka <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 88b02eb commit bb8e97e

File tree

7 files changed

+76
-22
lines changed

7 files changed

+76
-22
lines changed

Documentation/accel/qaic/aic100.rst

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ AIC100 DID (0xa100).
3636

3737
AIC100 does not implement FLR (function level reset).
3838

39-
AIC100 implements MSI but does not implement MSI-X. AIC100 requires 17 MSIs to
40-
operate (1 for MHI, 16 for the DMA Bridge).
39+
AIC100 implements MSI but does not implement MSI-X. AIC100 prefers 17 MSIs to
40+
operate (1 for MHI, 16 for the DMA Bridge). Falling back to 1 MSI is possible in
41+
scenarios where reserving 32 MSIs isn't feasible.
4142

4243
As a PCIe device, AIC100 utilizes BARs to provide host interfaces to the device
4344
hardware. AIC100 provides 3, 64-bit BARs.

Documentation/accel/qaic/qaic.rst

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ accelerator products.
1010
Interrupts
1111
==========
1212

13+
IRQ Storm Mitigation
14+
--------------------
15+
1316
While the AIC100 DMA Bridge hardware implements an IRQ storm mitigation
1417
mechanism, it is still possible for an IRQ storm to occur. A storm can happen
1518
if the workload is particularly quick, and the host is responsive. If the host
@@ -35,6 +38,26 @@ generates 100k IRQs per second (per /proc/interrupts) is reduced to roughly 64
3538
IRQs over 5 minutes while keeping the host system stable, and having the same
3639
workload throughput performance (within run to run noise variation).
3740

41+
Single MSI Mode
42+
---------------
43+
44+
MultiMSI is not well supported on all systems; virtualized ones even less so
45+
(circa 2023). Between hypervisors masking the PCIe MSI capability structure to
46+
large memory requirements for vIOMMUs (required for supporting MultiMSI), it is
47+
useful to be able to fall back to a single MSI when needed.
48+
49+
To support this fallback, we allow the case where only one MSI is able to be
50+
allocated, and share that one MSI between MHI and the DBCs. The device detects
51+
when only one MSI has been configured and directs the interrupts for the DBCs
52+
to the interrupt normally used for MHI. Unfortunately this means that the
53+
interrupt handlers for every DBC and MHI wake up for every interrupt that
54+
arrives; however, the DBC threaded irq handlers only are started when work to be
55+
done is detected (MHI will always start its threaded handler).
56+
57+
If the DBC is configured to force MSI interrupts, this can circumvent the
58+
software IRQ storm mitigation mentioned above. Since the MSI is shared it is
59+
never disabled, allowing each new entry to the FIFO to trigger a new interrupt.
60+
3861

3962
Neural Network Control (NNC) Protocol
4063
=====================================

drivers/accel/qaic/mhi_controller.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ static int mhi_reset_and_async_power_up(struct mhi_controller *mhi_cntrl)
468468
}
469469

470470
struct mhi_controller *qaic_mhi_register_controller(struct pci_dev *pci_dev, void __iomem *mhi_bar,
471-
int mhi_irq)
471+
int mhi_irq, bool shared_msi)
472472
{
473473
struct mhi_controller *mhi_cntrl;
474474
int ret;
@@ -500,6 +500,10 @@ struct mhi_controller *qaic_mhi_register_controller(struct pci_dev *pci_dev, voi
500500
return ERR_PTR(-ENOMEM);
501501

502502
mhi_cntrl->irq[0] = mhi_irq;
503+
504+
if (shared_msi) /* MSI shared with data path, no IRQF_NO_SUSPEND */
505+
mhi_cntrl->irq_flags = IRQF_SHARED;
506+
503507
mhi_cntrl->fw_image = "qcom/aic100/sbl.bin";
504508

505509
/* use latest configured timeout */

drivers/accel/qaic/mhi_controller.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#define MHICONTROLLERQAIC_H_
99

1010
struct mhi_controller *qaic_mhi_register_controller(struct pci_dev *pci_dev, void __iomem *mhi_bar,
11-
int mhi_irq);
11+
int mhi_irq, bool shared_msi);
1212
void qaic_mhi_free_controller(struct mhi_controller *mhi_cntrl, bool link_up);
1313
void qaic_mhi_start_reset(struct mhi_controller *mhi_cntrl);
1414
void qaic_mhi_reset_done(struct mhi_controller *mhi_cntrl);

drivers/accel/qaic/qaic.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ struct qaic_device {
123123
struct srcu_struct dev_lock;
124124
/* true: Device under reset; false: Device not under reset */
125125
bool in_reset;
126+
/* true: single MSI is used to operate device */
127+
bool single_msi;
126128
/*
127129
* true: A tx MHI transaction has failed and a rx buffer is still queued
128130
* in control device. Such a buffer is considered lost rx buffer

drivers/accel/qaic/qaic_data.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1466,6 +1466,16 @@ irqreturn_t dbc_irq_handler(int irq, void *data)
14661466

14671467
rcu_id = srcu_read_lock(&dbc->ch_lock);
14681468

1469+
if (datapath_polling) {
1470+
srcu_read_unlock(&dbc->ch_lock, rcu_id);
1471+
/*
1472+
* Normally datapath_polling will not have irqs enabled, but
1473+
* when running with only one MSI the interrupt is shared with
1474+
* MHI so it cannot be disabled. Return ASAP instead.
1475+
*/
1476+
return IRQ_HANDLED;
1477+
}
1478+
14691479
if (!dbc->usr) {
14701480
srcu_read_unlock(&dbc->ch_lock, rcu_id);
14711481
return IRQ_HANDLED;
@@ -1488,7 +1498,8 @@ irqreturn_t dbc_irq_handler(int irq, void *data)
14881498
return IRQ_NONE;
14891499
}
14901500

1491-
disable_irq_nosync(irq);
1501+
if (!dbc->qdev->single_msi)
1502+
disable_irq_nosync(irq);
14921503
srcu_read_unlock(&dbc->ch_lock, rcu_id);
14931504
return IRQ_WAKE_THREAD;
14941505
}
@@ -1559,12 +1570,12 @@ irqreturn_t dbc_irq_threaded_fn(int irq, void *data)
15591570
u32 tail;
15601571

15611572
rcu_id = srcu_read_lock(&dbc->ch_lock);
1573+
qdev = dbc->qdev;
15621574

15631575
head = readl(dbc->dbc_base + RSPHP_OFF);
15641576
if (head == U32_MAX) /* PCI link error */
15651577
goto error_out;
15661578

1567-
qdev = dbc->qdev;
15681579
read_fifo:
15691580

15701581
if (!event_count) {
@@ -1645,14 +1656,14 @@ irqreturn_t dbc_irq_threaded_fn(int irq, void *data)
16451656
goto read_fifo;
16461657

16471658
normal_out:
1648-
if (likely(!datapath_polling))
1659+
if (!qdev->single_msi && likely(!datapath_polling))
16491660
enable_irq(irq);
1650-
else
1661+
else if (unlikely(datapath_polling))
16511662
schedule_work(&dbc->poll_work);
16521663
/* checking the fifo and enabling irqs is a race, missed event check */
16531664
tail = readl(dbc->dbc_base + RSPTP_OFF);
16541665
if (tail != U32_MAX && head != tail) {
1655-
if (likely(!datapath_polling))
1666+
if (!qdev->single_msi && likely(!datapath_polling))
16561667
disable_irq_nosync(irq);
16571668
goto read_fifo;
16581669
}
@@ -1661,9 +1672,9 @@ irqreturn_t dbc_irq_threaded_fn(int irq, void *data)
16611672

16621673
error_out:
16631674
srcu_read_unlock(&dbc->ch_lock, rcu_id);
1664-
if (likely(!datapath_polling))
1675+
if (!qdev->single_msi && likely(!datapath_polling))
16651676
enable_irq(irq);
1666-
else
1677+
else if (unlikely(datapath_polling))
16671678
schedule_work(&dbc->poll_work);
16681679

16691680
return IRQ_HANDLED;

drivers/accel/qaic/qaic_drv.c

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -424,30 +424,42 @@ static int init_msi(struct qaic_device *qdev, struct pci_dev *pdev)
424424
int i;
425425

426426
/* Managed release since we use pcim_enable_device */
427-
ret = pci_alloc_irq_vectors(pdev, 1, 32, PCI_IRQ_MSI);
428-
if (ret < 0)
429-
return ret;
427+
ret = pci_alloc_irq_vectors(pdev, 32, 32, PCI_IRQ_MSI);
428+
if (ret == -ENOSPC) {
429+
ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
430+
if (ret < 0)
431+
return ret;
430432

431-
if (ret < 32) {
432-
pci_err(pdev, "%s: Requested 32 MSIs. Obtained %d MSIs which is less than the 32 required.\n",
433-
__func__, ret);
434-
return -ENODEV;
433+
/*
434+
* Operate in one MSI mode. All interrupts will be directed to
435+
* MSI0; every interrupt will wake up all the interrupt handlers
436+
* (MHI and DBC[0-15]). Since the interrupt is now shared, it is
437+
* not disabled during DBC threaded handler, but only one thread
438+
* will be allowed to run per DBC, so while it can be
439+
* interrupted, it shouldn't race with itself.
440+
*/
441+
qdev->single_msi = true;
442+
pci_info(pdev, "Allocating 32 MSIs failed, operating in 1 MSI mode. Performance may be impacted.\n");
443+
} else if (ret < 0) {
444+
return ret;
435445
}
436446

437447
mhi_irq = pci_irq_vector(pdev, 0);
438448
if (mhi_irq < 0)
439449
return mhi_irq;
440450

441451
for (i = 0; i < qdev->num_dbc; ++i) {
442-
ret = devm_request_threaded_irq(&pdev->dev, pci_irq_vector(pdev, i + 1),
452+
ret = devm_request_threaded_irq(&pdev->dev,
453+
pci_irq_vector(pdev, qdev->single_msi ? 0 : i + 1),
443454
dbc_irq_handler, dbc_irq_threaded_fn, IRQF_SHARED,
444455
"qaic_dbc", &qdev->dbc[i]);
445456
if (ret)
446457
return ret;
447458

448459
if (datapath_polling) {
449-
qdev->dbc[i].irq = pci_irq_vector(pdev, i + 1);
450-
disable_irq_nosync(qdev->dbc[i].irq);
460+
qdev->dbc[i].irq = pci_irq_vector(pdev, qdev->single_msi ? 0 : i + 1);
461+
if (!qdev->single_msi)
462+
disable_irq_nosync(qdev->dbc[i].irq);
451463
INIT_WORK(&qdev->dbc[i].poll_work, irq_polling_work);
452464
}
453465
}
@@ -479,7 +491,8 @@ static int qaic_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
479491
goto cleanup_qdev;
480492
}
481493

482-
qdev->mhi_cntrl = qaic_mhi_register_controller(pdev, qdev->bar_0, mhi_irq);
494+
qdev->mhi_cntrl = qaic_mhi_register_controller(pdev, qdev->bar_0, mhi_irq,
495+
qdev->single_msi);
483496
if (IS_ERR(qdev->mhi_cntrl)) {
484497
ret = PTR_ERR(qdev->mhi_cntrl);
485498
goto cleanup_qdev;

0 commit comments

Comments
 (0)