* [PATCH] PCI/DOE: Poll briefly for a response before sleeping
@ 2026-09-17 23:03 Michal Clapinski
2026-09-18 14:27 ` Lukas Wunner
0 siblings, 1 reply; 5+ messages in thread
From: Michal Clapinski @ 2026-09-17 23:03 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci
Cc: Lukas Wunner, Jonathan Cameron, Ira Weiny, Dan Williams,
linux-kernel, Michal Clapinski
doe_statemachine_work() waits for a response by sleeping
PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128) between status reads,
which is 7 ms at HZ=1000 and 4 ms at HZ=250. Exchanges are served from
the device's local mailbox logic and typically complete within tens of
microseconds, so nearly all of that sleep is wasted. Every DOE-capable
endpoint pays it once per supported feature during enumeration.
Poll with usleep_range(20, 50) for the first ~2 ms before falling back
to the jiffies-based wait. On an Intel Emerald Rapids system, time spent
in pci_device_add() for a DOE-capable endpoint drops from 41.8 ms to
14.0 ms.
Signed-off-by: Michal Clapinski <mclapinski@google.com>
---
drivers/pci/doe.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
index ac95b1d2d999..6dc85133a88e 100644
--- a/drivers/pci/doe.c
+++ b/drivers/pci/doe.c
@@ -27,6 +27,9 @@
/* Timeout of 1 second from 6.30.2 Operation, PCI Spec r6.0 */
#define PCI_DOE_TIMEOUT HZ
#define PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128)
+#define PCI_DOE_POLL_FAST_MIN_US 20
+#define PCI_DOE_POLL_FAST_MAX_US 50
+#define PCI_DOE_POLL_FAST_TRIES 40
#define PCI_DOE_FLAG_CANCEL 0
#define PCI_DOE_FLAG_DEAD 1
@@ -486,7 +489,7 @@ static void doe_statemachine_work(struct work_struct *work)
int offset = doe_mb->cap_offset;
unsigned long timeout_jiffies;
u32 val;
- int rc;
+ int rc, tries = 0;
if (test_bit(PCI_DOE_FLAG_DEAD, &doe_mb->flags)) {
signal_task_complete(task, -EIO);
@@ -524,10 +527,25 @@ static void doe_statemachine_work(struct work_struct *work)
signal_task_abort(task, -EIO);
return;
}
- rc = pci_doe_wait(doe_mb, PCI_DOE_POLL_INTERVAL);
- if (rc) {
- signal_task_abort(task, rc);
- return;
+ /*
+ * Exchanges are served from the device's local mailbox
+ * logic and usually complete in tens of microseconds, far
+ * below the jiffies granularity of PCI_DOE_POLL_INTERVAL.
+ * Poll for up to ~2 ms before falling back to it.
+ */
+ if (tries++ < PCI_DOE_POLL_FAST_TRIES) {
+ if (test_bit(PCI_DOE_FLAG_CANCEL, &doe_mb->flags)) {
+ signal_task_abort(task, -EIO);
+ return;
+ }
+ usleep_range(PCI_DOE_POLL_FAST_MIN_US,
+ PCI_DOE_POLL_FAST_MAX_US);
+ } else {
+ rc = pci_doe_wait(doe_mb, PCI_DOE_POLL_INTERVAL);
+ if (rc) {
+ signal_task_abort(task, rc);
+ return;
+ }
}
goto retry_resp;
}
--
2.55.0.1082.g2b9226bbc0-goog
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] PCI/DOE: Poll briefly for a response before sleeping
2026-09-17 23:03 [PATCH] PCI/DOE: Poll briefly for a response before sleeping Michal Clapinski
@ 2026-09-18 14:27 ` Lukas Wunner
2026-09-18 19:28 ` Michał Cłapiński
0 siblings, 1 reply; 5+ messages in thread
From: Lukas Wunner @ 2026-09-18 14:27 UTC (permalink / raw)
To: Michal Clapinski; +Cc: Bjorn Helgaas, linux-pci, Jonathan Cameron, linux-kernel
On Fri, Sep 18, 2026 at 01:03:23AM +0200, Michal Clapinski wrote:
> doe_statemachine_work() waits for a response by sleeping
> PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128) between status reads,
> which is 7 ms at HZ=1000 and 4 ms at HZ=250. Exchanges are served from
> the device's local mailbox logic and typically complete within tens of
> microseconds, so nearly all of that sleep is wasted. Every DOE-capable
> endpoint pays it once per supported feature during enumeration.
>
> Poll with usleep_range(20, 50) for the first ~2 ms before falling back
> to the jiffies-based wait. On an Intel Emerald Rapids system, time spent
> in pci_device_add() for a DOE-capable endpoint drops from 41.8 ms to
> 14.0 ms.
Polling is just a poor man's replacement for interrupts. DOE supports
interrupt-driven transfers but we're not taking advantage of it so far.
Perhaps you could look into adding that? It would seem more worthwhile
than optimizing polling.
Thanks,
Lukas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI/DOE: Poll briefly for a response before sleeping
2026-09-18 14:27 ` Lukas Wunner
@ 2026-09-18 19:28 ` Michał Cłapiński
2026-10-15 20:01 ` Bjorn Helgaas
0 siblings, 1 reply; 5+ messages in thread
From: Michał Cłapiński @ 2026-09-18 19:28 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Bjorn Helgaas, linux-pci, linux-kernel
On Fri, Sep 18, 2026 at 7:27 AM Lukas Wunner <lukas@wunner.de> wrote:
>
> On Fri, Sep 18, 2026 at 01:03:23AM +0200, Michal Clapinski wrote:
> > doe_statemachine_work() waits for a response by sleeping
> > PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128) between status reads,
> > which is 7 ms at HZ=1000 and 4 ms at HZ=250. Exchanges are served from
> > the device's local mailbox logic and typically complete within tens of
> > microseconds, so nearly all of that sleep is wasted. Every DOE-capable
> > endpoint pays it once per supported feature during enumeration.
> >
> > Poll with usleep_range(20, 50) for the first ~2 ms before falling back
> > to the jiffies-based wait. On an Intel Emerald Rapids system, time spent
> > in pci_device_add() for a DOE-capable endpoint drops from 41.8 ms to
> > 14.0 ms.
>
> Polling is just a poor man's replacement for interrupts. DOE supports
> interrupt-driven transfers but we're not taking advantage of it so far.
>
> Perhaps you could look into adding that? It would seem more worthwhile
> than optimizing polling.
Thanks for the quick response.
This code runs during PCI device enumeration, before a driver is bound, so
the core doesn't really have an interrupt to work with: MSI/MSI-X vectors
belong to the driver, and enabling them in the core would collide with its
later pci_alloc_irq_vectors(); INTx isn't routed yet either, since dev->irq
only becomes valid via acpi_pci_irq_enable() at pci_enable_device() time.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI/DOE: Poll briefly for a response before sleeping
2026-09-18 19:28 ` Michał Cłapiński
@ 2026-10-15 20:01 ` Bjorn Helgaas
2026-10-16 4:19 ` Alistair
0 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2026-10-15 20:01 UTC (permalink / raw)
To: Michał Cłapiński
Cc: Lukas Wunner, Bjorn Helgaas, linux-pci, linux-kernel,
Jonathan Cameron, Alistair Francis
[+cc Jonathan, Alistair, thread at
https://lore.kernel.org/all/20260917230323.3701743-1-mclapinski@google.com]
On Fri, Sep 18, 2026 at 12:28:21PM -0700, Michał Cłapiński wrote:
> On Fri, Sep 18, 2026 at 7:27 AM Lukas Wunner <lukas@wunner.de> wrote:
> >
> > On Fri, Sep 18, 2026 at 01:03:23AM +0200, Michal Clapinski wrote:
> > > doe_statemachine_work() waits for a response by sleeping
> > > PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128) between status reads,
> > > which is 7 ms at HZ=1000 and 4 ms at HZ=250. Exchanges are served from
> > > the device's local mailbox logic and typically complete within tens of
> > > microseconds, so nearly all of that sleep is wasted. Every DOE-capable
> > > endpoint pays it once per supported feature during enumeration.
> > >
> > > Poll with usleep_range(20, 50) for the first ~2 ms before falling back
> > > to the jiffies-based wait. On an Intel Emerald Rapids system, time spent
> > > in pci_device_add() for a DOE-capable endpoint drops from 41.8 ms to
> > > 14.0 ms.
> >
> > Polling is just a poor man's replacement for interrupts. DOE supports
> > interrupt-driven transfers but we're not taking advantage of it so far.
> >
> > Perhaps you could look into adding that? It would seem more worthwhile
> > than optimizing polling.
>
> Thanks for the quick response.
>
> This code runs during PCI device enumeration, before a driver is
> bound, so the core doesn't really have an interrupt to work with:
> MSI/MSI-X vectors belong to the driver, and enabling them in the
> core would collide with its later pci_alloc_irq_vectors(); INTx
> isn't routed yet either, since dev->irq only becomes valid via
> acpi_pci_irq_enable() at pci_enable_device() time.
Do we have to run the DOE setup code during enumeration at all? Maybe
we can wait until a user that needs DOE functionality comes along?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI/DOE: Poll briefly for a response before sleeping
2026-10-15 20:01 ` Bjorn Helgaas
@ 2026-10-16 4:19 ` Alistair
0 siblings, 0 replies; 5+ messages in thread
From: Alistair @ 2026-10-16 4:19 UTC (permalink / raw)
To: Bjorn Helgaas, Michał Cłapiński
Cc: Lukas Wunner, Bjorn Helgaas, linux-pci, linux-kernel, Jonathan Cameron
On Thu, 2026-10-15 at 15:01 -0500, Bjorn Helgaas wrote:
> [+cc Jonathan, Alistair, thread at
> https://lore.kernel.org/all/20260917230323.3701743-1-mclapinski@google.com
> ]
>
> On Fri, Sep 18, 2026 at 12:28:21PM -0700, Michał Cłapiński wrote:
> > On Fri, Sep 18, 2026 at 7:27 AM Lukas Wunner <lukas@wunner.de>
> > wrote:
> > >
> > > On Fri, Sep 18, 2026 at 01:03:23AM +0200, Michal Clapinski wrote:
> > > > doe_statemachine_work() waits for a response by sleeping
> > > > PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128) between status
> > > > reads,
> > > > which is 7 ms at HZ=1000 and 4 ms at HZ=250. Exchanges are
> > > > served from
> > > > the device's local mailbox logic and typically complete within
> > > > tens of
> > > > microseconds, so nearly all of that sleep is wasted. Every DOE-
> > > > capable
> > > > endpoint pays it once per supported feature during enumeration.
> > > >
> > > > Poll with usleep_range(20, 50) for the first ~2 ms before
> > > > falling back
> > > > to the jiffies-based wait. On an Intel Emerald Rapids system,
> > > > time spent
> > > > in pci_device_add() for a DOE-capable endpoint drops from 41.8
> > > > ms to
> > > > 14.0 ms.
> > >
> > > Polling is just a poor man's replacement for interrupts. DOE
> > > supports
> > > interrupt-driven transfers but we're not taking advantage of it
> > > so far.
> > >
> > > Perhaps you could look into adding that? It would seem more
> > > worthwhile
> > > than optimizing polling.
> >
> > Thanks for the quick response.
> >
> > This code runs during PCI device enumeration, before a driver is
> > bound, so the core doesn't really have an interrupt to work with:
> > MSI/MSI-X vectors belong to the driver, and enabling them in the
> > core would collide with its later pci_alloc_irq_vectors(); INTx
> > isn't routed yet either, since dev->irq only becomes valid via
> > acpi_pci_irq_enable() at pci_enable_device() time.
>
> Do we have to run the DOE setup code during enumeration at all?
Yeah, I think we do.
We need to probe DOE to know if the device supports CMA, at which point
we need to attest and authenticate the device.
Granted that isn't upstream yet, but in general we will need DOE at
enumeration.
> Maybe
> we can wait until a user that needs DOE functionality comes along?
We don't know who that user is until we know the DOE features is the
issue
Alistair
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-16 4:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-17 23:03 [PATCH] PCI/DOE: Poll briefly for a response before sleeping Michal Clapinski
2026-09-18 14:27 ` Lukas Wunner
2026-09-18 19:28 ` Michał Cłapiński
2026-10-15 20:01 ` Bjorn Helgaas
2026-10-16 4:19 ` Alistair
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®