On Fri, Jun 26, 2015 at 01:31:12PM -0400, John Snow wrote: > On 06/26/2015 11:59 AM, Stefan Hajnoczi wrote: > > On Mon, Jun 22, 2015 at 08:21:13PM -0400, John Snow wrote: > >> @@ -744,8 +722,8 @@ static void ahci_write_fis_pio(AHCIDevice > >> *ad, uint16_t len) pio_fis[9] = s->hob_lcyl; pio_fis[10] = > >> s->hob_hcyl; pio_fis[11] = 0; - pio_fis[12] = cmd_fis[12]; - > >> pio_fis[13] = cmd_fis[13]; + pio_fis[12] = s->nsector & 0xFF; > >> + pio_fis[13] = (s->nsector >> 8) & 0xFF; > > > > hw/ide/core.c decreases s->nsector until it reaches 0 and the > > request ends. > > > > Will the values reported back to the guest be correct if we use > > s->nsector? > > > > See the commit message for justification of this one. Ultimately, it > doesn't matter what gets put in here (for data transfer commands) -- > but getting RID of the cmd_fis mapping is a strong positive.
Getting rid of cmd_fis mapping is good. Putting s->nsector into the undefined fields makes the code confusing. It is clearer to zero the bytes with a comment saying the value does not matter according to the spec.
pgpYusTHXiyM0.pgp
Description: PGP signature
