On Tue, Jun 18, 2024 at 12:33:05PM -0700, Doug Anderson wrote: > Hi, > > On Tue, Jun 18, 2024 at 8:59 AM Daniel Thompson > <daniel.thomp...@linaro.org> wrote: > > > > On Mon, Jun 17, 2024 at 05:34:47PM -0700, Douglas Anderson wrote: > > > Add commands that are like the other "md" commands but that allow you > > > to read memory that's in the IO space. > > > > > > Signed-off-by: Douglas Anderson <diand...@chromium.org> > > > > Sorry to be the bearer of bad news but... > > > > > > > --- > > > <snip> > > > +/* > > > + * kdb_getioword > > > + * Inputs: > > > + * word Pointer to the word to receive the result. > > > + * addr Address of the area to copy. > > > + * size Size of the area. > > > + * Returns: > > > + * 0 for success, < 0 for error. > > > + */ > > > +int kdb_getioword(unsigned long *word, unsigned long addr, size_t size) > > > +{ > > > + void __iomem *mapped = ioremap(addr, size); > > > > ioremap() is a might_sleep() function. It's unsafe to call it from the > > debug trap handler. > > Hmmmm. Do you have a pointer to documentation or code showing that > it's a might_sleep() function. I was worried about this initially but > I couldn't find any documentation or code indicating it to be so. I > also got no warnings when I ran my prototype code and then the code > worked fine, so I assumed that it must somehow work. Sigh... > > Looking more closely, maybe this is: > > ioremap() -> ioremap_prot() -> generic_ioremap_prot() -> > __get_vm_area_caller() -> __get_vm_area_node() -> __get_vm_area_node() > > ...and that has a kernel allocation with GFP_KERNEL?
To be honest there were a lot of problems, I just simplified. __get_vm_area_node() already commences with a BUG_ON(in_interrupt()) before it ends up doing a GFP_KERNEL memory allocation. I think there are multiple calls to might_sleep() (for example from __get_vm_area_node() -> alloc_vmap_area() ). However even if we had preallocated some vmap addresses for peek/poke there are still problems in ioremap_page_range() too. For example: generic_ioremap_prot() -> ioremap_page_range() -> find_vm_area() -> find_vmap_area() -> spin_lock() We could go further down the rabbit hole and pre-lookup the VM area too but then we hit. generic_ioremap_prot() -> ioremap_page_range() -> vmap_page_range() -> vmap_range_noflush() -> might_sleep() It is remotely possible that the only lock vmap_page_range() takes is init_mm->page_table_lock but I doubt we can be sure of that. > I guess it also then calls alloc_vmap_area() which has might_sleep()... > > I'll have to track down why no warnings triggered... > > > I'm afraid I don't know a safe alternative either. Machinary such as > > kmap_atomic() needs a page and iomem won't have one. > > Ugh. It would be nice to come up with something since it's not > uncommon to need to look at the state of hardware registers when a > crash happens. In the past I've managed to get into gdb, track down a > global variable where someone has already mapped the memory, and then > use gdb to look at the memory. It's always a big pain, though... > > ...even if I could just look up the virtual address where someone else > had already mapped it that might be enough. At least I wouldn't need > to go track down the globals myself... > > ...anyway, I guess I'll ponder on it and poke if I have time... I've often thought about implementing longjmp-on-spin-wait for kgdb for these kind of reasons. For example I have long wanted to be able to let the user see /proc/interrupts before the usespace comes up but the spin locks get in the way. This approach wouldn't make calling ioremap() safe (since we could end up bailing out halfway through a non-atomic operation) but it could at least give control back to kdb and let the user know they have ruined their system. I know... there is a *reason* I've never quite got round to writing this! Daniel. _______________________________________________ Kgdb-bugreport mailing list Kgdb-bugreport@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport