Re: [Qemu-block] [PATCH] atapi: allow 0 transfer bytes for read_cd command

[Hervé Poussineau]

Le 18/08/2016 à 16:24, Kevin Wolf a écrit :
> Hm, which of the paths in cmd_read_cd() does this hit? Is it the one
> that directly calls ide_atapi_cmd_ok() without doing anything?

This is in ide_atapi_cmd, at line:
if (cmd->handler && !(cmd->flags & NONDATA)) {
handler is cmd_read_cd and flags doesn't contain NONDATA and 
atapi_byte_count_limit is 0 and atapi_dma is false, so command is aborted.
Adding NONDATA flag prevents this command abort.

> I think adding NONDATA is okay, but we may need to add explicit
> atapi_byte_count_limit() == 0 checks to those paths that do transfer
> some data. At least at first sight I'm not sure that
> ide_atapi_cmd_read() can handle this.

ATAPI packet is:
ATAPI limit=0x0 packet: be 00 00 00 00 00 00 00 00 00 00 00
Note that byte count limit is 0x0.
I also checked that s->packet_dma is false.

cmd_read_cd calculates nb_sectors using buf[6], buf[7] and buf[8] => nb_sectors 
= 0.
There is a specific case in cmd_read_cd if nb_sectors == 0, which succeeds the 
command.

So, we have four cases:
a) byte limit == 0 && nb_sectors == 0 -> used by NT4, currently is aborting the 
command in ide_atapi_cmd
b) byte limit == 0 && nb_sectors != 0 -> command is aborted in ide_atapi_cmd
c) byte limit != 0 && nb_sectors == 0 -> command succeeds in cmd_read_cd
d) byte limit != 0 && nb_sectors != 0 -> usual case, works fine

Maybe we should add NONDATA flag for cmd_read_cd command, and add on top of 
cmd_read_cd
- if nb_sectors == 0, succeed command (for cases a and c)
- if byte limit == 0 && nb_sectors != 0, abort command (for case b)
- otherwise, process as usual (for case d)

Hervé