[RFC,0/2] Add a test to verify device probing on ACPI platforms

Message ID 20230925155806.1812249-1-laura.nao@collabora.com
Headers
Series Add a test to verify device probing on ACPI platforms |

Message

Laura Nao Sept. 25, 2023, 3:58 p.m. UTC
  Regressions that prevent a driver from probing a device can significantly
affect the functionality of a platform.

A kselftest to verify if devices on a DT-based platform are probed
correctly was recently introduced [1], but no such generic test is
available for ACPI platforms yet. bootrr [2]  provides device probe
testing, but relies on a pre-defined list of the peripherals present on
each DUT.

On ACPI based hardware, a complete description of the platform is
provided to the OS by the system firmware. ACPI namespace objects are
mapped by the Linux ACPI subsystem into a device tree in
/sys/devices/LNXSYSTEM:00; the information in this subtree can be parsed
to build a list of the hw peripherals present on the DUT dynamically.

This series adds a test to verify if the devices declared in the ACPI
namespace and supported by the kernel are probed correctly.

This work follows a similar approach to [1], adapted for the ACPI use
case.

The first patch introduces a script that builds a list of all ACPI device
IDs supported by the kernel, by inspecting the acpi_device_id structs in
the sources. This list can be used to avoid testing ACPI-enumerated
devices that don't have a matching driver in the kernel. This script was
highly inspired by the dt-extract-compatibles script [3].

In the second patch, a new kselftest is added. It parses the
/sys/devices/LNXSYSTEM:00 tree to obtain a list of all platform
peripherals and verifies which of those, if supported, are correctly
bound to a driver.

Feedback is much appreciated,

Thank you,

Laura

[1] https://lore.kernel.org/all/20230828211424.2964562-1-nfraprado@collabora.com/
[2] https://github.com/kernelci/bootr
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/dtc/dt-extract-compatibles

Laura Nao (2):
  acpi: Add script to extract ACPI device ids in the kernel
  kselftest: Add test to detect unprobed devices on ACPI platforms

 MAINTAINERS                                   |  2 +
 scripts/acpi/acpi-extract-ids                 | 60 +++++++++++++++
 tools/testing/selftests/Makefile              |  1 +
 tools/testing/selftests/acpi/.gitignore       |  2 +
 tools/testing/selftests/acpi/Makefile         | 23 ++++++
 .../selftests/acpi/test_unprobed_devices.sh   | 75 +++++++++++++++++++
 6 files changed, 163 insertions(+)
 create mode 100755 scripts/acpi/acpi-extract-ids
 create mode 100644 tools/testing/selftests/acpi/.gitignore
 create mode 100644 tools/testing/selftests/acpi/Makefile
 create mode 100755 tools/testing/selftests/acpi/test_unprobed_devices.sh
  

Comments

Laura Nao Oct. 26, 2023, 10 a.m. UTC | #1
Gentle ping to check if there are any feedback or comments on this series.

Thanks,
Laura
  
Dan Carpenter Nov. 23, 2023, 7:42 a.m. UTC | #2
Your talk was interesting at Linux Plumbers.

https://www.youtube.com/watch?v=oE73eVSyFXQ [time +2:35]

This is probably a stupid question, but why not just add something to
call_driver_probe() which creates a sysfs directory tree with all the
driver information?

regards,
dan carpenter
  
Laura Nao Nov. 23, 2023, 12:09 p.m. UTC | #3
> Your talk was interesting at Linux Plumbers.
> 
> https://www.youtube.com/watch?v=oE73eVSyFXQ [time +2:35]
> 
> This is probably a stupid question, but why not just add something to
> call_driver_probe() which creates a sysfs directory tree with all the
> driver information?
> 

Thanks for the feedback! 

Improving the device driver model to publish driver and devices info was indeed another option we considered. We could have a debugfs entry storing this kind of information, similar to what devices_deferred does and in a standardized format. This would provide an interface that is easier to query at runtime for getting a list of devices that were probed correctly.
This would cover devices with a driver that's built into the kernel or as a module; in view of catching also those cases where a device is not probed because the relevant config is not enabled, I think we'd still need another way of building a list of devices present on the platform to be used as reference.

The solution proposed in this RFC follows the same approach used for dt
based platforms for simplicity. But if adding a new sysfs entry storing devices and driver info proves to be a viable option for upstream, we can surely explore it and improve the probe test to leverage that.

Best,

Laura
  
