On 06/22/2015 10:06 AM, Stefan Hajnoczi wrote: > On Fri, Jun 19, 2015 at 09:50:35PM -0400, John Snow wrote: >> @@ -999,20 +1000,28 @@ static void process_ncq_command(AHCIState >> *s, int port, uint8_t *cmd_fis, ((uint64_t)ncq_fis->lba2 << 16) >> | ((uint64_t)ncq_fis->lba1 << 8) | (uint64_t)ncq_fis->lba0; + >> ncq_tfs->tag = tag; >> >> - /* Note: We calculate the sector count, but don't currently >> rely on it. - * The total size of the DMA buffer tells us the >> transfer size instead. */ ncq_tfs->sector_count = >> ((uint16_t)ncq_fis->sector_count_high << 8) | >> ncq_fis->sector_count_low; + ahci_populate_sglist(ad, >> &ncq_tfs->sglist, 0); + size = ncq_tfs->sector_count * 512; > > ncq_tfs->sector_count is used with - 2 and - 1 below. What is the > semantics of this field and why is it okay to use it without > subtracting here? >
Just a bonkers patch order. Patch #7 fixes the debug prints, which are wrong. The field is not "off by one" in the spec, sector_count == 1 means 1 sector. (sector_count == 0 means 64K sectors, though, like it does in regular ATA.)