Page MenuHomeFreeBSD

asmc: improve asmc_dumpall to read actual SMC key count
ClosedPublic

Authored by guest-seuros on Wed, Dec 31, 3:06 PM.
Tags
None
Referenced Files
F141874199: D54436.diff
Sun, Jan 11, 8:02 PM
Unknown Object (File)
Sat, Jan 10, 5:10 AM
Unknown Object (File)
Sat, Jan 10, 12:50 AM
Unknown Object (File)
Thu, Jan 8, 11:16 AM
Unknown Object (File)
Mon, Jan 5, 10:03 AM
Unknown Object (File)
Mon, Jan 5, 8:22 AM
Unknown Object (File)
Sun, Jan 4, 6:31 AM
Unknown Object (File)
Sat, Jan 3, 11:38 AM
Subscribers
None

Details

Summary

The asmc_dumpall debug function previously used a hardcoded loop limit of 0x100 (256) keys with a "XXX magic number" comment.

This change improves asmc_dumpall to:

  • Read the actual number of keys from the ASMC_NKEYS SMC key
  • Print the key count being dumped for better debugging output
  • Loop only up to the actual key count (e.g., 297 on Mac Mini 5,1)

This provides more accurate debug output and removes the magic number.

Test Plan

Tested on Mac Mini 5,1 (FreeBSD 16.0-CURRENT):

  1. Rebuild kernel with DEBUG enabled in asmc driver
  2. Boot with new kernel
  3. Verify dmesg shows "asmc_dumpall: dumping 297 keys" (or actual count)
  4. Verify all 297 keys are dumped

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

guest-seuros created this revision.
sys/dev/asmc/asmc.c
831

Did you consider reading this value once during driver attach time, and caching the result in the softc?

Honestly no.

I will do it and update.

This code is in debug mode, so i did not consider optimization at all. I just needed the value printed once.

oo neat, lemme go test this on my macbook air devices! thanks!

This looks ok, thanks. Next time please upload patches with more context; see the comment about using -U99999 in https://wiki.freebsd.org/Phabricator#Create_a_Revision

sys/dev/asmc/asmc.c
927

Style nit, there should be a space after * in a type name: ntohl(*(uint32_t *)buf);

Also, it would be better to read the value into a uint32_t variable rather than a potentially unaligned buf.

This revision is now accepted and ready to land.Wed, Dec 31, 5:00 PM
sys/dev/asmc/asmc.c
927

Yeah, I believe it should be something like be32dec(buf) ? That'll avoid the unaligned pointer cast/access and style nit in one hit.

This revision now requires review to proceed.Fri, Jan 2, 7:37 PM
markj added inline comments.
sys/dev/asmc/asmc.c
927

That works too.

This revision is now accepted and ready to land.Fri, Jan 2, 7:49 PM