Am 21.08.2016 um 23:16 hat Hervé Poussineau geschrieben:
> 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)
Yes, for the part about nb_sectors, this sounds about right.
I see annother immediate ide_atapi_cmd_ok() in the switch for
(transfer_request & 0xf8 == 0). I think this needs to be considered in
the check as well.
Kevin
