asmc: fix asmc_key_dump() page fault on T2 MMIO backend

[guest-seuros]

Summary

asmc_key_dump() used I/O port macros (ASMC_DATAPORT_WRITE/READ,
asmc_command()) unconditionally. On T2 Macs, sc_ioport is NULL
(MMIO backend is used instead), causing a page fault when
ASMC_DEBUG triggers asmc_dumpall() during attach.

Add an MMIO guard at the top of asmc_key_dump(): delegate to
asmc_key_dump_by_index() + asmc_key_read() for MMIO devices,
consistent with the rest of the T2 code paths.

[ngie]

This hints at needing the patches/driver available in https://github.com/t2linux/linux-t2-patches/tree/main (please note that the driver isn't technically GPLv2; I can add more details later if needed).

sys/dev/asmc/asmc.c

1156–1157

Same thing; just a bit more explicit.

1161–1165

Doesn't have to be done here or now, but if this is a point of contention around data flushing, I/O interleaving, etc, it would probably be a good idea to consider using the sbuf family of functions instead of the stdio ones (like *printf).

1206

Was this breaking the type representation somehow, or was it doing implicit formatting with the type names?

[ngie]

Add an MMIO guard at the top of asmc_key_dump(): delegate to
asmc_key_dump_by_index() + asmc_key_read() for MMIO devices,
consistent with the rest of the T2 code paths.

Having a more explicit statement for T2 Macs vs non-T2 Macs would be a good idea.

[guest-seuros]

Address review: add CMDGETINFO response layout comment explaining type[0] is data length, type[1..4] is ASCII type string

[guest-seuros]

This comment was removed by guest-seuros.

[guest-seuros]

@ngie , the separation of the T2 code will be done in a future diff.

[adrian]

this LOOKS right to me. @ngie any other comments?

[ngie]

@guest-seuros : could you please reply to my comments?

[guest-seuros]

@ngie, I got fooled by the reply mechanism again. my response was sitting unsubmitted.

For the first two (MIN + sbuf): updating now. I wasnt aware of sbuf, makes sense for kernel printf.

For the type representation (L1151): that's just the CMDGETINFO response layout, byte 0 is data length, bytes 1-4 are the ASCII type string. The I/O port path extracts the length then overwrites type[0] with a space to reuse the buffer for display. The MMIO path doesn't need this hack because asmc_key_dump_by_index() returns the type string separately. I noticed this when debugging Apple with the T2 chip.

[guest-seuros]

use helpers (MIN + sbuf)

[adrian]

ok, this looks better. ngie@, anything else?

[ngie]

sys/dev/asmc/asmc.c

1155

Ah yeah, MIN not MAX :D.

1156

Is there a reason why you removed the error check?

1224

The old way conforms to style(9).

1234

Same as before.

[guest-seuros]

sys/dev/asmc/asmc.c

1156

The key was just enumerated successfully, so a read failure right after would be a hardware fault.

I originally added the check while validating and understanding the SMC behavior itself.

Unless the Apple Trashcan or Cheese Grater models have some especially exotic SMC quirk, I tested this across basically the entire Intel Mac lineup from 2006 to 2019.

[ngie]

sys/dev/asmc/asmc.c

1156

While I agree with you, a bit of extra care is required (IMHO) because there's a fair bit of FUD around whether or not some operations could brick SMCs. Your proposed change is no doubt harmless, but it doesn't hurt to err on the side of caution to make sure the data is sane/correct (especially since logic errors result in kernel panics, which results in extended reboot times).

[adrian]

wait, we're still missing the sc_is_mmio bits, so I think seuros has to find those diffs and add them to this stack.