From: Filip Bozuta <Filip.Bozuta@syrmia.com>
To: Leif N Huhn <leif.n.huhn@gmail.com>, qemu-devel@nongnu.org
Subject: Re: [PATCH 0/1] linux-user: Add support for SG_IO and SG_GET_VERSION_NUM raw SCSI ioctls
Date: Fri, 31 Jul 2020 11:40:19 +0200 [thread overview]
Message-ID: <6f40ca09-40e4-6180-2a96-656ca1833d92@syrmia.com> (raw)
In-Reply-To: <20200730025548.237905-1-leif.n.huhn@gmail.com>
Hello Leif,
On 30.7.20. 04:55, Leif N Huhn wrote:
> Hi. This is my first time trying to contribute to qemu. This patch
> works correctly for architectures with the same bit-width, for example
> 32bit arm host and i386 user binary. Here is an example with the sg_simple2
> executable from https://github.com/hreinecke/sg3_utils
>
> 32-bit ARM native:
>
> strace -e trace=ioctl ./sg_simple2 /dev/sg0
> ioctl(3, SG_GET_VERSION_NUM, [30536]) = 0
> ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_FROM_DEV, cmd_len=6, cmdp="\x12\x00\x00\x00\x60\x00", mx_sb_len=32, iovec_count=0, dxfer_len=96, timeout=20000, flags=0, dxferp="\x05\x80\x00\x32\x5b\x00\x00\x00\x48\x4c\x2d\x44\x54\x2d\x53\x54\x42\x44\x2d\x52\x45\x20\x20\x57\x48\x31\x36\x4e\x53\x34\x30\x20"..., status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, driver_status=0, resid=0, duration=3, info=0}) = 0
> Some of the INQUIRY command's results:
> HL-DT-ST BD-RE WH16NS40 1.05 [wide=0 sync=0 cmdque=0 sftre=0]
> ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_NONE, cmd_len=6, cmdp="\x00\x00\x00\x00\x00\x00", mx_sb_len=32, iovec_count=0, dxfer_len=0, timeout=20000, flags=0, status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, driver_status=0, resid=0, duration=4, info=0}) = 0
> Test Unit Ready successful so unit is ready!
> +++ exited with 0 +++
>
> i386 binary on 32-bit arm host:
>
> strace -f -e trace=ioctl qemu/build/i386-linux-user/qemu-i386 sg3_utils/examples/sg_simple2 /dev/sg0
> strace: Process 690 attached
> [pid 689] ioctl(3, SG_GET_VERSION_NUM, [30536]) = 0
> [pid 689] ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_FROM_DEV, cmd_len=6, cmdp="\x12\x00\x00\x00\x60\x00", mx_sb_len=32, iovec_count=0, dxfer_len=96, timeout=20000, flags=0, dxferp="\x05\x80\x00\x32\x5b\x00\x00\x00\x48\x4c\x2d\x44\x54\x2d\x53\x54\x42\x44\x2d\x52\x45\x20\x20\x57\x48\x31\x36\x4e\x53\x34\x30\x20"..., status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, driver_status=0, resid=0, duration=3, info=0}) = 0
> Some of the INQUIRY command's results:
> HL-DT-ST BD-RE WH16NS40 1.05 [wide=0 sync=0 cmdque=0 sftre=0]
> [pid 689] ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_NONE, cmd_len=6, cmdp="\x00\x00\x00\x00\x00\x00", mx_sb_len=32, iovec_count=0, dxfer_len=0, timeout=20000, flags=0, status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, driver_status=0, resid=0, duration=3, info=0}) = 0
> Test Unit Ready successful so unit is ready!
> [pid 690] +++ exited with 0 +++
> +++ exited with 0 +++
>
> However when I try i386 guest on x86_64 host, the cmdp bytes in the
> first SG_IO call are zero, incorrectly. I assume that is because I need
> to write a special ioctl handler. Is that correct? Should I be calling
> lock_user(VERIFY_WRITE...) to copy the buffers over?
Yes I think you need a special ioctl handler for SG_IO. I also tried
your patch by printing the
cmdp as pointer in hex format and noticed that it doesn't change after
each cross execution
through QEMU as it should (which is the case for native execution) so
there is definitely a problem there.
Check out IOCTL_SPECIAL() in file 'ioctls.h'. It enables defining a
special ioctl handling
function which is called specifically for that ioctl in cross execution.
>
> Also, is the current patch acceptable as is, or does it need to be
> reworked until the ioctl works with different architecture bit-widths?
No, patches are usually not accepted until they work for all targets.
There is also a little
issue with SG_GET_VERSION_NUM. I will write you a review in your patch
so you can
see.
By the way, I like the form of your patch description.
Best regards,
Filip
>
> Thanks!
>
> Leif N Huhn (1):
> linux-user: Add support for SG_IO and SG_GET_VERSION_NUM raw SCSI
> ioctls
>
> linux-user/ioctls.h | 2 ++
> linux-user/syscall.c | 1 +
> linux-user/syscall_defs.h | 33 +++++++++++++++++++++++++++++++++
> linux-user/syscall_types.h | 5 +++++
> 4 files changed, 41 insertions(+)
>
prev parent reply other threads:[~2020-07-31 9:56 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-30 2:55 [PATCH 0/1] linux-user: Add support for SG_IO and SG_GET_VERSION_NUM raw SCSI ioctls Leif N Huhn
2020-07-30 2:55 ` [PATCH 1/1] " Leif N Huhn
2020-07-31 10:23 ` Filip Bozuta
2020-07-31 9:40 ` Filip Bozuta [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6f40ca09-40e4-6180-2a96-656ca1833d92@syrmia.com \
--to=filip.bozuta@syrmia.com \
--cc=leif.n.huhn@gmail.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).