On Tue, 19 Nov 2019 at 19:51, Klaus Birkelund <i...@irrelevant.dk> wrote: > > On Tue, Nov 12, 2019 at 03:04:59PM +0000, Beata Michalska wrote: > > Hi Klaus, > > > > On Tue, 15 Oct 2019 at 11:49, Klaus Jensen <i...@irrelevant.dk> wrote: > > > @@ -1188,6 +1326,9 @@ static int nvme_start_ctrl(NvmeCtrl *n) > > > > > > nvme_set_timestamp(n, 0ULL); > > > > > > + n->aer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_process_aers, > > > n); > > > + QTAILQ_INIT(&n->aer_queue); > > > + > > > > Is the timer really needed here ? The CEQ can be posted either when > > requested > > by host through AER, if there are any pending events, or once the > > event is triggered > > and there are active AER's. > > > > I guess you are right. I mostly cribbed this from Keith's tree, but I > see no reason to keep the timer. > > Keith, do you have any comments on this? > > > > @@ -1380,6 +1521,13 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr > > > addr, int val) > > > "completion queue doorbell write" > > > " for nonexistent queue," > > > " sqid=%"PRIu32", ignoring", qid); > > > + > > > + if (n->outstanding_aers) { > > > + nvme_enqueue_event(n, NVME_AER_TYPE_ERROR, > > > + NVME_AER_INFO_ERR_INVALID_DB_REGISTER, > > > + NVME_LOG_ERROR_INFO); > > > + } > > > + > > This one (as well as cases below) might not be entirely right > > according to the spec. If given event is enabled for asynchronous > > reporting the controller should retain that even. In this case, the event > > will be ignored as there is no pending request. > > > > I understand these notifications to be special cases (i.e. they cannot > be enabled/disabled through the Asynchronous Event Configuration > feature). See Section 4.1 of NVM Express 1.2.1. The spec specifically > says that "... and an Asynchronous Event Request command is outstanding, > ...). >
OK, I have missed that one. Thanks for the reference. BR Beata > > > @@ -1591,6 +1759,7 @@ static void nvme_init_ctrl(NvmeCtrl *n) > > > id->ver = cpu_to_le32(0x00010201); > > > id->oacs = cpu_to_le16(0); > > > id->acl = 3; > > > + id->aerl = n->params.aerl; > > > > What about the configuration for the asynchronous events ? > > > > It will default to an AEC vector of 0 (everything disabled). > > > K