Re: [Qemu-devel] [PATCH 12/16] ahci: ncq migration

[John Snow <jsnow@redhat.com>]

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



On 06/26/2015 11:48 AM, Stefan Hajnoczi wrote:
> On Mon, Jun 22, 2015 at 08:21:11PM -0400, John Snow wrote:
>> @@ -1555,6 +1573,35 @@ static int ahci_state_post_load(void
>> *opaque, int version_id) return -1; }
>> 
>> +        for (j = 0; j < AHCI_MAX_CMDS; j++) { +
>> ncq_tfs = &ad->ncq_tfs[j]; +            ncq_tfs->drive = ad; + +
>> if (ncq_tfs->used != ncq_tfs->halt) { +                return
>> -1; +            } +            if (!ncq_tfs->halt) { +
>> continue; +            } +            if (!is_ncq(ncq_tfs->cmd))
>> { +                return -1; +            } +            if
>> (ncq_tfs->slot != ncq_tfs->tag) { +                return -1; +
>> } +            if (ncq_tfs->slot > AHCI_MAX_CMDS) { +
>> return -1; +            } +            ncq_tfs->cmdh =
>> &((AHCICmdHdr *)ad->lst)[ncq_tfs->slot];
> 
> Is there a guarantee that ->lst has been mapped?  Maybe pr->cmd & 
> PORT_CMD_START was 0.
> 
> We need to check that the HBA is in a valid state for NCQ
> processing before attempting this.
> 

Slipped my mind, there should be no way to have ncq_tfs->halt set
under normal circumstances except if the engine is started (and lst is
mapped.)

That said, stream corruption is always possible, so I'll add another
validation check.

An "is not null" check here should be sufficient, due to the "if not
halt" continue up above.

Thanks,
- --js
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVjYHPAAoJEH3vgQaq/DkOkOsP/2O1bJJWaSAUVGB168GLQKGY
B4ffiV37g4JrrLdhVeRuJiHYZmHT31VwBYqX2ZfxFJWqOBsei3PUm6qN7OjKyUQr
td/oAaT3TKou8qySfbbgXMN1ubodtXU8CHOYykfNXiRqMeuW1CZUvP6IvEqJhZrj
nu5H5l+W1lzsgEt2MCbK6VN96l3LMpYp8EafbZV1tOiMW63U8Tdc2IVJD1UP8krv
U5ngLUegs5FCfHXYMBoIamDXDux52iE06EboP0J+nS21jNadtm/akJ2qq7ndKLVJ
vghtlI+9Teq8bKq8JjY9qQY9tAe3FCIqygquwCOcQPKsi7FxnkB396k3sY9BckFK
uBNMOBo6b0+5S4NnS7KV/L3986StepoStxBsEJj89EOi6AidDJocIs67m3TkOb5T
WGbFQsnzXi+MLJw4Ck583DbwZ92fkdUNKEGlZtbvZ3UxpJ0qNos8hNPenewGlmQO
BrcgTrD0f+Hr9GrLA+ACXgUah1I5giSsAYyjX3REwzrhuMEYMAv9vfBszrdMW/KP
VZiLNl763YcjdK4EMC72R/R7+pf25RyTA8w4yAcc8mvTR+fDIuTtgCrHo8MEhIfS
91UC72zgmVhOWq2JelVR0sPKWGC4dvWLs/r0gfTOlthjzx5twnbSG2/XhrWPuJmd
MCTuNL5CiuRCdjfq9hLB
=XQh5
-----END PGP SIGNATURE-----