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,
...).
> > @@ -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
