Re: [Qemu-devel] [PATCH 16/16] ahci: fix sdb fis semantics

[John Snow]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 06/26/2015 12:11 PM, Stefan Hajnoczi wrote:
> On Mon, Jun 22, 2015 at 08:21:15PM -0400, John Snow wrote:
>> @@ -682,19 +680,22 @@ static void ahci_write_fis_sdb(AHCIState
>> *s, int port, uint32_t finished)
>> 
>> sdb_fis->type = SATA_FIS_TYPE_SDB; /* Interrupt pending &
>> Notification bit */ -    sdb_fis->flags =
>> (ad->hba->control_regs.irqstatus ? (1 << 6) : 0); +
>> sdb_fis->flags = 0x40; /* Interrupt bit, always 1 for NCQ */
> ...
>> -    ahci_trigger_irq(s, ad, PORT_IRQ_SDB_FIS); +    if
>> (sdb_fis->flags & 0x40) { +        ahci_trigger_irq(s, ad,
>> PORT_IRQ_SDB_FIS); +    }
> 
> This if statement is always true.
> 

Yes. I did that on purpose. Maybe that was too weird.

The idea was to emphasize that the IRQ is definitely only triggered
*IF* the interrupt bit is set. It just so happens that we currently
always set it.

The generation and trigger mechanics are supposed to be separate, but
we currently have no use case for them being separate (because we are
an omniscient HBA-HDD amalgamation), so they're mushed together here.

This is kind of like a documentation "NB."

I can replace it with:

/* Trigger IRQ if interrupt bit is set, which currently it always is: */
ahci_trigger_irq(...);

or just add that comment to the existing structure, etc.

(Or if you really prefer, just the naked IRQ trigger, but I feel
that's kind of misleading.)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVjY2XAAoJEH3vgQaq/DkOl60P/j7dpKYPHobr+R8YP2ujWsc0
SinxemsCsgdvUXfTK5ma1yYYccHc0y5f7Ug617U4+/Tr7hK/gMhpQS0sLh+aTh1k
DZTo30kqvXsDXIkw6dUjqBJRAPg0bm3RSB+YjqJxOH8o6nyntOmSzaJrxU6Rd6Oj
AJFNtXPmlk3RLiU6uRD/EK7K+HaoRmvcvga6aGfNX4YJgM3+NUUUqiK6Urjok0sR
qRvoYEkqvsJqujMAyHxlZuzW50PVcvEcWMyBXYaO2bvQWG/e/1SueQ9fLO8emdiF
HQEyeCts4fN5VsZqZBdp+68PUqcRm6BtDodh5pPdPOe8OUc3l9Hu4tkTFqbe+iXM
Em6+G9umMo0jPIepztYr56uXwYdLK0VGc2JQ91CqE0OXEmL+qh0+8xTg0e4Ix4Ff
z7M9g90Lvuq9yLRAGtwjBKO+NDdgGXszc40W64yEtChqgt2GNOfLUIUbx45tVU4O
vmddjNVyKE432AcMDC1+Je4uIKHrXwQGHO6ZdncrCvCeAKaDhZF77gpuy7fvga9v
/I51zZVuxCNXEGPSuLYCE7ynA0vNBzQ1zgcEcxTM/2/odFBmgACY2nMSlgFy2b5x
qpTQsTyIis8/mi6sdaX/l3FZ1i/pXbuoUzmrTxOfOmcjFJjS7n5Qb7Yj6aX64sHn
/vESog1pKwxvzwyPmejk
=OYom
-----END PGP SIGNATURE-----