Hi all, short summary from my side. The whole thing seems to get complicated, let me explain why:
1) During review I found that the code in ide_atapi_cmd_reply_end can't work correctly if the byte_count_limit is not a divider or a multiple of cd_sector_size. The reason is that as soon as we load the next sector we start at io_buffer offset 0 overwriting whatever is left in there for transfer. We also reset the io_buffer_index to 0 which means if we continue with the elementary transfer we always transfer a whole sector (of corrupt data) regardless if we are allowed to transfer that much data. Before we consider fixing this I wonder if it is legal at all to have an unaligned byte_count_limit. It obviously has never caused trouble in practice so maybe its not happening in real life. 2) I found that whatever cool optimization I put in to buffer multiple sectors at once I end up with code that breaks migration because older versions would either not fill the io_buffer as expected or we introduce variables that older versions do not understand. This will lead to problems if we migrate in the middle of a transfer. 3) My current plan to get this patch to a useful state would be to use my initial patch and just change the code to use a sync request if we need to buffer additional sectors in an elementary transfer. I found that in real world operating systems the byte_count_limit seems to be equal to the cd_sector_size. After all its just a PIO transfer an operating system will likely switch to DMA as soon as the kernel ist loaded. Thanks, Peter
