Verify Command-register BME readback in the existing PCI bus-mastering methods. Return ENXIO for inaccessible configuration space and EIO when the requested state did not take effect. Keep their existing signatures and do not add a transaction wait to ordinary enable or disable calls. Add pci_disable_busmaster_drain() as an explicit, potentially sleeping PCI bus method. Disable and verify bus mastering, then reuse the PCIe Transactions Pending wait with the configured completion timeout and a 10 ms floor. Forward the operation through VGA and host-bridge PCI children so it reaches the function which owns configuration space. Neither operation serializes callers or retains quarantine state. Drivers must prevent concurrent configuration restores or DMA enables and discard stale DMA addresses before restoring bus mastering or allowing another driver to attach. Document when to use ordinary disable versus disable-and-drain, including the limited scope of the readback ordering and non-posted-request drain. Neither is a device reset or a universal posted-DMA drain. MFC after: 2 weeks
Details
Details
Pattern may replace analogs in drivers around things like firmware resets or failed init.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Leave the ordinary PCI configuration and bus-mastering methods unchanged.
Why? Is it out of fear of regressions, or do we legitimately want to ignore errors by default? Existing pci_enable_busmaster()/pci_disable_busmaster() callers already don't check the return values, though today I think these functions can't fail.
Should these be bus methods like the existing functions are?
| share/man/man9/pci.9 | ||
|---|---|---|
| 624 | When should a driver author use this variant over the existing one? The man page should provide some guidance. | |
Comment Actions
Yes probably.
| share/man/man9/pci.9 | ||
|---|---|---|
| 624 | I'm not sure on the implications of changing the KPI.. I would be in favor of changing the existing function to work like pci_enable_busmaster_checked and just ignore or cast away the return as needed. Can a change like that MFC or no? | |
| share/man/man9/pci.9 | ||
|---|---|---|
| 624 | The KPIs are the same: int func(device_t);. All of the existing callers of pci_enable_busmaster() and pci_disable_busmaster() are just ignoring the return value. So as far as I can see, you could just change the existing implementations of pci_enable_busmaster() and pci_disable_busmaster(). As for whether it's a good idea, I'm not sure, but you can try it and see if anything breaks. All else being equal, it'd be better to avoid adding new variants of the existing functions. | |
Comment Actions
Alter existing bme function.
I am thinking disable and drain is different enough from disable with the timeout we might want to keep this separate but not a strongly held opinion.