IDE CDROM packet command buffer size restriction.

Hello Jens,
I would like to propose another patch to the Linux IDE CD driver. The driver fails any packet command operation in case the amount of data written into the buffer was less than the buflen passed with the packet command. It happens in the interrupt handler ide-cd:cdrom_pc_intr() in these lines:

        /* If DRQ is clear, the command has completed.
           Complain if we still have data left to transfer. */
        if ((stat & DRQ_STAT) == 0) {
                /* ... deleted - pad buffer with 0's for REQUEST_SENSE command */
                if (pc->buflen == 0)
                        cdrom_end_request (1, drive);
                else {
                        /* Comment this out, because this always happens
                           right after a reset occurs, and it is annoying to
                           always print expected stuff.  */
                        /*
                        printk ("%s: cdrom_pc_intr: data underrun %d\n",
                                drive->name, pc->buflen);
                        */
                        pc->stat = 1;
                        cdrom_end_request (1, drive);
                }
                return ide_stopped;

The SCSI driver does no such thing. It does not care if less than pc->buflen amount of data existed. It simply writes to the buffer and returns the whole thing to userland.

We are trying to use the generic packet interface (CDROM_SEND_PACKET) for all CDROM accesses in VMware. The idea is to pass through all ATAPI packets to the host driver. But this strict behaviour of ide-cd driver presents a big problem to using this interface. There is no way to know the amount of data in flight for most packets in PIO mode, other than trying to figure it out from the Allocation Length part of the packets themselves. Unfortunately, the packets routinely issued by Windows OSes have allocation length far greater than the amount of data available from the device. For example, it issues READ_TOC packet (0x43) with allocation length 0x03 0x24, i.e. 804 bytes. In case data CD is in the drive, there are in fact only 20 bytes returned by the device, and in case of audio CD - 124 bytes. Passing cgc->buflen any larger than 20 or 124 bytes, correspondingly, fails that ioctl() call. The READ_TOC packet in question coming from the guest OS is identical. There are similar problems for MODE_SENSE, READ_CD, and many other commands - including multiple variations for all of their parameters. In the DMA case, those "incorrect" buflen's are also the values determined from the scatter gather table, so the situation is the same.c

I don't see a reason why IDE CD driver has to fail the requests in this case, and differ from the SCSI driver. Because ioctl() is flagged as failing, the buffer is then NOT copied into the userland in cdrom.c:cdrom_do_cmd():

        ret = cdi->ops->generic_packet(cdi, cgc);
        __copy_to_user(usense, cgc->sense, sizeof(*usense));
        if (!ret && cgc->data_direction == CGC_DATA_READ)     <== ret is !=0
                __copy_to_user(ubuf, cgc->buffer, cgc->buflen);

Why not just return buflen actually written into the buffer instead? Here is the patch that does just that.
Thank you very much for your review,
Sasha Malchik.

Hi Alan,

Jens seems too busy to check out this patch. I post it here again hopefully to get boarder audiences:

VMware try to use the generic packet interface (CDROM_SEND_PACKET) for cdrom simulation. There are some packet command used by windows can return different data size, depend on the type of CD in the CDROM. Current linux kernel will fail the ioctl call if packet command return data less than expected.

ide-scsi driver do not have this problem.

We make a patch allow kernel return successful and return the actual transfer data size. Is it the prefer behavior in this case? If not, what is the best way to solve this problem?

P.S. I am very surprised to find out that, vmware suffers from bugs in cdrom driver for years. Developers give up after some attempt to submit patches to kernel, it is not easy to make it right at the first time. The broken sense data bug should have been fix long time ago if they try hard enough.

On Thu, Oct 31 2002, [email protected] wrote:

Yes sorry, it's fallen through the cracks.

Quoted 7 lines

VMware try to use the generic packet interface (CDROM_SEND_PACKET) for cdrom simulation. There are some packet command used by windows can return different data size, depend on the type of CD in the CDROM. Current linux kernel will fail the ioctl call if packet command return data less than expected.

Quoted 5 lines

ide-scsi driver do not have this problem.

We make a patch allow kernel return successful and return the actual transfer data size. Is it the prefer behavior in this case? If not, what is the best way to solve this problem?

The patch does look good, thanks.

I can only say resend and resend. It's no secret that I regurlarly loose patches and don't respond to emails, because there are just so many of them.

Quoted 3 lines

We make a patch allow kernel return successful and return the actual transfer data size. Is it the prefer behavior in this case? If not, what is the best way to solve this problem?

The patch does look good, thanks.

Quoted 4 lines · Jens Axboe

Quoted 3 lines

We make a patch allow kernel return successful and return the actual transfer data size. Is it the prefer behavior in this case? If not, what is the best way to solve this problem?

Quoted 1 line

The patch does look good, thanks.

Sasha just find my change to the patch has some fault. The pc.buflen changed after cdrom_queue_packet_command. So his original patch is more correct.

I paste it here again. I am sorry for the confusion.