e6000sw: Fix locking in miibus_{read,write}reg implementations

[markj]

Summary

Commit 469290648005e13b819a19353032ca53dda4378f made e6000sw's
implementation of miibus_(read|write)reg assume that the softc lock is
held. I presume that is to avoid lock recursion in e6000sw_attach() ->
e6000sw_attach_miibus() -> mii_attach() -> MIIBUS_READREG().

However, the lock assertion in e6000sw_readphy_locked() can fail:

panic: Lock e6000sw not exclusively locked @ /usr/home/markj/src/freebsd/sys/dev/etherswitch/e6000sw/e6000sw.c:773

cpuid = 0 time = 1698599456
KDB: stack backtrace:
db_trace_self() at db_trace_self db_trace_self_wrapper() at db_trace_self_wrapper+0x38
vpanic() at vpanic+0x1a0 panic() at panic+0x48 _sx_assert() at
_sx_assert+0x100 e6000sw_readphy_locked() at e6000sw_readphy_locked+0x40
gentbi_probe() at gentbi_probe+0x7c device_probe_child() at device_probe_child+0x150
device_probe() at device_probe+0xa0
device_probe_and_attach() at device_probe_and_attach+0x38
bus_generic_attach() at bus_generic_attach+0x1c
miibus_attach() at miibus_attach+0x88
device_attach() at device_attach+0x3fc device_probe_and_attach() at
device_probe_and_attach+0x80
bus_generic_driver_added() at bus_generic_driver_added+0x90
devclass_driver_added() at devclass_driver_added+0x48
devclass_add_driver() at devclass_add_driver+0x148
module_register_init() at module_register_init+0xb4
linker_load_module() at linker_load_module+0xacc
kern_kldload() at kern_kldload+0x190
sys_kldload() at sys_kldload+0x64
do_el0_sync() at do_el0_sync+0x59c
handle_el0_sync() at handle_el0_sync+0x48

In particular, gentbi_probe() obviously didn't acquire the softc lock.

Though checking whether the current thread owns the lock is a bit hacky,
I think it's reasonable here and I don't see a better way to handle the
problem.

PR: 274795
Fixes: 469290648005 ("e6000sw: add readphy and writephy wrappers")

[kp]

I'm not sure if it'd be any better, but I suppose we could also make the E6000SW_LOCK a recursive lock, and then just lock unconditionally in read/write phy.

[markj]

In D42466#969240, @kp wrote:

I'm not sure if it'd be any better, but I suppose we could also make the E6000SW_LOCK a recursive lock, and then just lock unconditionally in read/write phy.

That should work too, but given that this is the only code path that needs some introspection (so far), I think it's best to avoid making the lock recursive. It's easier to make locking mistakes with recursive locks, and they make it harder to understand what the code is doing.

[imp]

I agree with markj.

We may have a great sweep through the tree when we get newbus locking and that will change the dynamic here again, I fear. Butter to be explicit about doing the lock/no-lock dance than to make it recursive.

[kp]

In D42466#969452, @imp wrote:

I agree with markj.

To be extra clear here: I'm happy with Mark's proposed solution and do not advocate for the recursive lock option. I just wanted to mention it as 'we could also do this other thing', not 'we must do the other thing'.