Re: [Qemu-devel] [PATCH 04/16] ahci: check for ncq prdtl overflow

[John Snow]

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.)