Dan Carpenter Nov. 23, 2023, 3:14 p.m. UTC | #4
On Thu, Nov 23, 2023 at 01:09:42PM +0100, Laura Nao wrote:
> > Your talk was interesting at Linux Plumbers.
> > 
> > https://www.youtube.com/watch?v=oE73eVSyFXQ [time +2:35]
> > 
> > This is probably a stupid question, but why not just add something to
> > call_driver_probe() which creates a sysfs directory tree with all the
> > driver information?
> > 
> 
> Thanks for the feedback! 
> 
> Improving the device driver model to publish driver and devices info
> was indeed another option we considered. We could have a debugfs entry
> storing this kind of information, similar to what devices_deferred
> does and in a standardized format. This would provide an interface
> that is easier to query at runtime for getting a list of devices that
> were probed correctly.
> This would cover devices with a driver that's built into the kernel or
> as a module; in view of catching also those cases where a device is
> not probed because the relevant config is not enabled, I think we'd
> still need another way of building a list of devices present on the
> platform to be used as reference.

Yeah.  So we'd still need patch #1 as-is and but patch #2 would probably
be simpler if we had this information in sysfs.  Or a different solution
would be to do what someone said in the LPC talk and just save the
output of the previous boot and complain if there was a regression where
something didn't probe.

> 
> The solution proposed in this RFC follows the same approach used for
> dt based platforms for simplicity. But if adding a new sysfs entry
> storing devices and driver info proves to be a viable option for
> upstream, we can surely explore it and improve the probe test to
> leverage that.

You're saying "simplicity" but I think you mean easiest from a political
point of view.  It's not the most simple format at all.  It's like
massive detective work to find the information and then you'll have to
redo it for DT and for USB.  Are there other kinds of devices which can
be probed?

I feel like you're not valuing your stuff at the right level.  This
shouldn't be in debugfs.  It should be a first class citizen in sysfs.

The exact format for this information is slightly tricky and people will
probably debate that.  But I think most people will agree that it's
super useful.

regards,
dan carpenter
  
Laura Nao Nov. 24, 2023, 4:04 p.m. UTC | #5
On 11/23/23 16:14, Dan Carpenter wrote:
> On Thu, Nov 23, 2023 at 01:09:42PM +0100, Laura Nao wrote:
>>> Your talk was interesting at Linux Plumbers.
>>>
>>> https://www.youtube.com/watch?v=oE73eVSyFXQ [time +2:35]
>>>
>>> This is probably a stupid question, but why not just add something to
>>> call_driver_probe() which creates a sysfs directory tree with all the
>>> driver information?
>>>
>>
>> Thanks for the feedback!
>>
>> Improving the device driver model to publish driver and devices info
>> was indeed another option we considered. We could have a debugfs entry
>> storing this kind of information, similar to what devices_deferred
>> does and in a standardized format. This would provide an interface
>> that is easier to query at runtime for getting a list of devices that
>> were probed correctly.
>> This would cover devices with a driver that's built into the kernel or
>> as a module; in view of catching also those cases where a device is
>> not probed because the relevant config is not enabled, I think we'd
>> still need another way of building a list of devices present on the
>> platform to be used as reference.
> 
> Yeah.  So we'd still need patch #1 as-is and but patch #2 would probably
> be simpler if we had this information in sysfs.  Or a different solution
> would be to do what someone said in the LPC talk and just save the
> output of the previous boot and complain if there was a regression where
> something didn't probe.
> 

Right. The main drawback of using the status of a known good boot as
reference is to keep it up to date over time. If support for a
peripheral gets added at a later stage, the reference needs to be updated
as well.

>>
>> The solution proposed in this RFC follows the same approach used for
>> dt based platforms for simplicity. But if adding a new sysfs entry
>> storing devices and driver info proves to be a viable option for
>> upstream, we can surely explore it and improve the probe test to
>> leverage that.
> 
> You're saying "simplicity" but I think you mean easiest from a political
> point of view.  It's not the most simple format at all.  It's like
> massive detective work to find the information and then you'll have to
> redo it for DT and for USB.  Are there other kinds of devices which can
> be probed?
> 

Yeah, that's what I meant. The ACPI use case is in a way simpler to
handle than the dt one, as we can get information on non removable
devices on enumerable buses such as PCI from the ACPI
tables (leveraging the _ADR objects). But it still requires quite a lot
digging in sysfs to get info on what was actually probed.
So having a list of probed devices would help both use cases.

> I feel like you're not valuing your stuff at the right level.  This
> shouldn't be in debugfs.  It should be a first class citizen in sysfs.
> 
> The exact format for this information is slightly tricky and people will
> probably debate that.  But I think most people will agree that it's
> super useful.
>

Right, agreeing on a format will be tricky. Judging by the response here
and in LPC it's still worth a shot though. I'll put some thought into
this and experiment a bit to come up with a proposal to submit in
another RFC.

Again, thanks for the helpful feedback!

Best,
Laura