On Fri, Jun 26, 2015 at 12:46:07PM -0400, John Snow wrote:
> 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.Sounds good.
pgpbPlZBYTvjk.pgp
Description: PGP signature
