Re: [Qemu-devel] [PATCH 14/16] ahci: Do not map cmd_fis to generate response

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]