Re: git: ccabc7c2e556 - main - DEVICE_IDENTIFY.9: Modernize description and use cases

On 1/9/25 18:01, Konstantin Belousov wrote:
> On Thu, Jan 09, 2025 at 08:20:55PM +0000, John Baldwin wrote:
>> The branch main has been updated by jhb:
>>
>> URL: https://cgit.FreeBSD.org/src/commit/?id=ccabc7c2e556ac0b14da9b682b706ccaf251c0fe
>>
>> commit ccabc7c2e556ac0b14da9b682b706ccaf251c0fe
>> Author:     John Baldwin <jhb@FreeBSD.org>
>> AuthorDate: 2025-01-09 20:20:16 +0000
>> Commit:     John Baldwin <jhb@FreeBSD.org>
>> CommitDate: 2025-01-09 20:20:16 +0000
>>
>>      DEVICE_IDENTIFY.9: Modernize description and use cases
>>      
>>      Mention adding devices based on firmware tables and software-only
>>      pseudo-devices as use cases for identify methods as those are more
>>      common than reading random I/O ports to identify a legacy ISA device.
>>      
>>      Describe how device_find_chid can be used to avoid duplicates.  While
>>      here, explicitly note that devices added in identify methods typically
>>      use a fixed device name.
>>      
>>      Trim the cross-references a bit.
>>      
>>      Reviewed by:    ziaee, imp
>>      Differential Revision:  https://reviews.freebsd.org/D48367
>> ---
>>   share/man/man9/DEVICE_IDENTIFY.9 | 52 +++++++++++++++++++---------------------
>>   1 file changed, 25 insertions(+), 27 deletions(-)
>>
>> diff --git a/share/man/man9/DEVICE_IDENTIFY.9 b/share/man/man9/DEVICE_IDENTIFY.9
>> index d75c1a91ce4a..b10d94143050 100644
>> --- a/share/man/man9/DEVICE_IDENTIFY.9
>> +++ b/share/man/man9/DEVICE_IDENTIFY.9
>> @@ -26,44 +26,46 @@
>>   .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>>   .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>   .\"
>> -.Dd January 15, 2017
>> +.Dd January 9, 2025
>>   .Dt DEVICE_IDENTIFY 9
>>   .Os
>>   .Sh NAME
>>   .Nm DEVICE_IDENTIFY
>> -.Nd identify a device, register it
>> +.Nd identify new child devices and register them
>>   .Sh SYNOPSIS
>>   .In sys/param.h
>>   .In sys/bus.h
>>   .Ft void
>>   .Fn DEVICE_IDENTIFY "driver_t *driver" "device_t parent"
> So what is the 'parent' for driver which creates devices based on the
> firmware tables?

Hmmm, I could maybe try to clarify this further.  In new-bus, drivers are
associated with a parent bus devclass.  All of the drivers associated with
a given parent bus are then eligible for use with children of any bus
devices for that bus.  Thus, for example:

DRIVER_MODULE(foo, bar, ....)

Associates the "bar" driver with the bus "foo".  For any fooX bus devices,
the "bar" driver can attach to children of fooX.

Most device_if.m methods operate on "child" devices, so device_probe,
device_attach, device_detach, etc. all operate on a given barX device
that is a child of a fooX.

device_identify is different.  Instead, each fooX bus device can call
bus_identify_children (formerly the somewhat misnamed bus_generic_probe)
during the fooX device_attach routine.  bus_identify_children looks for
the devclass of foo and then walks all of the eligible device drivers
for potential children of foo invoking this method with fooX as the
parent.  The idea being that device_identify will create "barX" children
of "fooX" explicitly using BUS_ADD_CHILD.

In terms of which parent device, it's really about where a given "barX"
device should live.  For system-wide "top-level" devices that aren't
behind some other bridge on an ACPI system, the pattern we use is to
hang those devices as children of acpi0, so you end up with

DRIVER_MODULE(acpi, bar, ...)

and the parent device at the time of DEVICE_IDENTIFY is acpi0.  However,
we also use identify in some other places.  nexus0 children all tend to
either be explicitly added in nexus_attach() or added via identify
routines that use nexus as the parent.  legacy0 on x86 also uses identify
routines to add child devices like pcibX.

Another case is the ipmi(4) device.  Legacy ISA IPMI devices are described
by an entry in the SMBIOS table.  The ipmi(4) driver uses an identify
routine to add an ipmi0 device as a child of isa0 (since it has I/O ports
like a typical ISA device), but the identify routine is table-driven since
it depends on parsing the smbios table.

cpufreqX is another odd case, and I'm not quite convinced it is correct.
Today we enumerate cpuX devices hung off of some nexus-like device (on
x86 cpuX are children of either acpi0 or legacy0).  Each cpufreqX
driver then uses identify routines to add named children (p4tccX, estX,
hwpstateX, etc.).  Those identify routines all have "cpu" as the parent
bus so that cpuX is the parent device (and they are called for each
instance of a cpuX device).  For cpufreq I feel like we actually want
something a bit different on x86 at least.  I think we want to create
an explicit cpufreqX device in cpu_attach, and that the various cpufreq
drivers that manage frequency should all be "cpufreq" drivers that bid
to attach to that device node instead of creating duplicate nodes that
try to duplicate work.  Today the various identify routines try to
check for each other instead which is fragile.  It may be that we'd
need/want two device_t nodes, one for P-states and one for throttling,
though it might be that we only want the throttling for certain P-state
drivers or some such.

For your IOMMU case, you can use an ACPI table "anywhere" in the device
tree to enumerate device nodes if necessary (though PCI buses don't
currently call bus_identify_children today and don't have a BUS_ADD_CHILD
method), but if you want to be "ready" before other generic children like
PCI bridges are attached, you probably need to be a child of acpi0.

-- 
John Baldwin