Re: git: ccabc7c2e556 - main - DEVICE_IDENTIFY.9: Modernize description and use cases
Date: Fri, 10 Jan 2025 14:05:12 UTC
On Fri, Jan 10, 2025 at 5:45 AM John Baldwin <jhb@freebsd.org> wrote: > 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. > These are likely OK. It's arguably possible to model this as devices known in other places, but it gets super awkward and you'd have to do hand-stands that are worse in other ways. > 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. > This is bogus, imho, but not worth fixing. We should have a isasmbb device that parses the child, creates a isasmb bus and then adds the children it finds from parsing the smbtable and moves on. It would be much cleaner. There's several other devices that kinda live here, but none worth supporting these days, so a rewrite is a waste of time. > 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. > This is definitely wrong in a big way. I have about 80-90% of the conversion to be proper children of cpu nodes, as appropriate. Warner > 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 > >