Date: Sat, 25 Nov 2023 10:30:13 -0800 From: John Baldwin <jhb@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com>, d@delphij.net Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 19f073c612af - main - new-bus: Add resource_validate_map_request function Message-ID: <0e2a051b-d676-4f28-b0c6-60b5409d931e@FreeBSD.org> In-Reply-To: <ZWGy0gKPwYpYqtb6@kib.kiev.ua> References: <202311231707.3ANH758D008755@gitrepo.freebsd.org> <934faa57-60d2-4be7-bdd3-e64c62ae71ad@delphij.net> <ZWGy0gKPwYpYqtb6@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11/25/23 12:39 AM, Konstantin Belousov wrote: > On Fri, Nov 24, 2023 at 10:49:06PM -0800, Xin Li wrote: >> On 2023-11-23 09:07, John Baldwin wrote: >>> The branch main has been updated by jhb: >>> >>> URL: https://cgit.FreeBSD.org/src/commit/?id=19f073c612afa0111d216e5ccab9525bfc97ec32 >>> >>> commit 19f073c612afa0111d216e5ccab9525bfc97ec32 >>> Author: John Baldwin <jhb@FreeBSD.org> >>> AuthorDate: 2023-11-23 17:06:24 +0000 >>> Commit: John Baldwin <jhb@FreeBSD.org> >>> CommitDate: 2023-11-23 17:06:24 +0000 >>> >>> new-bus: Add resource_validate_map_request function >>> This helper function for BUS_MAP_RESOURCE performs common argument >>> validation. >>> Reviewed by: imp >>> Differential Revision: https://reviews.freebsd.org/D42723 >> >> After this change my kernel panics with: >> >> HPTS is in INVARIANT mode!! >> random: entropy device external interface >> kbd0 at kbdmux0 >> WARNING: Device "spkr" is Giant locked and may be deleted before FreeBSD >> 15.0. >> vtvga0: <VT VGA driver> >> aesni0: <AES-CBC,AES-CCM,AES-GCM,AES-ICM,AES-XTS> >> acpi0: <ALASKA A M I > >> acpi0: Power Button (fixed) >> cpu0: <ACPI CPU> on acpi0 >> hpet0: <High Precision Event Timer> iomem 0xfed00000-0xfed003ff on acpi0 >> panic: bus_generic_rman_activate_resource: rman 0xffffffff81222770 doesn't >> match for resource 0xfffff80003d1a400 >> cpuid = 0 >> time = 1 >> KDB: stack backtrace: >> db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame >> 0xffffffff82d5c880 >> vpanic() at vpanic+0x132/frame 0xffffffff82d5c9b0 >> panic() at panic+0x43/frame 0xffffffff82d5ca10 >> bus_generic_rman_activate_resource() at >> bus_generic_rman_activate_resource+0x146/frame 0xffffffff82d5ca70 >> acpi_alloc_sysres() at acpi_alloc_sysres+0x81/frame 0xffffffff82d5cab0 >> acpi_alloc_resource() at acpi_alloc_resource+0x106/frame 0xffffffff82d5cb70 >> bus_alloc_resource() at bus_alloc_resource+0x9b/frame 0xffffffff82d5cbd0 >> hpet_attach() at hpet_attach+0xb4/frame 0xffffffff82d5ccb0 >> device_attach() at device_attach+0x3bc/frame 0xffffffff82d5ccf0 >> device_probe_and_attach() at device_probe_and_attach+0x70/frame >> 0xffffffff82d5cd20 >> bus_generic_attach() at bus_generic_attach+0x18/frame 0xffffffff82d5cd40 >> acpi_probe_children() at acpi_probe_children+0x226/frame 0xffffffff82d5cda0 >> acpi_attach() at acpi_attach+0x97b/frame 0xffffffff82d5ce30 >> device_attach() at device_attach+0x3bc/frame 0xffffffff82d5ce70 >> device_probe_and_attach() at device_probe_and_attach+0x70/frame >> 0xffffffff82d5cea0 >> bus_generic_attach() at bus_generic_attach+0x18/frame 0xffffffff82d5cec0 >> device_attach() at device_attach+0x3bc/frame 0xffffffff82d5cf00 >> device_probe_and_attach() at device_probe_and_attach+0x70/frame >> 0xffffffff82d5cf30 >> bus_generic_new_pass() at bus_generic_new_pass+0xed/frame 0xffffffff82d5cf60 >> bus_set_pass() at bus_set_pass+0x36/frame 0xffffffff82d5cf90 >> configure() at configure+0x9/frame 0xffffffff82d5cfa0 >> mi_startup() at mi_startup+0x1c8/frame 0xffffffff82d5cff0 >> KDB: enter: panic >> [ thread pid 0 tid 100000 ] >> Stopped at kdb_enter+0x32: movq $0,0xa388b3(%rip) >> db> > > Me too. > > But in my case, it is not HPET but atrtc_acpi_attach(). I think I know why this is, and I'll have to disable the assertion for now. Logically speaking, bus devices should not be passing resources they've created form their internal rman's up to a parent device to activate. Instead, when a device allocates a rman such as the sysres rman for ACPI, or the windows for PCI-PCI bridges, it allocates a real resource from the upstream bus and then uses an rman to sub-allocate that resource to child devices. What these bus devices should be doing (eventually) is taking a request to map one of these resources and turning it into a request to map the corresponding window (subset) of the resource allocated from the parent. bus_map/unmap_resource are designed to facilitate that. For now I can quiet the assertion, but I will fix the ACPI bus eventually. This also provides a better way to handle "translated" resources btw as a host bridge that does translation (e.g. of PCI BARs) should allocate the "translated" resource from the parent and use bus_map/unmap to activate regions of that parent resource to use as mappings of the child resources allocated from an internal rman in the bridge driver. However, fixing all that requires having proper bus_map/unmap up in all the nexus drivers which we don't have yet (I have most (all?) of them fixed in a branch that these recent patches came from). I'm not sure why the VM I tested booted didn't trip over this case though as bhyve VMs do have ACPI devices that allocate from sysres ranges. :( -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0e2a051b-d676-4f28-b0c6-60b5409d931e>