Skip to content

ledctl: add AMD SGPIO slot management support - #288

Open
sergeimonakhov wants to merge 1 commit into
md-raid-utilities:mainfrom
sergeimonakhov:amd-sgpio-slots
Open

sergeimonakhov wants to merge 1 commit into
md-raid-utilities:mainfrom
sergeimonakhov:amd-sgpio-slots

Conversation

@sergeimonakhov

Copy link
Copy Markdown

Add AMD SGPIO support to ledctl slot management

This adds AMD SGPIO bays to the existing --list-slots, --get-slot, and --set-slot API. AMD slots are now enumerated from AHCI SGPIO EM buffers instead of attached block devices, so empty SATA bays can be located even after the disk has disappeared from Linux.

The implementation reuses the existing AMD SGPIO register encoding/write path used by locate=/dev/sdX, preserving the existing device-based behavior.

Tested on AMD FCH SATA AHCI [1022:7901]:

  • --list-slots --controller-type AMD lists empty and populated bays
  • --set-slot --controller-type AMD --slot ata5-bay3 --state locate works for an empty bay
  • --get-slot reports the updated state

@github-actions

Copy link
Copy Markdown

Hey @nfont, a file(s) you own were modified in this PR! Could you take a look?


Caused by:

@mtkaczyk mtkaczyk mentioned this pull request Oct 14, 2026
@mtkaczyk

Copy link
Copy Markdown
Member

Hey @sergeimonakhov

This is actually very good to me. Nice job! Please fix those minor checkpatch issues:

WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
Error: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one


ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)
Error: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)

I have no experience with AMD therefore I need to wait for @nfont ack before it is merged.

@mtkaczyk

Copy link
Copy Markdown
Member

AI findings:

[P2] Validate port_no before using it as a drive-bay index. amd_sgpio.c:588-598 rejects only -1, then derives drive_bay without checking that the port is in the expected range 1..8. The new slot-state callback uses that value directly at cache->leds[amd_slot->drive_bay]; the set callback also passes it into the LED-array update. For example, port 9 produces bay -1, and port 0 produces bay 4, leading to out-of-bounds access and possible memory corruption when sysfs reports an unexpected value.

Validation against malformed sysfs entry. Since catched now, please add it as second commit, thanks!

[P3] Free a slot if list-node allocation fails. amd_sgpio.c:784-790 allocates a slot and passes it to list_append_ctx. If the list-node allocation fails, list.c:77-80 records an error but does not free the item, leaking the slot allocation. Handle the failed insertion by releasing the slot.

Indeed, we should free slot in that case.

Expose AMD SGPIO slots in ledctl with state read and write support.
Validate port numbers, handle allocation failures, and keep blink
rates consistent between writes and state reporting.

Signed-off-by: Sergei Monakhov <sergey.monakhov@gmail.com>
@sergeimonakhov

Copy link
Copy Markdown
Author

@mtkaczyk Thanks! I squashed the changes into one commit and fixed the reported issue. I also corrected blink-pattern state detection and TX/CFG generator selection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants