Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

[STALE] add SCSI Protocol support #1155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sky5454 wants to merge 12 commits into rust-osdev:main
base: main
Choose a base branch
Loading
from sky5454:main
Draft

Conversation

Copy link
Contributor

@sky5454 sky5454 commented May 1, 2024 •
edited by phip1611
Loading

Steps to Undraft

  • Restart the work (PR ist stale for over 1 year now)

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

Only preliminary support.
some problem I faced:

  • the buffer of I/O data. maybe the best way is to wrap by Reader/Writer trait
    • uefi-raw/src/protocol/scsi.rs: ScsiIoProtocol::execute_scsi_command()
    • uefi/src/proto/scsi.rs: ExtScsiPassThru::pass_thru()
    • wrap and make the ScsiIoScsiRequestPacket/ExtScsiIoScsiRequestPacket to be conveniently used in Rust
  • The dev path which using the Device Path Protocol
  • TARGET_MAX_BYTES unused, for: The Target is an array of size TARGET_MAX_BYTES and it represents the id of the SCSI device to send the SCSI Request Packet.
  • The naming rule of enum.
  • The data sync with c and rust ffi, vec <-> c data ptr and len.
#[derive(Debug)]
#[repr(C)]
pub struct ScsiIoScsiRequestPacket {
 pub timeout: u64,
 /// DataBuffer: A pointer to the data buffer to transfer from or to the SCSI device.
 /// - InDataBuffer: A pointer to the data buffer to transfer between the SCSI controller and the SCSI device for SCSI READ command. For all SCSI WRITE Commands this must point to NULL .
 /// - OutDataBuffer: A pointer to the data buffer to transfer between the SCSI controller and the SCSI device for SCSI WRITE command. For all SCSI READ commands this field must point to NULL .
 
 pub in_data_buffer: *mut c_void,
 pub out_data_buffer: *mut c_void,
 pub sense_data: *mut c_void,
 pub cdb: *mut c_void,
 pub in_transfer_length: u32,
 pub out_transfer_length: u32,
 pub cdb_length: u8,
 pub data_direction: DataDirection,
 pub host_adapter_status: HostAdapterStatus,
 pub target_status: TargetStatus,
 pub sense_data_length: u8,
}
#[derive(Debug)]
#[repr(C)]
pub struct ExtScsiIoScsiRequestPacket {
 pub timeout: u64,
 pub in_data_buffer: *mut c_void,
 pub out_data_buffer: *mut c_void,
 pub sense_data: *mut c_void,
 pub cdb: *mut c_void,
 pub in_transfer_length: u32,
 pub out_transfer_length: u32,
 pub cdb_length: u8,
 pub data_direction: ExtDataDirection,
 pub host_adapter_status: ExtHostAdapterStatus,
 pub target_status: ExtTargetStatus,
 pub sense_data_length: u8,
}

After read "SPC" which docs from T10 (SCSI Standard Groups), I Know that in_data_buffer,out_data_buffer,sense_data,cdb have variable length. so I decide to make a struct like

 pub timeout: u64,
 /// data_buffer is in_data_buffer or out_data_buffer
 pub data_buffer: Vec<u8>
 pub cdb: Vec<u8>
 pub sense_data: Vec<u8>
 pub data_direction: ExtDataDirection,
 pub host_adapter_status: ExtHostAdapterStatus,
 pub target_status: ExtTargetStatus,

but data couldn't share between FFI and c. That's a problem. We should share those data by ptr.

according to uefi scsi usage from edk2:

sky5454 added 2 commits May 4, 2024 20:38 
...w `execute_scsi_command` send req packet is ok.
**PASSED**:
 - SCSI_IO all api, SCSI_THRU `mode()` and `reset()` api, are test passed.
**PROBLEM**:
 1. panic at allocator.rs while test_scsi_io return;
 2. still problem with `DevicePath`,`target_id`,`pass_thru`;
**UNKNOWN**:
 - scsi req ok but resp without test at `execute_scsi_command()`.
Copy link
Contributor Author

sky5454 commented May 5, 2024 •
edited
Loading

SCSI_IO

The main part of SCSI_IO is finished. though something need some optimization.

  • data_buf maybe need in and out at the sametime.?
  • resp no test still.

Running at VMWare with two SCSI sample TestUnitReady and InquiryCommand:
image

EXT_SCSI_THRU

only test mode() and reset() passed.
some trouble.

  • DevicePath/DevicePathProtocol
  • Target ids
  • the packet sample about scsi thru. I couldn't find it clearly.

image

@sky5454 sky5454 marked this pull request as ready for review May 11, 2024 03:04
Copy link
Contributor Author

sky5454 commented May 11, 2024 •
edited
Loading

  • Now UEFI SCSI I/O protocol is ready.
  • And UEFI Scsi Thru I/O protocol Will not be published in this PR. I would continue to code it at branch sky5454:dev-scsi

image

after you review the code and resolve the panic, you could merge it.

Copy link
Member

Before I review fully, it would help to do some cleanup:

  • Fix CI errors (run formatting, fix lints, etc)
  • Remove all unused code
  • Resolve TODOs if possible

This will make review easier :)
phip1611 and sky5454 reacted with thumbs up emoji

Copy link
Contributor Author

sky5454 commented Dec 29, 2024

sry, has no time to cleanup it for my tired life, just review/reedit and merge it plz.

Copy link
Member

The split-out from #1517 took care of the uefi-raw part. Someone needs to rebase it and do the remaining work for uefi (high-level wrapper)

@phip1611 phip1611 changed the title (削除) add SCSI Protocol support (削除ここまで) (追記) [STALE] add SCSI Protocol support (追記ここまで) Jun 22, 2025
@phip1611 phip1611 marked this pull request as draft June 22, 2025 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicholasbishop nicholasbishop Awaiting requested review from nicholasbishop

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /