From nobody Fri Jan 10 17:13:21 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4YV7XV3GL1z5kgGH; Fri, 10 Jan 2025 17:13:30 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4YV7XT64RKz466d; Fri, 10 Jan 2025 17:13:29 +0000 (UTC) (envelope-from kostikbel@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: from tom.home (kib@localhost [127.0.0.1] (may be forged)) by kib.kiev.ua (8.18.1/8.18.1) with ESMTP id 50AHDMga082024; Fri, 10 Jan 2025 19:13:25 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 50AHDMga082024 Received: (from kostik@localhost) by tom.home (8.18.1/8.18.1/Submit) id 50AHDL7U082023; Fri, 10 Jan 2025 19:13:21 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 10 Jan 2025 19:13:21 +0200 From: Konstantin Belousov To: John Baldwin Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: ccabc7c2e556 - main - DEVICE_IDENTIFY.9: Modernize description and use cases Message-ID: References: <202501092020.509KKt1U058876@gitrepo.freebsd.org> <9acd1878-2ee3-47c8-aab9-29d5be200081@FreeBSD.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9acd1878-2ee3-47c8-aab9-29d5be200081@FreeBSD.org> X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=4.0.1 X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on tom.home X-Rspamd-Queue-Id: 4YV7XT64RKz466d X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:6939, ipnet:2001:470::/32, country:US] On Fri, Jan 10, 2025 at 07:45:19AM -0500, John Baldwin 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 > > > AuthorDate: 2025-01-09 20:20:16 +0000 > > > Commit: John Baldwin > > > 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. It is other way around I suppose, bus "bar" and driver "foo". Thank you for answering, would you consider adding this (adapted) text to the man page? > > 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. For DMAR the current situation with being the child of acpi0 is fine, apparently. But for AMD IOMMU, there is a serious problem there, at least from my current PoV. Issue is, AMD IOMMUs are exposed as the normal PCI functions just having a funny PCIe capability, that points to the MMIO register window (not BAR). And the issue is that IOMMUs are enumerated too late to properly intercept interesting events during other devices' attach. I did set the early pass for the iommu driver EARLY_DRIVER_MODULE(amdiommu, pci, amdiommu_driver, 0, 0, BUS_PASS_SUPPORTDEV); but as I understand, it only regulates the identity pass over the same bus. What the driver needs is being able to create devices before any other devices (like HPET or IOAPICs) which could request interrupts, are enumerated and attached.