Re: git: 34a6ad848fbf - main - nvme: Don't use version to listen for events for ns and fw changes

On Fri, Nov 17, 2023 at 8:53 PM Yuri <yuri@aetern.org> wrote:
>
> Warner Losh wrote:
> > The branch main has been updated by imp:
> >
> > URL: https://cgit.FreeBSD.org/src/commit/?id=34a6ad848fbf471773a430a7e457bf49816e756e
> >
> > commit 34a6ad848fbf471773a430a7e457bf49816e756e
> > Author:     Warner Losh <imp@FreeBSD.org>
> > AuthorDate: 2023-11-18 04:24:00 +0000
> > Commit:     Warner Losh <imp@FreeBSD.org>
> > CommitDate: 2023-11-18 04:25:57 +0000
> >
> >     nvme: Don't use version to listen for events for ns and fw changes
...
> >       if (ctrlr->cdata.ver >= NVME_REV(1, 2))
>
> It seems to be under version check still? Or am I misreading the commit
> message?
>
> > -             ctrlr->async_event_config |= NVME_ASYNC_EVENT_NS_ATTRIBUTE |
> > -                 NVME_ASYNC_EVENT_FW_ACTIVATE;
> > +             ctrlr->async_event_config |=
> > +                 ctrlr->cdata.oaes & (NVME_ASYNC_EVENT_NS_ATTRIBUTE |
> > +                     NVME_ASYNC_EVENT_FW_ACTIVATE);

I think this logic is correct as the Namespace and Firmware events
didn't exist prior to version 1.2. The significant change here
(irrespective of the commit message wording) is masking these events
if the device doesn't support them. That is, their respective bit in
OAES (aka Optional Async Events Supported) will be zero if not
supported and then won't be added to the async_event_config.

Perhaps the intent was "Don't _only_ use the version to listen for
events, but also check they are supported".
My $.02

--chuck